From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 2209) id 2ED0C3857018; Wed, 23 Mar 2022 12:37:57 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 2ED0C3857018 MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="utf-8" From: David Malcolm To: gcc-cvs@gcc.gnu.org Subject: [gcc r12-7783] analyzer: use tainted_allocation_size::m_mem_space [PR105017] X-Act-Checkin: gcc X-Git-Author: David Malcolm X-Git-Refname: refs/heads/master X-Git-Oldrev: 160b095fc9ded4eaa2bf4d49bd97319f4aabff0a X-Git-Newrev: e6a3991ea15c0b14117b5693d77e15fd0477ce51 Message-Id: <20220323123757.2ED0C3857018@sourceware.org> Date: Wed, 23 Mar 2022 12:37:57 +0000 (GMT) X-BeenThere: gcc-cvs@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-cvs mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 23 Mar 2022 12:37:57 -0000 https://gcc.gnu.org/g:e6a3991ea15c0b14117b5693d77e15fd0477ce51 commit r12-7783-ge6a3991ea15c0b14117b5693d77e15fd0477ce51 Author: David Malcolm Date: Wed Mar 23 08:37:06 2022 -0400 analyzer: use tainted_allocation_size::m_mem_space [PR105017] gcc/analyzer/ChangeLog: PR analyzer/105017 * sm-taint.cc (taint_diagnostic::subclass_equal_p): Check m_has_bounds as well as m_arg. (tainted_allocation_size::subclass_equal_p): Chain up to base class implementation. Also check m_mem_space. (tainted_allocation_size::emit): Add note showing stack-based vs heap-based allocations. gcc/testsuite/ChangeLog: PR analyzer/105017 * gcc.dg/analyzer/taint-alloc-1.c: Add expected messages relating to heap vs stack. Signed-off-by: David Malcolm Diff: --- gcc/analyzer/sm-taint.cc | 82 ++++++++++++++++++--------- gcc/testsuite/gcc.dg/analyzer/taint-alloc-1.c | 2 + 2 files changed, 58 insertions(+), 26 deletions(-) diff --git a/gcc/analyzer/sm-taint.cc b/gcc/analyzer/sm-taint.cc index e2c78cdd42b..17669ae7685 100644 --- a/gcc/analyzer/sm-taint.cc +++ b/gcc/analyzer/sm-taint.cc @@ -137,7 +137,9 @@ public: bool subclass_equal_p (const pending_diagnostic &base_other) const OVERRIDE { - return same_tree_p (m_arg, ((const taint_diagnostic &)base_other).m_arg); + const taint_diagnostic &other = (const taint_diagnostic &)base_other; + return (same_tree_p (m_arg, other.m_arg) + && m_has_bounds == other.m_has_bounds); } label_text describe_state_change (const evdesc::state_change &change) @@ -523,6 +525,15 @@ public: return "tainted_allocation_size"; } + bool subclass_equal_p (const pending_diagnostic &base_other) const OVERRIDE + { + if (!taint_diagnostic::subclass_equal_p (base_other)) + return false; + const tainted_allocation_size &other + = (const tainted_allocation_size &)base_other; + return m_mem_space == other.m_mem_space; + } + int get_controlling_option () const FINAL OVERRIDE { return OPT_Wanalyzer_tainted_allocation_size; @@ -533,29 +544,32 @@ public: diagnostic_metadata m; /* "CWE-789: Memory Allocation with Excessive Size Value". */ m.add_cwe (789); - // TODO: make use of m_mem_space + + bool warned; if (m_arg) switch (m_has_bounds) { default: gcc_unreachable (); case BOUNDS_NONE: - return warning_meta (rich_loc, m, get_controlling_option (), - "use of attacker-controlled value %qE as" - " allocation size without bounds checking", - m_arg); + warned = warning_meta (rich_loc, m, get_controlling_option (), + "use of attacker-controlled value %qE as" + " allocation size without bounds checking", + m_arg); break; case BOUNDS_UPPER: - return warning_meta (rich_loc, m, get_controlling_option (), - "use of attacker-controlled value %qE as" - " allocation size without lower-bounds checking", - m_arg); + warned = warning_meta (rich_loc, m, get_controlling_option (), + "use of attacker-controlled value %qE as" + " allocation size without" + " lower-bounds checking", + m_arg); break; case BOUNDS_LOWER: - return warning_meta (rich_loc, m, get_controlling_option (), - "use of attacker-controlled value %qE as" - " allocation size without upper-bounds checking", - m_arg); + warned = warning_meta (rich_loc, m, get_controlling_option (), + "use of attacker-controlled value %qE as" + " allocation size without" + " upper-bounds checking", + m_arg); break; } else @@ -564,24 +578,40 @@ public: default: gcc_unreachable (); case BOUNDS_NONE: - return warning_meta (rich_loc, m, get_controlling_option (), - "use of attacker-controlled value as" - " allocation size without bounds" - " checking"); + warned = warning_meta (rich_loc, m, get_controlling_option (), + "use of attacker-controlled value as" + " allocation size without bounds" + " checking"); break; case BOUNDS_UPPER: - return warning_meta (rich_loc, m, get_controlling_option (), - "use of attacker-controlled value as" - " allocation size without lower-bounds" - " checking"); + warned = warning_meta (rich_loc, m, get_controlling_option (), + "use of attacker-controlled value as" + " allocation size without" + " lower-bounds checking"); break; case BOUNDS_LOWER: - return warning_meta (rich_loc, m, get_controlling_option (), - "use of attacker-controlled value as" - " allocation size without upper-bounds" - " checking"); + warned = warning_meta (rich_loc, m, get_controlling_option (), + "use of attacker-controlled value as" + " allocation size without" + " upper-bounds checking"); break; } + if (warned) + { + location_t loc = rich_loc->get_loc (); + switch (m_mem_space) + { + default: + break; + case MEMSPACE_STACK: + inform (loc, "stack-based allocation"); + break; + case MEMSPACE_HEAP: + inform (loc, "heap-based allocation"); + break; + } + } + return warned; } label_text describe_final_event (const evdesc::final_event &ev) FINAL OVERRIDE diff --git a/gcc/testsuite/gcc.dg/analyzer/taint-alloc-1.c b/gcc/testsuite/gcc.dg/analyzer/taint-alloc-1.c index bc4f63bb3bd..cb2db6c69cf 100644 --- a/gcc/testsuite/gcc.dg/analyzer/taint-alloc-1.c +++ b/gcc/testsuite/gcc.dg/analyzer/taint-alloc-1.c @@ -25,6 +25,7 @@ void *test_1 (FILE *f) return malloc (tmp.sz); /* { dg-warning "use of attacker-controlled value 'tmp\\.sz' as allocation size without upper-bounds checking" "warning" } */ /* { dg-message "23: \\(\[0-9\]+\\) 'tmp.i' has an unchecked value here \\(from 'tmp'\\)" "event: tmp.i has an unchecked value" { xfail *-*-* } .-1 } */ /* { dg-message "\\(\[0-9\]+\\) use of attacker-controlled value 'tmp\\.sz' as allocation size without upper-bounds checking" "final event" { target *-*-* } .-2 } */ + /* { dg-message "heap-based allocation" "memory space" { target *-*-* } .-3 } */ // TOOD: better messages for state changes } @@ -46,6 +47,7 @@ void *test_2 (FILE *f) char buf[tmp.sz]; /* { dg-warning "use of attacker-controlled value 'tmp\\.sz' as allocation size without upper-bounds checking" "warning" } */ /* { dg-message "\\(\[0-9\]+\\) 'tmp.i' has an unchecked value here \\(from 'tmp'\\)" "event: tmp.i has an unchecked value" { xfail *-*-* } .-1 } */ /* { dg-message "\\(\[0-9\]+\\) use of attacker-controlled value 'tmp\\.sz' as allocation size without upper-bounds checking" "final event" { target *-*-* } .-2 } */ + /* { dg-message "stack-based allocation" "memory space" { target *-*-* } .-3 } */ fread (buf, tmp.sz, 1, f); }