Skip to content
Snippets Groups Projects
Commit d777b38c authored by David Malcolm's avatar David Malcolm
Browse files

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: default avatarDavid Malcolm <dmalcolm@redhat.com>
parent 58e7732a
No related branches found
No related tags found
No related merge requests found
Showing
with 821 additions and 21 deletions
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment