]> git.ipfire.org Git - thirdparty/gcc.git/commit
analyzer: fix taint false positives with UNKNOWN [PR112850]
authorDavid Malcolm <dmalcolm@redhat.com>
Thu, 7 Dec 2023 00:25:26 +0000 (19:25 -0500)
committerDavid Malcolm <dmalcolm@redhat.com>
Thu, 7 Dec 2023 00:25:26 +0000 (19:25 -0500)
commit08b7462d3ad8e5acd941b7c777c5b26b4064d686
tree9d418e45863ed55b22f8fc4cb5422a1efe906ee2
parentae9e48e5c0acaecf181117bdf3632b6eabf907ec
analyzer: fix taint false positives with UNKNOWN [PR112850]

PR analyzer/112850 reports a false positive from
-Wanalyzer-tainted-allocation-size on the Linux kernel [1] where
-fanalyzer complains that an allocation size is attacker-controlled
despite the value being correctly sanitized against upper and lower
limits.

The root cause is that the expression is sufficiently complex
to exceed the -param=analyzer-max-svalue-depth= threshold,
currently at 12, with depth 13, and so it is treated as UNKNOWN.
Hence the sanitizations are seen as comparisons of an UNKNOWN
symbolic value against constants, and these were being ignored
by the taint state machine.

The expression in question is relatively typical for those seen in
Linux kernel ioctl handlers, and I was surprised that it had exceeded
the analyzer's default expression complexity limit.

This patch addresses this problem in three ways:
(a) the default value of the threshold parameter is increased, from 12
to 18, so that such expressions are precisely handled
(b) adding a new -Wanalyzer-symbol-too-complex to warn when the symbol
complexity limit is reached.  This is off by default for users, and
on by default in the test suite.
(c) the taint state machine handles comparisons against UNKNOWN svalues
by dropping all taint information on that execution path, so that if
the complexity limit has been exceeded we don't generate false positives

As well as fixing the taint false positive (PR analyzer/112850), the
patch also fixes a couple of leak false positives seen on flex-generated
scanners (PR analyzer/103546).

[1] specifically, in sound/core/rawmidi.c's handler for
SNDRV_RAWMIDI_STREAM_OUTPUT.

gcc/ChangeLog:
PR analyzer/103546
PR analyzer/112850
* doc/invoke.texi: Add -Wanalyzer-symbol-too-complex.

gcc/analyzer/ChangeLog:
PR analyzer/103546
PR analyzer/112850
* analyzer.opt (-param=analyzer-max-svalue-depth=): Increase from
12 to 18.
(Wanalyzer-symbol-too-complex): New.
* diagnostic-manager.cc
(null_assignment_sm_context::clear_all_per_svalue_state): New.
* engine.cc (impl_sm_context::clear_all_per_svalue_state): New.
* program-state.cc (sm_state_map::clear_all_per_svalue_state):
New.
* program-state.h (sm_state_map::clear_all_per_svalue_state): New
decl.
* region-model-manager.cc
(region_model_manager::reject_if_too_complex): Add
-Wanalyzer-symbol-too-complex.
* sm-taint.cc (taint_state_machine::on_condition): Handle
comparisons against UNKNOWN.
* sm.h (sm_context::clear_all_per_svalue_state): New.

gcc/testsuite/ChangeLog:
PR analyzer/103546
PR analyzer/112850
* c-c++-common/analyzer/call-summaries-pr107158-2.c: Add
-Wno-analyzer-symbol-too-complex.
* c-c++-common/analyzer/call-summaries-pr107158.c: Likewise.
* c-c++-common/analyzer/deref-before-check-pr109060-haproxy-cfgparse.c:
Likewise.
* c-c++-common/analyzer/feasibility-3.c: Add
-Wno-analyzer-too-complex and -Wno-analyzer-symbol-too-complex.
* c-c++-common/analyzer/flex-with-call-summaries.c: Add
-Wno-analyzer-symbol-too-complex.  Remove fail for
PR analyzer/103546 leak false positive.
* c-c++-common/analyzer/flex-without-call-summaries.c: Remove
xfail for PR analyzer/103546 leak false positive.
* c-c++-common/analyzer/infinite-recursion-3.c: Add
-Wno-analyzer-symbol-too-complex.
* c-c++-common/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c:
Likewise.
* c-c++-common/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early.c:
Likewise.
* c-c++-common/analyzer/null-deref-pr108400-SoftEtherVPN-WebUi.c:
Likewise.
* c-c++-common/analyzer/null-deref-pr108806-qemu.c: Likewise.
* c-c++-common/analyzer/null-deref-pr108830.c: Likewise.
* c-c++-common/analyzer/pr94596.c: Likewise.
* c-c++-common/analyzer/strtok-2.c: Likewise.
* c-c++-common/analyzer/strtok-4.c: Add -Wno-analyzer-too-complex
and -Wno-analyzer-symbol-too-complex.
* c-c++-common/analyzer/strtok-cppreference.c: Likewise.
* gcc.dg/analyzer/analyzer.exp: Add -Wanalyzer-symbol-too-complex
to DEFAULT_CFLAGS.
* gcc.dg/analyzer/attr-const-3.c: Add
-Wno-analyzer-symbol-too-complex.
* gcc.dg/analyzer/call-summaries-pr107072.c: Likewise.
* gcc.dg/analyzer/doom-s_sound-pr108867.c: Likewise.
* gcc.dg/analyzer/explode-4.c: Likewise.
* gcc.dg/analyzer/null-deref-pr102671-1.c: Likewise.
* gcc.dg/analyzer/null-deref-pr105755.c: Likewise.
* gcc.dg/analyzer/out-of-bounds-curl.c: Likewise.
* gcc.dg/analyzer/pr101503.c: Likewise.
* gcc.dg/analyzer/pr103892.c: Add -Wno-analyzer-too-complex and
-Wno-analyzer-symbol-too-complex.
* gcc.dg/analyzer/pr94851-4.c: Add
-Wno-analyzer-symbol-too-complex.
* gcc.dg/analyzer/pr96860-1.c: Likewise.
* gcc.dg/analyzer/pr96860-2.c: Likewise.
* gcc.dg/analyzer/pr98918.c: Likewise.
* gcc.dg/analyzer/pr99044-2.c: Likewise.
* gcc.dg/analyzer/uninit-pr108806-qemu.c: Likewise.
* gcc.dg/analyzer/use-after-free.c: Add -Wno-analyzer-too-complex
and -Wno-analyzer-symbol-too-complex.
* gcc.dg/plugin/plugin.exp: Add new tests for
analyzer_kernel_plugin.c.
* gcc.dg/plugin/taint-CVE-2011-0521-4.c: Update expected results.
* gcc.dg/plugin/taint-CVE-2011-0521-5.c: Likewise.
* gcc.dg/plugin/taint-CVE-2011-0521-6.c: Likewise.
* gcc.dg/plugin/taint-CVE-2011-0521-5-fixed.c: Remove xfail.
* gcc.dg/plugin/taint-pr112850-precise.c: New test.
* gcc.dg/plugin/taint-pr112850-too-complex.c: New test.
* gcc.dg/plugin/taint-pr112850-unsanitized.c: New test.
* gcc.dg/plugin/taint-pr112850.c: New test.

Signed-off-by: David Malcolm <dmalcolm@redhat.com>
51 files changed:
gcc/analyzer/analyzer.opt
gcc/analyzer/diagnostic-manager.cc
gcc/analyzer/engine.cc
gcc/analyzer/program-state.cc
gcc/analyzer/program-state.h
gcc/analyzer/region-model-manager.cc
gcc/analyzer/sm-taint.cc
gcc/analyzer/sm.h
gcc/doc/invoke.texi
gcc/testsuite/c-c++-common/analyzer/call-summaries-pr107158-2.c
gcc/testsuite/c-c++-common/analyzer/call-summaries-pr107158.c
gcc/testsuite/c-c++-common/analyzer/deref-before-check-pr109060-haproxy-cfgparse.c
gcc/testsuite/c-c++-common/analyzer/feasibility-3.c
gcc/testsuite/c-c++-common/analyzer/flex-with-call-summaries.c
gcc/testsuite/c-c++-common/analyzer/flex-without-call-summaries.c
gcc/testsuite/c-c++-common/analyzer/infinite-recursion-3.c
gcc/testsuite/c-c++-common/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c
gcc/testsuite/c-c++-common/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early.c
gcc/testsuite/c-c++-common/analyzer/null-deref-pr108400-SoftEtherVPN-WebUi.c
gcc/testsuite/c-c++-common/analyzer/null-deref-pr108806-qemu.c
gcc/testsuite/c-c++-common/analyzer/null-deref-pr108830.c
gcc/testsuite/c-c++-common/analyzer/pr94596.c
gcc/testsuite/c-c++-common/analyzer/strtok-2.c
gcc/testsuite/c-c++-common/analyzer/strtok-4.c
gcc/testsuite/c-c++-common/analyzer/strtok-cppreference.c
gcc/testsuite/gcc.dg/analyzer/analyzer.exp
gcc/testsuite/gcc.dg/analyzer/attr-const-3.c
gcc/testsuite/gcc.dg/analyzer/call-summaries-pr107072.c
gcc/testsuite/gcc.dg/analyzer/doom-s_sound-pr108867.c
gcc/testsuite/gcc.dg/analyzer/explode-4.c
gcc/testsuite/gcc.dg/analyzer/null-deref-pr102671-1.c
gcc/testsuite/gcc.dg/analyzer/null-deref-pr105755.c
gcc/testsuite/gcc.dg/analyzer/out-of-bounds-curl.c
gcc/testsuite/gcc.dg/analyzer/pr101503.c
gcc/testsuite/gcc.dg/analyzer/pr103892.c
gcc/testsuite/gcc.dg/analyzer/pr94851-4.c
gcc/testsuite/gcc.dg/analyzer/pr96860-1.c
gcc/testsuite/gcc.dg/analyzer/pr96860-2.c
gcc/testsuite/gcc.dg/analyzer/pr98918.c
gcc/testsuite/gcc.dg/analyzer/pr99044-2.c
gcc/testsuite/gcc.dg/analyzer/uninit-pr108806-qemu.c
gcc/testsuite/gcc.dg/analyzer/use-after-free.c
gcc/testsuite/gcc.dg/plugin/plugin.exp
gcc/testsuite/gcc.dg/plugin/taint-CVE-2011-0521-4.c
gcc/testsuite/gcc.dg/plugin/taint-CVE-2011-0521-5-fixed.c
gcc/testsuite/gcc.dg/plugin/taint-CVE-2011-0521-5.c
gcc/testsuite/gcc.dg/plugin/taint-CVE-2011-0521-6.c
gcc/testsuite/gcc.dg/plugin/taint-pr112850-precise.c [new file with mode: 0644]
gcc/testsuite/gcc.dg/plugin/taint-pr112850-too-complex.c [new file with mode: 0644]
gcc/testsuite/gcc.dg/plugin/taint-pr112850-unsanitized.c [new file with mode: 0644]
gcc/testsuite/gcc.dg/plugin/taint-pr112850.c [new file with mode: 0644]