public inbox for gcc-cvs@sourceware.org help / color / mirror / Atom feed
From: David Malcolm <dmalcolm@gcc.gnu.org> To: gcc-cvs@gcc.gnu.org Subject: [gcc r12-8638] analyzer: fix false positives from -Wanalyzer-tainted-divisor [PR106225] Date: Wed, 27 Jul 2022 21:55:59 +0000 (GMT) [thread overview] Message-ID: <20220727215559.4ABA5385AE7F@sourceware.org> (raw) https://gcc.gnu.org/g:71a4f739c218746df70612eeb844024d1fe206bb commit r12-8638-g71a4f739c218746df70612eeb844024d1fe206bb Author: David Malcolm <dmalcolm@redhat.com> Date: Wed Jul 27 17:38:55 2022 -0400 analyzer: fix false positives from -Wanalyzer-tainted-divisor [PR106225] (cherry picked from r13-1562-g897b3b31f0a94b) gcc/analyzer/ChangeLog: PR analyzer/106225 * sm-taint.cc (taint_state_machine::on_stmt): Move handling of assignments from division to... (taint_state_machine::check_for_tainted_divisor): ...this new function. Reject warning when the divisor is known to be non-zero. * sm.cc: Include "analyzer/program-state.h". (sm_context::get_old_region_model): New. * sm.h (sm_context::get_old_region_model): New decl. gcc/testsuite/ChangeLog: PR analyzer/106225 * gcc.dg/analyzer/taint-divisor-1.c: Add test coverage for various correct and incorrect checks against zero. Signed-off-by: David Malcolm <dmalcolm@redhat.com> Diff: --- gcc/analyzer/sm-taint.cc | 51 ++++++++++++++----- gcc/analyzer/sm.cc | 12 +++++ gcc/analyzer/sm.h | 2 + gcc/testsuite/gcc.dg/analyzer/taint-divisor-1.c | 66 +++++++++++++++++++++++++ 4 files changed, 119 insertions(+), 12 deletions(-) diff --git a/gcc/analyzer/sm-taint.cc b/gcc/analyzer/sm-taint.cc index 17669ae7685..70da585f391 100644 --- a/gcc/analyzer/sm-taint.cc +++ b/gcc/analyzer/sm-taint.cc @@ -109,6 +109,9 @@ private: const supernode *node, const gcall *call, tree callee_fndecl) const; + void check_for_tainted_divisor (sm_context *sm_ctxt, + const supernode *node, + const gassign *assign) const; public: /* State for a "tainted" value: unsanitized data potentially under an @@ -792,18 +795,7 @@ taint_state_machine::on_stmt (sm_context *sm_ctxt, case ROUND_MOD_EXPR: case RDIV_EXPR: case EXACT_DIV_EXPR: - { - tree divisor = gimple_assign_rhs2 (assign);; - state_t state = sm_ctxt->get_state (stmt, divisor); - enum bounds b; - if (get_taint (state, TREE_TYPE (divisor), &b)) - { - tree diag_divisor = sm_ctxt->get_diagnostic_tree (divisor); - sm_ctxt->warn (node, stmt, divisor, - new tainted_divisor (*this, diag_divisor, b)); - sm_ctxt->set_next_state (stmt, divisor, m_stop); - } - } + check_for_tainted_divisor (sm_ctxt, node, assign); break; } } @@ -978,6 +970,41 @@ taint_state_machine::check_for_tainted_size_arg (sm_context *sm_ctxt, } } +/* Complain if ASSIGN (a division operation) has a tainted divisor + that could be zero. */ + +void +taint_state_machine::check_for_tainted_divisor (sm_context *sm_ctxt, + const supernode *node, + const gassign *assign) const +{ + const region_model *old_model = sm_ctxt->get_old_region_model (); + if (!old_model) + return; + + tree divisor_expr = gimple_assign_rhs2 (assign);; + const svalue *divisor_sval = old_model->get_rvalue (divisor_expr, NULL); + + state_t state = sm_ctxt->get_state (assign, divisor_sval); + enum bounds b; + if (get_taint (state, TREE_TYPE (divisor_expr), &b)) + { + const svalue *zero_sval + = old_model->get_manager ()->get_or_create_int_cst + (TREE_TYPE (divisor_expr), 0); + tristate ts + = old_model->eval_condition (divisor_sval, NE_EXPR, zero_sval); + if (ts.is_true ()) + /* The divisor is known to not equal 0: don't warn. */ + return; + + tree diag_divisor = sm_ctxt->get_diagnostic_tree (divisor_expr); + sm_ctxt->warn (node, assign, divisor_expr, + new tainted_divisor (*this, diag_divisor, b)); + sm_ctxt->set_next_state (assign, divisor_sval, m_stop); + } +} + } // anonymous namespace /* Internal interface to this file. */ diff --git a/gcc/analyzer/sm.cc b/gcc/analyzer/sm.cc index 515f86da86a..09ad68b2224 100644 --- a/gcc/analyzer/sm.cc +++ b/gcc/analyzer/sm.cc @@ -40,6 +40,7 @@ along with GCC; see the file COPYING3. If not see #include "analyzer/program-point.h" #include "analyzer/store.h" #include "analyzer/svalue.h" +#include "analyzer/program-state.h" #if ENABLE_ANALYZER @@ -159,6 +160,17 @@ state_machine::to_json () const return sm_obj; } +/* class sm_context. */ + +const region_model * +sm_context::get_old_region_model () const +{ + if (const program_state *old_state = get_old_program_state ()) + return old_state->m_region_model; + else + return NULL; +} + /* Create instances of the various state machines, each using LOGGER, and populate OUT with them. */ diff --git a/gcc/analyzer/sm.h b/gcc/analyzer/sm.h index 7ce1c73e217..e3e9a8d557b 100644 --- a/gcc/analyzer/sm.h +++ b/gcc/analyzer/sm.h @@ -278,6 +278,8 @@ public: const svalue *get_old_svalue (tree expr) const; + const region_model *get_old_region_model () const; + protected: sm_context (int sm_idx, const state_machine &sm) : m_sm_idx (sm_idx), m_sm (sm) {} diff --git a/gcc/testsuite/gcc.dg/analyzer/taint-divisor-1.c b/gcc/testsuite/gcc.dg/analyzer/taint-divisor-1.c index 5a5a0b93ce0..b7c1faef1c4 100644 --- a/gcc/testsuite/gcc.dg/analyzer/taint-divisor-1.c +++ b/gcc/testsuite/gcc.dg/analyzer/taint-divisor-1.c @@ -24,3 +24,69 @@ int test_2 (FILE *f) fread (&s, sizeof (s), 1, f); return s.a % s.b; /* { dg-warning "use of attacker-controlled value 's\\.b' as divisor without checking for zero" } */ } + +/* We shouldn't complain if the divisor has been checked for zero. */ + +int test_checked_ne_zero (FILE *f) +{ + struct st1 s; + fread (&s, sizeof (s), 1, f); + if (s.b) + return s.a / s.b; /* { dg-bogus "divisor" } */ + else + return 0; +} + +int test_checked_gt_zero (FILE *f) +{ + struct st1 s; + fread (&s, sizeof (s), 1, f); + if (s.b > 0) + return s.a / s.b; /* { dg-bogus "divisor" } */ + else + return 0; +} + +int test_checked_lt_zero (FILE *f) +{ + struct st1 s; + fread (&s, sizeof (s), 1, f); + if (s.b < 0) + return s.a / s.b; /* { dg-bogus "divisor" } */ + else + return 0; +} + +/* We should complain if the check on the divisor still allows it to be + zero. */ + +int test_checked_ge_zero (FILE *f) +{ + struct st1 s; + fread (&s, sizeof (s), 1, f); + if (s.b >= 0) + return s.a / s.b; /* { dg-warning "use of attacker-controlled value 's\\.b' as divisor without checking for zero" } */ + else + return 0; +} + +int test_checked_le_zero (FILE *f) +{ + struct st1 s; + fread (&s, sizeof (s), 1, f); + if (s.b <= 0) + return s.a / s.b; /* { dg-warning "use of attacker-controlled value 's\\.b' as divisor without checking for zero" } */ + else + return 0; +} + +int test_checked_eq_zero (FILE *f) +{ + struct st1 s; + fread (&s, sizeof (s), 1, f); + /* Wrong sense of test. */ + if (s.b != 0) + return 0; + else + return s.a / s.b; /* { dg-warning "use of attacker-controlled value 's\\.b' as divisor without checking for zero" } */ +}
reply other threads:[~2022-07-27 21:55 UTC|newest] Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20220727215559.4ABA5385AE7F@sourceware.org \ --to=dmalcolm@gcc.gnu.org \ --cc=gcc-cvs@gcc.gnu.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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).