* [committed] analyzer: new warning: -Wanalyzer-tainted-assertion [PR106235]
@ 2022-11-13 23:03 David Malcolm
0 siblings, 0 replies; only message in thread
From: David Malcolm @ 2022-11-13 23:03 UTC (permalink / raw)
To: gcc-patches; +Cc: David Malcolm
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__))'.
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to trunk as r13-3947-gd777b38cde91a8.
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>
---
gcc/analyzer/analyzer.opt | 4 +
gcc/analyzer/checker-path.cc | 2 +-
gcc/analyzer/diagnostic-manager.cc | 2 +-
gcc/analyzer/pending-diagnostic.cc | 2 +-
gcc/analyzer/pending-diagnostic.h | 8 +-
gcc/analyzer/sm-taint.cc | 183 ++++++++-
.../gcc-command-options/option-summary.rst | 1 +
.../options-that-control-static-analysis.rst | 60 +++
.../gcc.dg/analyzer/taint-assert-BUG_ON.c | 76 ++++
.../analyzer/taint-assert-macro-expansion.c | 96 +++++
| 52 +++
gcc/testsuite/gcc.dg/analyzer/taint-assert.c | 346 ++++++++++++++++++
gcc/testsuite/gcc.dg/analyzer/test-assert.h | 7 +
.../gcc.dg/plugin/analyzer_gil_plugin.c | 3 +-
14 files changed, 821 insertions(+), 21 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/analyzer/taint-assert-BUG_ON.c
create mode 100644 gcc/testsuite/gcc.dg/analyzer/taint-assert-macro-expansion.c
create mode 100644 gcc/testsuite/gcc.dg/analyzer/taint-assert-system-header.c
create mode 100644 gcc/testsuite/gcc.dg/analyzer/taint-assert.c
create mode 100644 gcc/testsuite/gcc.dg/analyzer/test-assert.h
diff --git a/gcc/analyzer/analyzer.opt b/gcc/analyzer/analyzer.opt
index 518a5d422ff..49448697046 100644
--- a/gcc/analyzer/analyzer.opt
+++ b/gcc/analyzer/analyzer.opt
@@ -174,6 +174,10 @@ Wanalyzer-tainted-array-index
Common Var(warn_analyzer_tainted_array_index) Init(1) Warning
Warn about code paths in which an unsanitized value is used as an array index.
+Wanalyzer-tainted-assertion
+Common Var(warn_analyzer_tainted_assertion) Init(1) Warning
+Warn about code paths in which an 'assert()' is made involving an unsanitized value.
+
Wanalyzer-tainted-divisor
Common Var(warn_analyzer_tainted_divisor) Init(1) Warning
Warn about code paths in which an unsanitized value is used as a divisor.
diff --git a/gcc/analyzer/checker-path.cc b/gcc/analyzer/checker-path.cc
index 39de7453f51..e548ede3821 100644
--- a/gcc/analyzer/checker-path.cc
+++ b/gcc/analyzer/checker-path.cc
@@ -1316,7 +1316,7 @@ void
checker_path::fixup_locations (pending_diagnostic *pd)
{
for (checker_event *e : m_events)
- e->set_location (pd->fixup_location (e->get_location ()));
+ e->set_location (pd->fixup_location (e->get_location (), false));
}
/* Return true if there is a (start_cfg_edge_event, end_cfg_edge_event) pair
diff --git a/gcc/analyzer/diagnostic-manager.cc b/gcc/analyzer/diagnostic-manager.cc
index 9b319c3a3f5..1b19e58201b 100644
--- a/gcc/analyzer/diagnostic-manager.cc
+++ b/gcc/analyzer/diagnostic-manager.cc
@@ -933,7 +933,7 @@ get_emission_location (const gimple *stmt, function *fun,
location_t loc = get_stmt_location (stmt, fun);
/* Allow the pending_diagnostic to fix up the location. */
- loc = pd.fixup_location (loc);
+ loc = pd.fixup_location (loc, true);
return loc;
}
diff --git a/gcc/analyzer/pending-diagnostic.cc b/gcc/analyzer/pending-diagnostic.cc
index 9216a22e64d..53cab2065dd 100644
--- a/gcc/analyzer/pending-diagnostic.cc
+++ b/gcc/analyzer/pending-diagnostic.cc
@@ -153,7 +153,7 @@ fixup_location_in_macro_p (cpp_hashnode *macro)
Don't unwind inside macros for which fixup_location_in_macro_p is true. */
location_t
-pending_diagnostic::fixup_location (location_t loc) const
+pending_diagnostic::fixup_location (location_t loc, bool) const
{
if (linemap_location_from_macro_expansion_p (line_table, loc))
{
diff --git a/gcc/analyzer/pending-diagnostic.h b/gcc/analyzer/pending-diagnostic.h
index 0e91e71a208..5d5d126b342 100644
--- a/gcc/analyzer/pending-diagnostic.h
+++ b/gcc/analyzer/pending-diagnostic.h
@@ -214,10 +214,10 @@ class pending_diagnostic
diagnostic deduplication. */
static bool same_tree_p (tree t1, tree t2);
- /* A vfunc for fixing up locations (both the primary location for the
- diagnostic, and for events in their paths), e.g. to avoid unwinding
- inside specific macros. */
- virtual location_t fixup_location (location_t loc) const;
+ /* Vfunc for fixing up locations, e.g. to avoid unwinding
+ inside specific macros. PRIMARY is true for the primary location
+ for the diagnostic, and FALSE for events in their paths. */
+ virtual location_t fixup_location (location_t loc, bool primary) const;
/* For greatest precision-of-wording, the various following "describe_*"
virtual functions give the pending diagnostic a way to describe events
diff --git a/gcc/analyzer/sm-taint.cc b/gcc/analyzer/sm-taint.cc
index d4ee6c435b8..a2b442a4ef2 100644
--- a/gcc/analyzer/sm-taint.cc
+++ b/gcc/analyzer/sm-taint.cc
@@ -109,6 +109,10 @@ public:
state_t combine_states (state_t s0, state_t s1) const;
private:
+ void check_control_flow_arg_for_taint (sm_context *sm_ctxt,
+ const gimple *stmt,
+ tree expr) const;
+
void check_for_tainted_size_arg (sm_context *sm_ctxt,
const supernode *node,
const gcall *call,
@@ -130,6 +134,9 @@ public:
/* Stop state, for a value we don't want to track any more. */
state_t m_stop;
+
+ /* Global state, for when the last condition had tainted arguments. */
+ state_t m_tainted_control_flow;
};
/* Class for diagnostics relating to taint_state_machine. */
@@ -149,8 +156,7 @@ public:
&& m_has_bounds == other.m_has_bounds);
}
- label_text describe_state_change (const evdesc::state_change &change)
- final override
+ label_text describe_state_change (const evdesc::state_change &change) override
{
if (change.m_new_state == m_sm.m_tainted)
{
@@ -761,6 +767,100 @@ private:
enum memory_space m_mem_space;
};
+/* Concrete taint_diagnostic subclass for reporting attacker-controlled
+ value being used as part of the condition of an assertion. */
+
+class tainted_assertion : public taint_diagnostic
+{
+public:
+ tainted_assertion (const taint_state_machine &sm, tree arg,
+ tree assert_failure_fndecl)
+ : taint_diagnostic (sm, arg, BOUNDS_NONE),
+ m_assert_failure_fndecl (assert_failure_fndecl)
+ {
+ gcc_assert (m_assert_failure_fndecl);
+ }
+
+ const char *get_kind () const final override
+ {
+ return "tainted_assertion";
+ }
+
+ bool subclass_equal_p (const pending_diagnostic &base_other) const override
+ {
+ if (!taint_diagnostic::subclass_equal_p (base_other))
+ return false;
+ const tainted_assertion &other
+ = (const tainted_assertion &)base_other;
+ return m_assert_failure_fndecl == other.m_assert_failure_fndecl;
+ }
+
+ int get_controlling_option () const final override
+ {
+ return OPT_Wanalyzer_tainted_assertion;
+ }
+
+ bool emit (rich_location *rich_loc) final override
+ {
+ diagnostic_metadata m;
+ /* "CWE-617: Reachable Assertion". */
+ m.add_cwe (617);
+
+ return warning_meta (rich_loc, m, get_controlling_option (),
+ "use of attacked-controlled value in"
+ " condition for assertion");
+ }
+
+ location_t fixup_location (location_t loc,
+ bool primary) const final override
+ {
+ if (primary)
+ /* For the primary location we want to avoid being in e.g. the
+ <assert.h> system header, since this would suppress the
+ diagnostic. */
+ return expansion_point_location_if_in_system_header (loc);
+ else if (in_system_header_at (loc))
+ /* For events, we want to show the implemenation of the assert
+ macro when we're describing them. */
+ return linemap_resolve_location (line_table, loc,
+ LRK_SPELLING_LOCATION,
+ NULL);
+ else
+ return pending_diagnostic::fixup_location (loc, primary);
+ }
+
+ label_text describe_state_change (const evdesc::state_change &change) override
+ {
+ if (change.m_new_state == m_sm.m_tainted_control_flow)
+ return change.formatted_print
+ ("use of attacker-controlled value for control flow");
+ return taint_diagnostic::describe_state_change (change);
+ }
+
+ label_text describe_final_event (const evdesc::final_event &ev) final override
+ {
+ if (mention_noreturn_attribute_p ())
+ return ev.formatted_print
+ ("treating %qE as an assertion failure handler"
+ " due to %<__attribute__((__noreturn__))%>",
+ m_assert_failure_fndecl);
+ else
+ return ev.formatted_print
+ ("treating %qE as an assertion failure handler",
+ m_assert_failure_fndecl);
+ }
+
+private:
+ bool mention_noreturn_attribute_p () const
+ {
+ if (fndecl_built_in_p (m_assert_failure_fndecl, BUILT_IN_UNREACHABLE))
+ return false;
+ return true;
+ }
+
+ tree m_assert_failure_fndecl;
+};
+
/* taint_state_machine's ctor. */
taint_state_machine::taint_state_machine (logger *logger)
@@ -770,6 +870,7 @@ taint_state_machine::taint_state_machine (logger *logger)
m_has_lb = add_state ("has_lb");
m_has_ub = add_state ("has_ub");
m_stop = add_state ("stop");
+ m_tainted_control_flow = add_state ("tainted-control-flow");
}
state_machine::state_t
@@ -810,6 +911,15 @@ taint_state_machine::alt_get_inherited_state (const sm_state_map &map,
{
default:
break;
+
+ case EQ_EXPR:
+ case GE_EXPR:
+ case LE_EXPR:
+ case NE_EXPR:
+ case GT_EXPR:
+ case LT_EXPR:
+ case UNORDERED_EXPR:
+ case ORDERED_EXPR:
case PLUS_EXPR:
case MINUS_EXPR:
case MULT_EXPR:
@@ -823,17 +933,6 @@ taint_state_machine::alt_get_inherited_state (const sm_state_map &map,
}
break;
- case EQ_EXPR:
- case GE_EXPR:
- case LE_EXPR:
- case NE_EXPR:
- case GT_EXPR:
- case LT_EXPR:
- case UNORDERED_EXPR:
- case ORDERED_EXPR:
- /* Comparisons are just booleans. */
- return m_start;
-
case BIT_AND_EXPR:
case RSHIFT_EXPR:
return NULL;
@@ -844,6 +943,19 @@ taint_state_machine::alt_get_inherited_state (const sm_state_map &map,
return NULL;
}
+/* Return true iff FNDECL should be considered to be an assertion failure
+ handler by -Wanalyzer-tainted-assertion. */
+
+static bool
+is_assertion_failure_handler_p (tree fndecl)
+{
+ // i.e. "noreturn"
+ if (TREE_THIS_VOLATILE (fndecl))
+ return true;
+
+ return false;
+}
+
/* Implementation of state_machine::on_stmt vfunc for taint_state_machine. */
bool
@@ -871,6 +983,14 @@ taint_state_machine::on_stmt (sm_context *sm_ctxt,
/* External function with "access" attribute. */
if (sm_ctxt->unknown_side_effects_p ())
check_for_tainted_size_arg (sm_ctxt, node, call, callee_fndecl);
+
+ if (is_assertion_failure_handler_p (callee_fndecl)
+ && sm_ctxt->get_global_state () == m_tainted_control_flow)
+ {
+ sm_ctxt->warn (node, call, NULL_TREE,
+ make_unique<tainted_assertion> (*this, NULL_TREE,
+ callee_fndecl));
+ }
}
// TODO: ...etc; many other sources of untrusted data
@@ -897,9 +1017,46 @@ taint_state_machine::on_stmt (sm_context *sm_ctxt,
}
}
+ if (const gcond *cond = dyn_cast <const gcond *> (stmt))
+ {
+ /* Reset the state of "tainted-control-flow" before each
+ control flow statement, so that only the last one before
+ an assertion-failure-handler counts. */
+ sm_ctxt->set_global_state (m_start);
+ check_control_flow_arg_for_taint (sm_ctxt, cond, gimple_cond_lhs (cond));
+ check_control_flow_arg_for_taint (sm_ctxt, cond, gimple_cond_rhs (cond));
+ }
+
+ if (const gswitch *switch_ = dyn_cast <const gswitch *> (stmt))
+ {
+ /* Reset the state of "tainted-control-flow" before each
+ control flow statement, so that only the last one before
+ an assertion-failure-handler counts. */
+ sm_ctxt->set_global_state (m_start);
+ check_control_flow_arg_for_taint (sm_ctxt, switch_,
+ gimple_switch_index (switch_));
+ }
+
return false;
}
+/* If EXPR is tainted, mark this execution path with the
+ "tainted-control-flow" global state, in case we're about
+ to call an assertion-failure-handler. */
+
+void
+taint_state_machine::check_control_flow_arg_for_taint (sm_context *sm_ctxt,
+ const gimple *stmt,
+ tree expr) const
+{
+ const region_model *old_model = sm_ctxt->get_old_region_model ();
+ const svalue *sval = old_model->get_rvalue (expr, NULL);
+ state_t state = sm_ctxt->get_state (stmt, sval);
+ enum bounds b;
+ if (get_taint (state, TREE_TYPE (expr), &b))
+ sm_ctxt->set_global_state (m_tainted_control_flow);
+}
+
/* Implementation of state_machine::on_condition vfunc for taint_state_machine.
Potentially transition state 'tainted' to 'has_ub' or 'has_lb',
and states 'has_ub' and 'has_lb' to 'stop'. */
diff --git a/gcc/doc/gcc/gcc-command-options/option-summary.rst b/gcc/doc/gcc/gcc-command-options/option-summary.rst
index d068f98feac..b90b6600d70 100644
--- a/gcc/doc/gcc/gcc-command-options/option-summary.rst
+++ b/gcc/doc/gcc/gcc-command-options/option-summary.rst
@@ -309,6 +309,7 @@ in the following sections.
:option:`-Wno-analyzer-shift-count-overflow` |gol|
:option:`-Wno-analyzer-stale-setjmp-buffer` |gol|
:option:`-Wno-analyzer-tainted-allocation-size` |gol|
+ :option:`-Wno-analyzer-tainted-assertion` |gol|
:option:`-Wno-analyzer-tainted-array-index` |gol|
:option:`-Wno-analyzer-tainted-divisor` |gol|
:option:`-Wno-analyzer-tainted-offset` |gol|
diff --git a/gcc/doc/gcc/gcc-command-options/options-that-control-static-analysis.rst b/gcc/doc/gcc/gcc-command-options/options-that-control-static-analysis.rst
index 32a626c16a9..18f73d95e1e 100644
--- a/gcc/doc/gcc/gcc-command-options/options-that-control-static-analysis.rst
+++ b/gcc/doc/gcc/gcc-command-options/options-that-control-static-analysis.rst
@@ -549,6 +549,66 @@ Options That Control Static Analysis
Default setting; overrides :option:`-Wno-analyzer-tainted-allocation-size`.
+.. option:: -Wno-analyzer-tainted-assertion
+
+ This warning requires both :option:`-fanalyzer` and
+ :option:`-fanalyzer-checker=taint` to enable it;
+ use :option:`-Wno-analyzer-tainted-assertion` to disable it.
+
+ This diagnostic warns for paths through the code in which a value
+ that could be under an attacker's control is used as part of a
+ condition without being first sanitized, and that condition guards a
+ call to a function marked with attribute :fn-attr:`noreturn`
+ (such as the function ``__builtin_unreachable``). Such functions
+ typically indicate abnormal termination of the program, such as for
+ assertion failure handlers. For example:
+
+ .. code-block:: c
+
+ assert (some_tainted_value < SOME_LIMIT);
+
+ 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.
+
+ Note that when assertion-checking is disabled, the assertions are
+ typically removed by the preprocessor before the analyzer has a chance
+ to "see" them, so this diagnostic can only generate warnings on builds
+ in which assertion-checking is enabled.
+
+ For the purpose of this warning, any function marked with attribute
+ :fn-attr:`noreturn` is considered as a possible assertion failure
+ handler, including ``__builtin_unreachable``. Note that these functions
+ are sometimes removed by the optimizer before the analyzer "sees" them.
+ Hence optimization should be disabled when attempting to trigger this
+ diagnostic.
+
+ See `CWE-617: Reachable Assertion <https://cwe.mitre.org/data/definitions/617.html>`_.
+
+ The warning can also report problematic constructions such as
+
+ .. code-block:: c
+
+ switch (some_tainted_value) {
+ case 0:
+ /* [...etc; various valid cases omitted...] */
+ break;
+
+ default:
+ __builtin_unreachable (); /* BUG: attacker can trigger this */
+ }
+
+ despite the above not being an assertion failure, strictly speaking.
+
+.. option:: -Wanalyzer-tainted-assertion
+
+ Default setting; overrides :option:`-Wno-analyzer-tainted-assertion`.
+
.. option:: -Wno-analyzer-tainted-array-index
This warning requires both :option:`-fanalyzer` and
diff --git a/gcc/testsuite/gcc.dg/analyzer/taint-assert-BUG_ON.c b/gcc/testsuite/gcc.dg/analyzer/taint-assert-BUG_ON.c
new file mode 100644
index 00000000000..8aef0a44a6d
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/taint-assert-BUG_ON.c
@@ -0,0 +1,76 @@
+// TODO: remove need for this option
+/* { dg-additional-options "-fanalyzer-checker=taint" } */
+
+/* We need this, otherwise the warnings are emitted inside the macros, which
+ makes it hard to write the DejaGnu directives. */
+/* { dg-additional-options " -ftrack-macro-expansion=0" } */
+
+/* Adapted from code in the Linux kernel, which has this: */
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#define __noreturn __attribute__ ((__noreturn__))
+
+void panic(const char *fmt, ...) __noreturn;
+
+int _printk(const char *fmt, ...);
+#define __printk_index_emit(...) do {} while (0)
+#define printk_index_wrap(_p_func, _fmt, ...) \
+ ({ \
+ __printk_index_emit(_fmt, NULL, NULL); \
+ _p_func(_fmt, ##__VA_ARGS__); \
+ })
+#define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__)
+#define barrier_before_unreachable() do { } while (0)
+
+#define BUG() do { \
+ printk("BUG: failure at %s:%d/%s()!\n", __FILE__, __LINE__, __func__); \
+ barrier_before_unreachable(); \
+ panic("BUG!"); \
+} while (0)
+
+#define BUG_ON(condition) do { if (condition) BUG(); } while (0)
+
+void __attribute__((tainted_args))
+test_BUG(int n)
+{
+ if (n > 100) /* { dg-message "use of attacker-controlled value for control flow" } */
+ BUG(); /* { dg-warning "-Wanalyzer-tainted-assertion" "warning" } */
+ /* { dg-message "treating 'panic' as an assertion failure handler due to '__attribute__\\(\\(__noreturn__\\)\\)'" "final event" { target *-*-* } .-1 } */
+}
+
+void __attribute__((tainted_args))
+test_BUG_ON(int n)
+{
+ BUG_ON(n > 100); /* { dg-warning "-Wanalyzer-tainted-assertion" "warning" } */
+ /* { dg-message "treating 'panic' as an assertion failure handler due to '__attribute__\\(\\(__noreturn__\\)\\)'" "final event" { target *-*-* } .-1 } */
+}
+
+int __attribute__((tainted_args))
+test_switch_BUG_1(int n)
+{
+ switch (n) { /* { dg-message "use of attacker-controlled value for control flow" } */
+ default:
+ case 0:
+ return 5;
+ case 1:
+ return 22;
+ case 2:
+ return -1;
+ case 42:
+ BUG (); /* { dg-warning "-Wanalyzer-tainted-assertion" } */
+ }
+}
+
+int __attribute__((tainted_args))
+test_switch_BUG(int n)
+{
+ switch (n) { /* { dg-message "use of attacker-controlled value for control flow" } */
+ case 0:
+ return 5;
+ case 1:
+ return 22;
+ case 2:
+ return -1;
+ }
+ BUG (); /* { dg-warning "-Wanalyzer-tainted-assertion" } */
+}
diff --git a/gcc/testsuite/gcc.dg/analyzer/taint-assert-macro-expansion.c b/gcc/testsuite/gcc.dg/analyzer/taint-assert-macro-expansion.c
new file mode 100644
index 00000000000..24b175a0973
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/taint-assert-macro-expansion.c
@@ -0,0 +1,96 @@
+/* Integration test of how the execution path looks for
+ -Wanalyzer-tainted-assertion with macro-tracking enabled
+ (the default). */
+
+// TODO: remove need for this option
+/* { dg-additional-options "-fanalyzer-checker=taint" } */
+
+/* { dg-additional-options "-fdiagnostics-show-path-depths" } */
+/* { dg-additional-options "-fdiagnostics-path-format=inline-events -fdiagnostics-show-caret" } */
+
+/* An assertion macro that has a call to a __noreturn__ function. */
+
+extern void my_assert_fail (const char *expr, const char *file, int line)
+ __attribute__ ((__noreturn__));
+
+#define MY_ASSERT_1(EXPR) \
+ do { if (!(EXPR)) my_assert_fail (#EXPR, __FILE__, __LINE__); } while (0) /* { dg-warning "use of attacked-controlled value in condition for assertion \\\[CWE-617\\\] \\\[-Wanalyzer-tainted-assertion\\\]" } */
+
+int __attribute__((tainted_args))
+test_tainted_MY_ASSERT_1 (int n)
+{
+ MY_ASSERT_1 (n > 0);
+ return n * n;
+}
+
+/* { dg-begin-multiline-output "" }
+ do { if (!(EXPR)) my_assert_fail (#EXPR, __FILE__, __LINE__); } while (0)
+ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ { dg-end-multiline-output "" } */
+// note: in expansion of macro 'MY_ASSERT_1'
+/* { dg-begin-multiline-output "" }
+ MY_ASSERT_1 (n > 0);
+ ^~~~~~~~~~~
+ 'test_tainted_MY_ASSERT_1': event 1 (depth 0)
+ |
+ | test_tainted_MY_ASSERT_1 (int n)
+ | ^~~~~~~~~~~~~~~~~~~~~~~~
+ | |
+ | (1) function 'test_tainted_MY_ASSERT_1' marked with '__attribute__((tainted_args))'
+ |
+ +--> 'test_tainted_MY_ASSERT_1': event 2 (depth 1)
+ |
+ | test_tainted_MY_ASSERT_1 (int n)
+ | ^~~~~~~~~~~~~~~~~~~~~~~~
+ | |
+ | (2) entry to 'test_tainted_MY_ASSERT_1'
+ |
+ 'test_tainted_MY_ASSERT_1': event 3 (depth 1)
+ |
+ | do { if (!(EXPR)) my_assert_fail (#EXPR, __FILE__, __LINE__); } while (0)
+ | ^
+ | |
+ | (3) use of attacker-controlled value for control flow
+ { dg-end-multiline-output "" } */
+// note: in expansion of macro 'MY_ASSERT_1'
+/* { dg-begin-multiline-output "" }
+ | MY_ASSERT_1 (n > 0);
+ | ^~~~~~~~~~~
+ |
+ 'test_tainted_MY_ASSERT_1': event 4 (depth 1)
+ |
+ | do { if (!(EXPR)) my_assert_fail (#EXPR, __FILE__, __LINE__); } while (0)
+ | ^
+ | |
+ | (4) following 'true' branch (when 'n <= 0')...
+ { dg-end-multiline-output "" } */
+// note: in expansion of macro 'MY_ASSERT_1'
+/* { dg-begin-multiline-output "" }
+ | MY_ASSERT_1 (n > 0);
+ | ^~~~~~~~~~~
+ |
+ 'test_tainted_MY_ASSERT_1': event 5 (depth 1)
+ |
+ | do { if (!(EXPR)) my_assert_fail (#EXPR, __FILE__, __LINE__); } while (0)
+ | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ | |
+ | (5) ...to here
+ { dg-end-multiline-output "" } */
+// note: in expansion of macro 'MY_ASSERT_1'
+/* { dg-begin-multiline-output "" }
+ | MY_ASSERT_1 (n > 0);
+ | ^~~~~~~~~~~
+ |
+ 'test_tainted_MY_ASSERT_1': event 6 (depth 1)
+ |
+ | do { if (!(EXPR)) my_assert_fail (#EXPR, __FILE__, __LINE__); } while (0)
+ | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ | |
+ | (6) treating 'my_assert_fail' as an assertion failure handler due to '__attribute__((__noreturn__))'
+ { dg-end-multiline-output "" } */
+// note: in expansion of macro 'MY_ASSERT_1'
+/* { dg-begin-multiline-output "" }
+ | MY_ASSERT_1 (n > 0);
+ | ^~~~~~~~~~~
+ |
+ { dg-end-multiline-output "" } */
--git a/gcc/testsuite/gcc.dg/analyzer/taint-assert-system-header.c b/gcc/testsuite/gcc.dg/analyzer/taint-assert-system-header.c
new file mode 100644
index 00000000000..a65853c7886
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/taint-assert-system-header.c
@@ -0,0 +1,52 @@
+/* Integration test of how the execution path looks for
+ -Wanalyzer-tainted-assertion with macro-tracking enabled
+ (the default), where the assertion macro is defined in
+ a system header. */
+
+// TODO: remove need for this option
+/* { dg-additional-options "-fanalyzer-checker=taint" } */
+
+/* { dg-additional-options "-fdiagnostics-show-path-depths" } */
+/* { dg-additional-options "-fdiagnostics-path-format=inline-events -fdiagnostics-show-caret" } */
+
+/* An assertion macro that has a call to a __noreturn__ function. */
+
+/* This is marked as a system header. */
+#include "test-assert.h"
+
+int __attribute__((tainted_args))
+test_tainted_assert (int n)
+{
+ assert (n > 0); /* { dg-warning "use of attacked-controlled value in condition for assertion \\\[CWE-617\\\] \\\[-Wanalyzer-tainted-assertion\\\]" } */
+ return n * n;
+}
+
+/* { dg-begin-multiline-output "" }
+ assert (n > 0);
+ ^~~~~~
+ 'test_tainted_assert': event 1 (depth 0)
+ |
+ | test_tainted_assert (int n)
+ | ^~~~~~~~~~~~~~~~~~~
+ | |
+ | (1) function 'test_tainted_assert' marked with '__attribute__((tainted_args))'
+ |
+ +--> 'test_tainted_assert': event 2 (depth 1)
+ |
+ | test_tainted_assert (int n)
+ | ^~~~~~~~~~~~~~~~~~~
+ | |
+ | (2) entry to 'test_tainted_assert'
+ |
+ 'test_tainted_assert': events 3-6 (depth 1)
+ |
+ |
+ | do { if (!(EXPR)) __assert_fail (#EXPR, __FILE__, __LINE__); } while (0)
+ | ^ ~~~~~~~~~~~~~
+ | | |
+ | | (5) ...to here
+ | | (6) treating '__assert_fail' as an assertion failure handler due to '__attribute__((__noreturn__))'
+ | (3) use of attacker-controlled value for control flow
+ | (4) following 'true' branch (when 'n <= 0')...
+ |
+ { dg-end-multiline-output "" } */
diff --git a/gcc/testsuite/gcc.dg/analyzer/taint-assert.c b/gcc/testsuite/gcc.dg/analyzer/taint-assert.c
new file mode 100644
index 00000000000..b09f8c51a16
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/taint-assert.c
@@ -0,0 +1,346 @@
+// TODO: remove need for this option
+/* { dg-additional-options "-fanalyzer-checker=taint" } */
+
+/* We need this, otherwise the warnings are emitted inside the macros, which
+ makes it hard to write the DejaGnu directives. */
+/* { dg-additional-options " -ftrack-macro-expansion=0" } */
+
+#include "analyzer-decls.h"
+
+/* An assertion macro that has a call to a __noreturn__ function. */
+
+extern void my_assert_fail (const char *expr, const char *file, int line)
+ __attribute__ ((__noreturn__));
+
+#define MY_ASSERT_1(EXPR) \
+ do { if (!(EXPR)) my_assert_fail (#EXPR, __FILE__, __LINE__); } while (0)
+
+int
+test_not_tainted_MY_ASSERT_1 (int n)
+{
+ MY_ASSERT_1 (n > 0); /* { dg-bogus "-Wanalyzer-tainted-assertion" } */
+ return n * n;
+}
+
+int __attribute__((tainted_args))
+test_tainted_MY_ASSERT_1 (int n)
+{
+ MY_ASSERT_1 (n > 0); /* { dg-warning "use of attacked-controlled value in condition for assertion \\\[CWE-617\\\] \\\[-Wanalyzer-tainted-assertion\\\]" "warning" } */
+ /* { dg-message "treating 'my_assert_fail' as an assertion failure handler due to '__attribute__\\(\\(__noreturn__\\)\\)'" "final event" { target *-*-* } .-1 } */
+ return n * n;
+}
+
+
+/* An assertion macro that has a call to __builtin_unreachable. */
+
+#define MY_ASSERT_2(EXPR) \
+ do { if (!(EXPR)) __builtin_unreachable (); } while (0)
+
+int
+test_not_tainted_MY_ASSERT_2 (int n)
+{
+ MY_ASSERT_2 (n > 0); /* { dg-bogus "-Wanalyzer-tainted-assertion" } */
+ return n * n;
+}
+
+int __attribute__((tainted_args))
+test_tainted_MY_ASSERT_2 (int n)
+{
+ MY_ASSERT_2 (n > 0); /* { dg-warning "-Wanalyzer-tainted-assertion" "warning" } */
+ /* { dg-message "treating '__builtin_unreachable' as an assertion failure handler" "final event" { target *-*-* } .-1 } */
+ return n * n;
+}
+
+
+/* An assertion macro that's preprocessed away.
+ The analyzer doesn't see this, so can't warn. */
+
+#define MY_ASSERT_3(EXPR) do { } while (0)
+
+int
+test_not_tainted_MY_ASSERT_3 (int n)
+{
+ MY_ASSERT_3 (n > 0); /* { dg-bogus "-Wanalyzer-tainted-assertion" } */
+ return n * n;
+}
+
+int __attribute__((tainted_args))
+test_tainted_MY_ASSERT_3 (int n)
+{
+ MY_ASSERT_3 (n > 0); /* { dg-bogus "-Wanalyzer-tainted-assertion" } */
+ return n * n;
+}
+
+
+/* A macro that isn't an assertion. */
+
+extern void do_something_benign ();
+
+#define NOT_AN_ASSERT(EXPR) \
+ do { if (!(EXPR)) do_something_benign (); } while (0)
+
+int
+test_not_tainted_NOT_AN_ASSERT (int n)
+{
+ NOT_AN_ASSERT (n > 0); /* { dg-bogus "-Wanalyzer-tainted-assertion" } */
+ return n * n;
+}
+
+int __attribute__((tainted_args))
+test_tainted_NOT_AN_ASSERT (int n)
+{
+ NOT_AN_ASSERT (n > 0); /* { dg-bogus "-Wanalyzer-tainted-assertion" } */
+ return n * n;
+}
+
+
+/* A condition that isn't an assertion. */
+
+int __attribute__((tainted_args))
+test_tainted_condition (int n)
+{
+ if (n > 0) /* { dg-bogus "-Wanalyzer-tainted-assertion" } */
+ return 1;
+ else
+ return -1;
+}
+
+
+/* More complicated expressions in assertions. */
+
+int g;
+
+void __attribute__((tainted_args))
+test_compound_condition_in_assert_1 (int n)
+{
+ MY_ASSERT_1 ((n * 2) < (g + 3)); /* { dg-warning "-Wanalyzer-tainted-assertion" } */
+}
+
+void __attribute__((tainted_args))
+test_compound_condition_in_assert_2 (int x, int y)
+{
+ MY_ASSERT_1 (x < 100 && y < 100); /* { dg-warning "-Wanalyzer-tainted-assertion" } */
+}
+
+void __attribute__((tainted_args))
+test_compound_condition_in_assert_3 (int x, int y)
+{
+ MY_ASSERT_1 (x < 100 || y < 100); /* { dg-warning "-Wanalyzer-tainted-assertion" } */
+}
+
+void __attribute__((tainted_args))
+test_sanitized_expression_in_assert (int n)
+{
+ __analyzer_dump_state ("taint", n); /* { dg-warning "state: 'tainted'" } */
+ if (n < 0 || n >= 100)
+ return;
+ __analyzer_dump_state ("taint", n); /* { dg-warning "state: 'stop'" } */
+ MY_ASSERT_1 (n < 200); /* { dg-bogus "-Wanalyzer-tainted-assertion" } */
+}
+
+void __attribute__((tainted_args))
+test_sanitization_then_ok_assertion (unsigned n)
+{
+ if (n >= 100)
+ return;
+
+ /* Shouldn't warn here, as g isn't attacker-controlled. */
+ MY_ASSERT_1 (g > 42); /* { dg-bogus "-Wanalyzer-tainted-assertion" } */
+}
+
+void __attribute__((tainted_args))
+test_good_assert_then_bad_assert (unsigned n)
+{
+ /* Shouldn't warn here, as g isn't attacker-controlled. */
+ MY_ASSERT_1 (g > 42); /* { dg-bogus "-Wanalyzer-tainted-assertion" } */
+
+ /* ...but n is: */
+ MY_ASSERT_1 (n < 100); /* { dg-warning "-Wanalyzer-tainted-assertion" } */
+}
+
+void __attribute__((tainted_args))
+test_bad_assert_then_good_assert (unsigned n)
+{
+ MY_ASSERT_1 (n < 100); /* { dg-warning "-Wanalyzer-tainted-assertion" } */
+ MY_ASSERT_1 (g > 42); /* { dg-bogus "-Wanalyzer-tainted-assertion" } */
+}
+
+
+/* */
+
+void __attribute__((tainted_args))
+test_zero_MY_ASSERT_1 (unsigned n)
+{
+ if (n >= 100)
+ MY_ASSERT_1 (0); /* { dg-warning "-Wanalyzer-tainted-assertion" } */
+}
+
+void __attribute__((tainted_args))
+test_nonzero_MY_ASSERT_1 (unsigned n)
+{
+ if (n >= 100)
+ MY_ASSERT_1 (1); /* { dg-bogus "-Wanalyzer-tainted-assertion" } */
+}
+
+void __attribute__((tainted_args))
+test_zero_MY_ASSERT_2 (unsigned n)
+{
+ if (n >= 100)
+ MY_ASSERT_2 (0); /* { dg-warning "-Wanalyzer-tainted-assertion" } */
+}
+
+void __attribute__((tainted_args))
+test_nonzero_MY_ASSERT_2 (unsigned n)
+{
+ if (n >= 100)
+ MY_ASSERT_2 (1); /* { dg-bogus "-Wanalyzer-tainted-assertion" } */
+}
+
+
+/* Assertions that call a subroutine to do validity checking. */
+
+static int
+__analyzer_valid_1 (int x)
+{
+ return x < 100;
+}
+
+void __attribute__((tainted_args))
+test_assert_calling_valid_1 (int n)
+{
+ MY_ASSERT_1 (__analyzer_valid_1 (n)); /* { dg-warning "-Wanalyzer-tainted-assertion" } */
+}
+
+static int
+__analyzer_valid_2 (int x)
+{
+ return x < 100;
+}
+
+void __attribute__((tainted_args))
+test_assert_calling_valid_2 (int n)
+{
+ MY_ASSERT_1 (__analyzer_valid_2 (n)); /* { dg-warning "-Wanalyzer-tainted-assertion" } */
+}
+
+static int
+__analyzer_valid_3 (int x, int y)
+{
+ if (x >= 100)
+ return 0;
+ if (y >= 100)
+ return 0;
+ return 1;
+}
+
+void __attribute__((tainted_args))
+test_assert_calling_valid_3 (int a, int b)
+{
+ MY_ASSERT_1 (__analyzer_valid_3 (a, b)); /* { dg-warning "-Wanalyzer-tainted-assertion" "TODO" { xfail *-*-* } } */
+}
+
+
+/* 'switch' statements with supposedly unreachable cases/defaults. */
+
+int __attribute__((tainted_args))
+test_switch_default (int n)
+{
+ switch (n) /* { dg-message "use of attacker-controlled value for control flow" "why" } */
+ /* { dg-message "following 'default:' branch" "dest" { target *-*-* } .-1 } */
+ {
+ case 0:
+ return 5;
+ case 1:
+ return 22;
+ case 2:
+ return -1;
+ default:
+ /* The wording is rather inaccurate here. */
+ __builtin_unreachable (); /* { dg-warning "use of attacked-controlled value in condition for assertion" } */
+ }
+}
+
+int __attribute__((tainted_args))
+test_switch_unhandled_case (int n)
+{
+ switch (n) /* { dg-message "use of attacker-controlled value for control flow" "why" } */
+ /* { dg-message "following 'default:' branch" "dest" { target *-*-* } .-1 } */
+ {
+ case 0:
+ return 5;
+ case 1:
+ return 22;
+ case 2:
+ return -1;
+ }
+
+ /* The wording is rather inaccurate here. */
+ __builtin_unreachable (); /* { dg-warning "use of attacked-controlled value in condition for assertion" } */
+}
+
+int __attribute__((tainted_args))
+test_switch_bogus_case_MY_ASSERT_1 (int n)
+{
+ switch (n)
+ {
+ default:
+ case 0:
+ return 5;
+ case 1:
+ return 22;
+ case 2:
+ return -1;
+ case 42:
+ MY_ASSERT_1 (0); /* { dg-warning "-Wanalyzer-tainted-assertion" } */
+ }
+}
+
+int __attribute__((tainted_args))
+test_switch_bogus_case_MY_ASSERT_2 (int n)
+{
+ switch (n)
+ {
+ default:
+ case 0:
+ return 5;
+ case 1:
+ return 22;
+ case 2:
+ return -1;
+ case 42:
+ MY_ASSERT_2 (0); /* { dg-warning "-Wanalyzer-tainted-assertion" } */
+ }
+}
+
+int __attribute__((tainted_args))
+test_switch_bogus_case_unreachable (int n)
+{
+ switch (n)
+ {
+ default:
+ case 0:
+ return 5;
+ case 1:
+ return 22;
+ case 2:
+ return -1;
+ case 42:
+ /* This case gets optimized away before we see it. */
+ __builtin_unreachable ();
+ }
+}
+
+
+/* Contents of a struct. */
+
+struct s
+{
+ int x;
+ int y;
+};
+
+int __attribute__((tainted_args))
+test_assert_struct (struct s *p)
+{
+ MY_ASSERT_1 (p->x < p->y); /* { dg-warning "-Wanalyzer-tainted-assertion" } */
+}
diff --git a/gcc/testsuite/gcc.dg/analyzer/test-assert.h b/gcc/testsuite/gcc.dg/analyzer/test-assert.h
new file mode 100644
index 00000000000..4a24b720ac8
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/test-assert.h
@@ -0,0 +1,7 @@
+#pragma GCC system_header
+
+extern void __assert_fail (const char *expr, const char *file, int line)
+ __attribute__ ((__noreturn__));
+
+#define assert(EXPR) \
+ do { if (!(EXPR)) __assert_fail (#EXPR, __FILE__, __LINE__); } while (0)
diff --git a/gcc/testsuite/gcc.dg/plugin/analyzer_gil_plugin.c b/gcc/testsuite/gcc.dg/plugin/analyzer_gil_plugin.c
index b72856bf6f6..e494315bb34 100644
--- a/gcc/testsuite/gcc.dg/plugin/analyzer_gil_plugin.c
+++ b/gcc/testsuite/gcc.dg/plugin/analyzer_gil_plugin.c
@@ -89,7 +89,8 @@ public:
return 0;
}
- location_t fixup_location (location_t loc) const final override
+ location_t fixup_location (location_t loc,
+ bool) const final override
{
/* Ideally we'd check for specific macros here, and only
resolve certain macros. */
--
2.26.3
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2022-11-13 23:03 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-13 23:03 [committed] analyzer: new warning: -Wanalyzer-tainted-assertion [PR106235] David Malcolm
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).