]> git.ipfire.org Git - thirdparty/gcc.git/commit
analyzer: new warning: -Wanalyzer-tainted-assertion [PR106235]
authorDavid Malcolm <dmalcolm@redhat.com>
Sun, 13 Nov 2022 22:53:23 +0000 (17:53 -0500)
committerDavid Malcolm <dmalcolm@redhat.com>
Sun, 13 Nov 2022 22:53:23 +0000 (17:53 -0500)
commitd777b38cde91a87f2345dcd13901862a9513562a
treedeaa3de833f7d2f00338b187c2cc7d6c1e7d119f
parent58e7732a2feddf475e72b232bf16494d84a41acf
analyzer: new warning: -Wanalyzer-tainted-assertion [PR106235]

This patch adds a new -Wanalyzer-tainted-assertion warning to
-fanalyzer's "taint" mode (which also requires -fanalyzer-checker=taint).

It complains about attacker-controlled values being used in assertions,
or in any expression affecting control flow that guards a "noreturn"
function.  As noted in the docs part of the patch, in such cases:

  - when assertion-checking is enabled: an attacker could trigger
    a denial of service by injecting an assertion failure

  - when assertion-checking is disabled, such as by defining NDEBUG,
    an attacker could inject data that subverts the process, since it
    presumably violates a precondition that is being assumed by the code.

For example, given:

#include <assert.h>

int __attribute__((tainted_args))
test_tainted_assert (int n)
{
  assert (n > 0);
  return n * n;
}

compiling with
  -fanalyzer -fanalyzer-checker=taint
gives:

t.c: In function 'test_tainted_assert':
t.c:6:3: warning: use of attacked-controlled value in condition for assertion [CWE-617] [-Wanalyzer-tainted-assertion]
    6 |   assert (n > 0);
      |   ^~~~~~
  'test_tainted_assert': event 1
    |
    |    4 | test_tainted_assert (int n)
    |      | ^~~~~~~~~~~~~~~~~~~
    |      | |
    |      | (1) function 'test_tainted_assert' marked with '__attribute__((tainted_args))'
    |
    +--> 'test_tainted_assert': event 2
           |
           |    4 | test_tainted_assert (int n)
           |      | ^~~~~~~~~~~~~~~~~~~
           |      | |
           |      | (2) entry to 'test_tainted_assert'
           |
         'test_tainted_assert': events 3-6
           |
           |/usr/include/assert.h:106:10:
           |  106 |       if (expr)                                                         \
           |      |          ^
           |      |          |
           |      |          (3) use of attacker-controlled value for control flow
           |      |          (4) following 'false' branch (when 'n <= 0')...
           |......
           |  109 |         __assert_fail (#expr, __FILE__, __LINE__, __ASSERT_FUNCTION);   \
           |      |         ~~~~~~~~~~~~~
           |      |         |
           |      |         (5) ...to here
           |      |         (6) treating '__assert_fail' as an assertion failure handler due to '__attribute__((__noreturn__))'
           |

The testcases have various examples for BUG and BUG_ON from the
Linux kernel; there, the diagnostic treats "panic" as an assertion
failure handler, due to '__attribute__((__noreturn__))'.

gcc/analyzer/ChangeLog:
PR analyzer/106235
* analyzer.opt (Wanalyzer-tainted-assertion): New.
* checker-path.cc (checker_path::fixup_locations): Pass false to
pending_diagnostic::fixup_location.
* diagnostic-manager.cc (get_emission_location): Pass true to
pending_diagnostic::fixup_location.
* pending-diagnostic.cc (pending_diagnostic::fixup_location): Add
bool param.
* pending-diagnostic.h (pending_diagnostic::fixup_location): Add
bool param to decl.
* sm-taint.cc (taint_state_machine::m_tainted_control_flow): New.
(taint_diagnostic::describe_state_change): Drop "final".
(class tainted_assertion): New.
(taint_state_machine::taint_state_machine): Initialize
m_tainted_control_flow.
(taint_state_machine::alt_get_inherited_state): Support
comparisons being tainted, based on their arguments.
(is_assertion_failure_handler_p): New.
(taint_state_machine::on_stmt): Complain about calls to assertion
failure handlers guarded by an attacker-controller conditional.
Detect attacker-controlled gcond conditionals and gswitch index
values.
(taint_state_machine::check_control_flow_arg_for_taint): New.

gcc/ChangeLog:
PR analyzer/106235
* doc/gcc/gcc-command-options/option-summary.rst: Add
-Wno-analyzer-tainted-assertion.
* doc/gcc/gcc-command-options/options-that-control-static-analysis.rst:
Add -Wno-analyzer-tainted-assertion.

gcc/testsuite/ChangeLog:
PR analyzer/106235
* gcc.dg/analyzer/taint-assert-BUG_ON.c: New test.
* gcc.dg/analyzer/taint-assert-macro-expansion.c: New test.
* gcc.dg/analyzer/taint-assert.c: New test.
* gcc.dg/analyzer/taint-assert-system-header.c: New test.
* gcc.dg/analyzer/test-assert.h: New header.
* gcc.dg/plugin/analyzer_gil_plugin.c
(gil_diagnostic::fixup_location): Add bool param.

Signed-off-by: David Malcolm <dmalcolm@redhat.com>
14 files changed:
gcc/analyzer/analyzer.opt
gcc/analyzer/checker-path.cc
gcc/analyzer/diagnostic-manager.cc
gcc/analyzer/pending-diagnostic.cc
gcc/analyzer/pending-diagnostic.h
gcc/analyzer/sm-taint.cc
gcc/doc/gcc/gcc-command-options/option-summary.rst
gcc/doc/gcc/gcc-command-options/options-that-control-static-analysis.rst
gcc/testsuite/gcc.dg/analyzer/taint-assert-BUG_ON.c [new file with mode: 0644]
gcc/testsuite/gcc.dg/analyzer/taint-assert-macro-expansion.c [new file with mode: 0644]
gcc/testsuite/gcc.dg/analyzer/taint-assert-system-header.c [new file with mode: 0644]
gcc/testsuite/gcc.dg/analyzer/taint-assert.c [new file with mode: 0644]
gcc/testsuite/gcc.dg/analyzer/test-assert.h [new file with mode: 0644]
gcc/testsuite/gcc.dg/plugin/analyzer_gil_plugin.c