Add -fstack-protector support to wasi-libc

[kateinoigakukun]

Inlcude __stack_chk_fail.c and initialize __stack_chk_guard in ctor.
This allows userland stack smash protection.

$ cat main.c
char input[] = "0123456789012345";
int main(void) {
    char buf[8];

    for (char *sp = input, *dp = buf; *sp != '\0'; sp++, dp++) {
        *dp = *sp;
    }
    return 0;
}

$ clang main.c -fstack-protector
$ wasmtime ./a.out
Error: failed to run main module `./a.out`

Caused by:
    0: failed to invoke command default
    1: wasm trap: wasm `unreachable` instruction executed
       wasm backtrace:
           0:  0x258 - <unknown>!__stack_chk_fail
           1:  0x21e - <unknown>!__original_main
           2:   0xca - <unknown>!_start

[sbc100]

Seems reasonable.

IIUC -fstack-protector is about detecting cases of overflowing the individual stack frame. Does it detect stack overflow too? i.e. valid programs with too-deep recursion?

Is suppose the security benefits of -fstack-protector in the wasm world are fairly minimal (compared to native platforms)? But it can still be useful for detect program bugs?

[pchickey]

This uses wasi random_get at initialization time, which could make it impossible to run wizer on programs which expect to run a bunch of "constant" initialization code. Is there a benefit to using randomness for stack protection on the wasm platform? I can understand how that would be desirable on platforms where you want to randomize virtual memory layout, but I don't know what it is used for here.

[TerrorJack]

Would be nice to make it optional via a Makefile flag; I see value in -fstack-protector support, by I'd also like to make it easy to produce sysroots without the extra overhead in non-debug builds.

[kateinoigakukun]

@sbc100

IIUC -fstack-protector is about detecting cases of overflowing the individual stack frame. Does it detect stack overflow too? i.e. valid programs with too-deep recursion?

It doesn't detect stack overflow. wasm-ld's --stack-first can be used for the purpose, but...

Is suppose the security benefits of -fstack-protector in the wasm world are fairly minimal (compared to native platforms)? But it can still be useful for detect program bugs?

Yes, I agree that the security benefit would be small. TBH, my main motivation is just reducing if(wasm) in toolchains like this. But I think it's still useful to detect BOF bugs in C/C++ on Wasm.

@pchickey

This uses wasi random_get at initialization time, which could make it impossible to run wizer on programs which expect to run a bunch of "constant" initialization code.

Good point. To be precise, the ctor is included only when stack protector is enabled in user code, so it can be an issue when wizer and -fstack-protector are used for an executable.

Is there a benefit to using randomness for stack protection on the wasm platform? I can understand how that would be desirable on platforms where you want to randomize virtual memory layout, but I don't know what it is used for here.

The random seed is used for randomizing the canary. I think it's still useful without ASLR to prevent bypassing guards by overwriting statically known canary.

@TerrorJack
Currently, the __stack_chk_fail.c is always compiled in wasi-libc build system, but it's linked only when -fstack-protector is enabled. Do you think we still need additional flag in Makefile?

[sbc100]

The Makefile already supports EXTRA_CFALGS=xxx for folks that might want to compile libc itself with -fstack-protector enabled.

[TerrorJack]

Ah, I just experimented compiling with this patch and discovered there's no extra overhead if none of libc or user code is compiled with -fstack-protector, so it seems as long as the __stack_chk_fail symbol is not reachable, the object file will not be linked so that extra ctor will not come into play. In that case, no need for any extra Makefile logic then!

[kateinoigakukun]

@TerrorJack Yes! Thank you for experimenting on your side :)

[sunfishcode]

@TerrorJack Thanks for doing that experiment! This looks good.