Compiler flags notes/comments

[disconnect3d]

Forwarding this here from the #wg_best_practices_compilers OpenSSF slack channel, so we don't loose it. I am going to create separate issues or PRs from the poins below in the upcomming week[s].

Hey, Dominik Czarnota from Trail of Bits here and I've been working on similar internal guide in the past; I'd like to contribute to the compiler opts hardening guide in the future, but for now, I will send here my random notes about its contents.

1. We should recommend explicitly setting the "separate code" linker flag: -Wl,-z,separate-code which, e.g., makes so that the ELF header is not mapped with executable rights (https://bitlackeys.org/papers/secure_code_partitioning_2018.txt). This can be easily seen/compared against a binary compiled with -Wl,-z,no-separate-code and checking out info proc mappings (or Pwndbg's vmmap) in GDB.

2. I think the "When not to use?" for "3.14. Enable data execution prevention" regarding JIT apps makes not much sense? Most JIT apps would not care about -z noexecstack because they would not use stack region and would set or change memory permissions using mmap or mprotect anyway and -z noexecstack mostly influences how the memory of the program is mapped initially.

3. The description for 3.14 could also be more clear that if you compile a program with -z noexecstack but it requires executable code (e.g., when using nested functions, feature from GCC) the program will just crash when it will jump to the trampoline (since the generated code will put the trampoline on the stack and jump to it, and stack will be non-executable).
   (I have got an example code for that fwiw)

4. Fwiw regarding 3.14, in Linux, before version 5.8, if a program is run with executable stack then the kernel will set all other readable pages as executable and not just its stack memory. This is because of some elf_read_implies_exec sheniganis and changes within kernel code. (I can provide more details to it)

5. The "4. Discouraged Compiler Options" could mention -mmitigate-rop which was there once but then was removed since it didn't provide much benefit

6. Re: "Enabling -fstack-protector-strong is recommended as it provides the best balance between function coverage and performance". I recommended -fstack-protector-strong + --param ssp-buffer-size=4 in the past (while =8 is the default) but yeah, would be nice to read more on that.

• NOTE: best -> good #329 pretty much resolves this

7. "-D_FORTIFY_SOURCE=3 (requires -O1 or higher, may require prepending -U_FORTIFY_SOURCE)" - iirc the value =1 requires -O1 and =2 or =3 requires -O2, but I'd have to double check that. I'd also recommend checking if =3 works if your compiler does not support this value, since it is a fairly new addition.

8. Regarding GOT, note that "bind now" options are only relevant for the given binary we compile: the libc, libstdc++ and other libs will still have writable function pointers in GOT/GOTPLT if they are not compiled with those options.
   FWIW there is also a LD_BIND_NOW env var supported by ld.so, but from my tests now it seems it doesn't improve this for libc/libstdc++ at all.

9. In 3.16.2 the "The x86_64 architecture supports mov instructions that address memory using offsets relative to the instruction pointer" could be rephrased. This feature/thing is called "RIP-relative addressing" and its not just about mov instructions as e.g. lea (and likely some others) will also leverage that.

10. There are some compiler flags that were there at some point/version in time but were removed, for example:

• clang had an experimental -Wlifetime which was changed to two other flags (-Wdangling-gsl -Wreturn-stack-address) (https://stackoverflow.com/questions/52662135/the-purpose-of-the-wlifetime-flag). Clang docs says the two diagnostic is enabled by default but idk if its enabled with -Wall -Wextra or, e.g., only within the scan-build tool.

11. There's also -ftrapv that can be used to trap overflows, but I haven't played with it much and some people says its better to use sanitizers for that (https://news.ycombinator.com/item?id=24578534)

12. We should probably also mention GCC's -fanalyzer flag

[david-a-wheeler]

Thanks so much! We really appreciate the feedback!

[thomasnyman]

2. should be addressed by Remove considerations for JIT in -Wl,-z,noexecstack synopsis #353

[SecurityCRob]

Has this been addressed by the C/C++ Compiler Hardening options guide? @gkunz @thomasnyman @david-a-wheeler

[disconnect3d]

I believe not. I may have some time next week to send some PRs about some of these.

…

On Wed, 8 May 2024 at 20:37, CRob ***@***.***> wrote: Has this been addressed by the C/C++ Compiler Hardening options guide? @gkunz <https://github.com/gkunz> @thomasnyman <https://github.com/thomasnyman> @david-a-wheeler <https://github.com/david-a-wheeler> — Reply to this email directly, view it on GitHub <#330 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACMLWCT2WVFSPECSAN5ZUKLZBJWGHAVCNFSM6AAAAABADE6BACVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMBRGE4TQNRYGI> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[thomasnyman]

Trying to slowly work through these. I'm trying to split of suggestions for concrete additions to separate issues to allow them to be commented on and tracked separately, currently #588 and #589.

9. should be address by Refer to 'RIP-relative addressing' in position-independent code description #590

[thomasnyman]

3. and 4. should be addressed by Clarify the interaction between trampolines and -Wl,-z,noexecstack #611

@disconnect3d if your example code is short that could potentially be valuable to add as an example to either the description of -Wtrampolines or -Wl,-z,noexecstack

[thomasnyman]

For 7. I believe @siddhesh confirmed when the -D_FORTIFY_SOURCE recommendation was written that -O1 is the requirement, but -O2 or higher is recommended as it improves the accuracy of __builtin_object_size and __builtin_dynamic_object_size the -D_FORTIFY_SOURCE={2,3} instrumentation relies on. I've proposed some clarifying text to the corresponding "additional considerations" section in #620

[thomasnyman]

8. should be addressed by Add note on enabling RELRO for library dependencies #640

[thomasnyman]

Split 10. and 11 off to #684 and #685 respectively. 12 is a duplicate of #145.

I'm proposing closing this issue as the feedback here has now either been addressed or can be tracked via the more narrowly scoped issues.

[disconnect3d]

I personally like the fact of receiving notifications about the progress here but since we are moving to other cards, I am okay with closing this one. I'll schedule some time in upcomming weeks to revisit/review all the work that has been done here and see if I can fit in some gaps etc. :)

[thomasnyman]

I'm closing this then in favor of the more narrowly scoped tickets to track the work on the specific options to consider.