ossf / wg-best-practices-os-developers

The Best Practices for OSS Developers working group is dedicated to raising awareness and education of secure code best practices for open source developers.
https://openssf.org
Apache License 2.0
736 stars 128 forks source link

Compiler Options Hardening Guide for C and C++, -D_LIBCPP_ASSERT #149

Open rcseacord opened 1 year ago

rcseacord commented 1 year ago

I tried using this with libc++ and it blew up pretty bad. I'm using _LIBCPP_DEBUG now instead.

https://libcxx.llvm.org/DesignDocs/DebugMode.html https://releases.llvm.org/12.0.0/projects/libcxx/docs/DesignDocs/DebugMode.html

The following page mentions _LIBCPP_ASSERT but doesn't say you should define it: https://releases.llvm.org/8.0.0/projects/libcxx/docs/DesignDocs/DebugMode.html

siddhesh commented 1 year ago

We added it under the impression that it's the libc++ equivalent of _GLIBCXX_ASSERTIONS (https://github.com/ossf/wg-best-practices-os-developers/pull/128 for context) but evidently, it's not a documented feature. I'm in favour of dropping it until it actually shows up in the libc++ documentation.

rcseacord commented 1 year ago

What about _LIBCPP_DEBUG?

siddhesh commented 1 year ago

It's a debug flag, so I'm not sure if it's suitable for this guide, which intends to document flags for use in production. It would be nice to have a compilation for CI and debug builds though, see also the suggestion (https://github.com/ossf/wg-best-practices-os-developers/issues/147) to document the analyzer option.

rcseacord commented 1 year ago

The following flags: -Wall -Wformat=2 -Wconversion -Wtrampolines -Werror Don't effect code production at all, so it sort of violates your principle that you are only interesting in recommending flags to harden production code.

In my guide, I specify a number of implementations corresponding to build, debug, test, and production phases.

These same warning flags probably fall well short of what you might want to specify in the build phase, which is mostly about maximizing (true positive) compiler diagnostics.

Taking this approach would create a more complete but much longer guide. But if you were to say very focused on hardening binaries then warning flags are out of scope.

thomasnyman commented 1 year ago

The following page mentions _LIBCPP_ASSERT but doesn't say you should define it: https://releases.llvm.org/8.0.0/projects/libcxx/docs/DesignDocs/DebugMode.html

There is also https://libcxx.llvm.org/UsingLibcxx.html#assertions-mode but that seems to refer to both -D_LIBCXX_ENABLE_ASSERTIONS and -D_LIBCPP_ENABLE_ASSERTIONS. Possible one of them is typo.

thomasnyman commented 1 year ago

Starting with libc++ release 17.0.0 -D_LIBCPP_ENABLE_ASSERTIONS has been deprecated in favor a new hardened mode of operation. Moving _LIBCPP_ASSERT to the 'considered options' table and will look into documenting the new hardened mode as a separate set of changes.

david-a-wheeler commented 1 year ago

@rcseacord - the goal is to create option flags to harden "production code", that is, code that will be eventually used in production instead of experimental code. It's not to just affect the "production OF code". That said, I see where the confusion lies. E.g., you want "-Wall" because it provides a lot of static checks that detect potential security issues that otherwise might slip unnoticed into production code.

thesamesam commented 11 months ago

Starting with libc++ release 17.0.0 -D_LIBCPP_ENABLE_ASSERTIONS has been deprecated in favor a new hardened mode of operation. Moving _LIBCPP_ASSERT to the 'considered options' table and will look into documenting the new hardened mode as a separate set of changes.

Unfortunately, the situation with LLVM is a mess at the moment. This got reverted very late in the LLVM 17 cycle in https://reviews.llvm.org/D159171.

Future plans are being discussed at https://discourse.llvm.org/t/rfc-hardening-in-libc/73925.

SecurityCRob commented 5 months ago

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