public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r12-7783] analyzer: use tainted_allocation_size::m_mem_space [PR105017]
@ 2022-03-23 12:37 David Malcolm
  0 siblings, 0 replies; only message in thread
From: David Malcolm @ 2022-03-23 12:37 UTC (permalink / raw)
  To: gcc-cvs

https://gcc.gnu.org/g:e6a3991ea15c0b14117b5693d77e15fd0477ce51

commit r12-7783-ge6a3991ea15c0b14117b5693d77e15fd0477ce51
Author: David Malcolm <dmalcolm@redhat.com>
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 <dmalcolm@redhat.com>

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);
     }


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2022-03-23 12:37 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-23 12:37 [gcc r12-7783] analyzer: use tainted_allocation_size::m_mem_space [PR105017] 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).