From e3dda4854530a2d15ff30ac8efcdb3800cd3e5ff Mon Sep 17 00:00:00 2001 From: David Malcolm Date: Wed, 14 Aug 2019 10:45:51 -0400 Subject: [PATCH 076/124] FIXME: improvements to sm-taint.cc --- gcc/analyzer/sm-taint.cc | 83 +++++++++++++-------------------- gcc/testsuite/gcc.dg/analyzer/taint-1.c | 4 +- 2 files changed, 34 insertions(+), 53 deletions(-) diff --git a/gcc/analyzer/sm-taint.cc b/gcc/analyzer/sm-taint.cc index 1cd1c4d..ee0038e 100644 --- a/gcc/analyzer/sm-taint.cc +++ b/gcc/analyzer/sm-taint.cc @@ -88,7 +88,6 @@ public: state_machine::state_t state) const FINAL OVERRIDE; bool can_purge_p (state_t s) const FINAL OVERRIDE; -private: state_t m_start; state_t m_tainted; state_t m_has_lb; @@ -102,7 +101,8 @@ class tainted_array_index : public pending_diagnostic_subclass { public: - tainted_array_index (tree arg) : m_arg (arg) {} + tainted_array_index (const taint_state_machine &sm, tree arg) + : m_sm (sm), m_arg (arg) {} const char *get_kind () const FINAL OVERRIDE { return "tainted_array_index"; } @@ -116,20 +116,39 @@ public: metadata m; m.add_cwe (129); warning_at (rich_loc, m, OPT_Wanalyzer_tainted_array_index, - "use of tainted value %qE in array lookup without bounds checking", + "use of tainted value %qE in array lookup" + " without bounds checking", m_arg); } + label_text describe_state_change (const evdesc::state_change &change) + FINAL OVERRIDE + { + if (change.m_new_state == m_sm.m_tainted) + return change.formatted_print ("%qE gets an unchecked value here", + change.m_expr); + return label_text (); + } + + label_text describe_final_event (const evdesc::final_event &ev) FINAL OVERRIDE + { + return ev.formatted_print ("use of tainted value %qE in array lookup" + " without bounds checking", + m_arg); + } + private: + const taint_state_machine &m_sm; tree m_arg; }; //////////////////////////////////////////////////////////////////////////// +/* FIXME. */ + taint_state_machine::taint_state_machine (logger *logger) : state_machine ("taint", logger) { - // TODO: parse all of this from a file: m_start = add_state ("start"); m_tainted = add_state ("tainted"); m_has_lb = add_state ("has_lb"); @@ -137,6 +156,8 @@ taint_state_machine::taint_state_machine (logger *logger) m_stop = add_state ("stop"); } +/* FIXME. */ + bool taint_state_machine::on_stmt (sm_context *sm_ctxt, const supernode *node, @@ -144,7 +165,6 @@ taint_state_machine::on_stmt (sm_context *sm_ctxt, { //tree fndecl = node->m_fun->decl; - // FIXME: do all of this from data if (const gcall *call = dyn_cast (stmt)) { if (is_named_call_p (call, "fread", 4)) @@ -165,21 +185,6 @@ taint_state_machine::on_stmt (sm_context *sm_ctxt, } } -#if 0 - /* FIXME: It's not clear if we need this, due to the SSA representation: - each assignment creates a new SSA_NAME. */ - if (tree lhs = is_zero_assignment (stmt)) - { - if (any_pointer_p (lhs)) - { - sm_ctxt->on_transition (node, stmt, lhs, m_start, m_null); - sm_ctxt->on_transition (node, stmt, lhs, m_unchecked, m_null); - sm_ctxt->on_transition (node, stmt, lhs, m_nonnull, m_null); - sm_ctxt->on_transition (node, stmt, lhs, m_free, m_null); - } - } -#endif - if (const gassign *assign = dyn_cast (stmt)) { //tree lhs = gimple_assign_lhs (assign); @@ -190,41 +195,11 @@ taint_state_machine::on_stmt (sm_context *sm_ctxt, tree arg = TREE_OPERAND (rhs1, 1); arg = sm_ctxt->get_readable_tree (arg); sm_ctxt->warn_for_state (node, stmt, arg, m_tainted, - new tainted_array_index (arg)); + new tainted_array_index (*this, arg)); sm_ctxt->on_transition (node, stmt, arg, m_tainted, m_stop); } } -#if 0 - // TODO: dereferences - for (unsigned i = 0; i < gimple_num_ops (stmt); i++) - { - tree op = gimple_op (stmt, i); -#if 0 - if (op) - inform (stmt->location, "op %i: %qE", i, op); - else - inform (stmt->location, "op %i: NULL_TREE", i); -#endif - if (!op) - continue; - if (TREE_CODE (op) == MEM_REF) - { - tree arg = TREE_OPERAND (op, 0); - sm_ctxt->warn_for_state (node, stmt, arg, m_unchecked, - "use of possibly-NULL %qE", arg); - sm_ctxt->on_transition (node, stmt, arg, m_unchecked, m_nonnull); - - sm_ctxt->warn_for_state (node, stmt, arg, m_null, - "use of NULL %qE", arg); - sm_ctxt->on_transition (node, stmt, arg, m_null, m_stop); - - sm_ctxt->warn_for_state (node, stmt, arg, m_free, - "use after free of %qE", arg); - sm_ctxt->on_transition (node, stmt, arg, m_free, m_stop); - } - } -#endif return false; } @@ -271,6 +246,8 @@ taint_state_machine::on_condition (sm_context *sm_ctxt, } } +/* FIXME. */ + void taint_state_machine::on_leak (sm_context *sm_ctxt ATTRIBUTE_UNUSED, const supernode *node ATTRIBUTE_UNUSED, @@ -282,12 +259,16 @@ taint_state_machine::on_leak (sm_context *sm_ctxt ATTRIBUTE_UNUSED, /* Empty. */ } +/* FIXME. */ + bool taint_state_machine::can_purge_p (state_t s ATTRIBUTE_UNUSED) const { return true; } +/* FIXME. */ + state_machine * make_taint_state_machine (logger *logger) { diff --git a/gcc/testsuite/gcc.dg/analyzer/taint-1.c b/gcc/testsuite/gcc.dg/analyzer/taint-1.c index 3859dd4..8ae38fe 100644 --- a/gcc/testsuite/gcc.dg/analyzer/taint-1.c +++ b/gcc/testsuite/gcc.dg/analyzer/taint-1.c @@ -12,14 +12,14 @@ char test_1(FILE *f) { struct foo tmp; - if (1 == fread(&tmp, sizeof(tmp), 1, f)) { /* { dg-message "\\(1\\) state of 'tmp': 'start' -> 'tainted'" } */ + if (1 == fread(&tmp, sizeof(tmp), 1, f)) { /* { dg-message "\\(1\\) 'tmp' gets an unchecked value here" } */ /* { dg-message "\\(2\\) following 'true' branch\\.\\.\\." "" { target *-*-* } .-1 } */ /* BUG: the following array lookup trusts that the input data's index is in the range 0 <= i < 256; otherwise it's accessing the stack */ return tmp.buf[tmp.i]; // { dg-warning "use of tainted value 'tmp.i' in array lookup without bounds checking" } */ /* { dg-message "23: \\(3\\) \\.\\.\\.to here" "" { target *-*-* } .-1 } */ /* { dg-message "23: \\(4\\) state of 'tmp.i': 'start' -> 'tainted'" "" { target *-*-* } .-2 } */ - /* { dg-message "\\(5\\) here \\('tmp.i' is in state 'tainted'\\)" "" { target *-*-* } .-3 } */ + /* { dg-message "\\(5\\) use of tainted value 'tmp.i' in array lookup without bounds checking" "" { target *-*-* } .-3 } */ // TOOD: better messages for state changes } -- 1.8.5.3