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
775 stars 133 forks source link

Compiler Options: GCC 13.1 analyzer options #147

Open david-a-wheeler opened 1 year ago

david-a-wheeler commented 1 year ago

GCC 13.1 is now out. We should review its analyzer options, e.g.: https://gcc.gnu.org/gcc-13/changes.html I think it'd make sense to maximally enable the analyzers in most cases... but I'm happy to hear alternative views.

06kellyjac commented 1 year ago

Since this document is more about security and memory saftey IDK if it should go too deep into "general code quality" if thats' what a lot of the analyze rules do. If there are plenty of worthwhile security checks in analyze then maybe it is more sensible to recommend maximally enable


Also I didn't realise gcc could output sarif, very interesting

https://gcc.gnu.org/onlinedocs/gcc-13.1.0/gcc/Diagnostic-Message-Formatting-Options.html#index-fdiagnostics-format

siddhesh commented 1 year ago

Analyzer options are not suitable for default use even though we try to keep false positives/negatives down, but it would be great to have a section on analyzer (or even sanitizer) options for use in, e.g., CI or debug builds.

siddhesh commented 1 year ago

Since this document is more about security and memory saftey IDK if it should go too deep into "general code quality" if thats' what a lot of the analyze rules do. If there are plenty of worthwhile security checks in analyze then maybe it is more sensible to recommend maximally enable

There are actually a lot of security-relevant checks in the gcc analyzer like allocation tracking, but they're more heuristics than definite issues. Unfortunately some of the stringop warnings are heuristics too, which muddies the waters a bit.

davidmalcolm commented 1 year ago

(GCC analyzer author/maintainer here) Unfortunately there will be false positives (and false negatives) from the analyzer, so I can't recommend enabling it by default. I have a longer post about this that looks into the false positives rates per warning; I hope it will go live in the next week or so; sorry that it's not quite ready yet.

davidmalcolm commented 1 year ago

Also I didn't realise gcc could output sarif, very interesting

https://gcc.gnu.org/onlinedocs/gcc-13.1.0/gcc/Diagnostic-Message-Formatting-Options.html#index-fdiagnostics-format

FWIW there's some more information on GCC's SARIF support here: https://github.com/oasis-tcs/sarif-spec/issues/531 (and I'm now on the SARIF technical committee)

david-a-wheeler commented 1 year ago

I think whether or not an analyzer should be added should be considered case-by-case, considering how likely it is to identify a vulnerability (benefit) vs. likelihood of a false positive (cost). If it's finding vulnerabilities in many cases, but has occasional false positives that can be suppressed when they happen, that's likely to be worthwhile. At the least, that would make it worth discussion.

Flo4152 commented 1 year ago

Many GCC analyzers are linked to CWE. CWE gives useful technical details about potential security issues. these links to CWE give a first list of GCC analyzers related to vulnerabilities : arbitrary code execution (buffer overflow, use after free, double free, uninitialized buffer, and more), arbitrary memory access (read and write), denial of service and crash.

Flo4152 commented 12 months ago

Here a list of GCC analyzers made based on vulnerabilities described by CWEs from GCC official documentation. GCC - Static Analyzers.pdf

SecurityCRob commented 6 months ago

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

06kellyjac commented 6 months ago

This issue is in relation to the hardening guide. There are a few GCC 13 flags in the doc but IDK if we've covered all new relevant options in 13.1

thomasnyman commented 1 month ago

Revisiting this after analyzers were again brought up again in #330. The conclusion from this thread seems to be have been that it does not make sense to systematically cover the GCC static analyzers, but rather consider specific analyzers on a case by case basis. Since this issue is about analyzers as a whole, and I think the discussion here didn't narrow down specific analyzers to consider, can this issue be closed?

david-a-wheeler commented 1 month ago

Before closing it, let's document why we're not doing it.

Just before the section "What is our threat model, goal, and objective?" let's add a short section like this:

Why are general static analysis flags out-of-scope?

This guide identifies compiler hardening flags for generating instrumented test code and production code. We include flags that help identify constructs that are likely to lead to vulnerabilities, if they have few false positives and those are easily systemically silenced.

Many compilers can also act as a general static analysis tool that heuristically identifies suspicious constructs that may lead to bugs or vulnerabilities. However, these heuristics may often have false positives (reporting problems where none exist) and/or be difficult to silence to generate executable code. Using a compiler as a general static analysis tool, where a human is expected to read the reports instead of using the generated code, can be valuable but is outside the scope of this document.

davidmalcolm commented 1 month ago

(GCC analyzer author/maintainer here) Unfortunately there will be false positives (and false negatives) from the analyzer, so I can't recommend enabling it by default. I have a longer post about this that looks into the false positives rates per warning; I hope it will go live in the next week or so; sorry that it's not quite ready yet.

FWIW the article I wrote last year about this is here; see the "Fixing false positives" section.