[gcc r13-1768] analyzer: update "tainted" state of RHS in comparisons [PR106373]

[gcc r13-1768] analyzer: update "tainted" state of RHS in comparisons [PR106373]

[David Malcolm]

https://gcc.gnu.org/g:5e830693dd335621940368b6d39b23afc2c98545

commit r13-1768-g5e830693dd335621940368b6d39b23afc2c98545
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Wed Jul 20 17:25:35 2022 -0400

    analyzer: update "tainted" state of RHS in comparisons [PR106373]
    
    Doing so fixes various false positives from
    -Wanalyzer-tainted-array-index at -O1 and above (e.g. seen on the
    Linux kernel)
    
    gcc/analyzer/ChangeLog:
            PR analyzer/106373
            * sm-taint.cc (taint_state_machine::on_condition): Potentially
            update the state of the RHS as well as the LHS.
    
    gcc/testsuite/ChangeLog:
            PR analyzer/106373
            * gcc.dg/analyzer/torture/taint-read-index-3.c: New test.
    
    Signed-off-by: David Malcolm <dmalcolm@redhat.com>

Diff:
---
 gcc/analyzer/sm-taint.cc                           | 18 ++++++--
 .../gcc.dg/analyzer/torture/taint-read-index-3.c   | 52 ++++++++++++++++++++++
 2 files changed, 67 insertions(+), 3 deletions(-)

diff --git a/gcc/analyzer/sm-taint.cc b/gcc/analyzer/sm-taint.cc
index 9cb78886c9f..0486c01aaca 100644
--- a/gcc/analyzer/sm-taint.cc
+++ b/gcc/analyzer/sm-taint.cc
@@ -830,13 +830,11 @@ taint_state_machine::on_condition (sm_context *sm_ctxt,
 				   const gimple *stmt,
 				   const svalue *lhs,
 				   enum tree_code op,
-				   const svalue *rhs ATTRIBUTE_UNUSED) const
+				   const svalue *rhs) const
 {
   if (stmt == NULL)
     return;
 
-  // TODO: this doesn't use the RHS; should we make it symmetric?
-
   // TODO
   switch (op)
     {
@@ -845,10 +843,17 @@ taint_state_machine::on_condition (sm_context *sm_ctxt,
     case GE_EXPR:
     case GT_EXPR:
       {
+	/* (LHS >= RHS) or (LHS > RHS)
+	   LHS gains a lower bound
+	   RHS gains an upper bound.  */
 	sm_ctxt->on_transition (node, stmt, lhs, m_tainted,
 				m_has_lb);
 	sm_ctxt->on_transition (node, stmt, lhs, m_has_ub,
 				m_stop);
+	sm_ctxt->on_transition (node, stmt, rhs, m_tainted,
+				m_has_ub);
+	sm_ctxt->on_transition (node, stmt, rhs, m_has_lb,
+				m_stop);
       }
       break;
     case LE_EXPR:
@@ -896,10 +901,17 @@ taint_state_machine::on_condition (sm_context *sm_ctxt,
 		  }
 	  }
 
+	/* (LHS <= RHS) or (LHS < RHS)
+	   LHS gains an upper bound
+	   RHS gains a lower bound.  */
 	sm_ctxt->on_transition (node, stmt, lhs, m_tainted,
 				m_has_ub);
 	sm_ctxt->on_transition (node, stmt, lhs, m_has_lb,
 				m_stop);
+	sm_ctxt->on_transition (node, stmt, rhs, m_tainted,
+				m_has_lb);
+	sm_ctxt->on_transition (node, stmt, rhs, m_has_ub,
+				m_stop);
       }
       break;
     default:
diff --git a/gcc/testsuite/gcc.dg/analyzer/torture/taint-read-index-3.c b/gcc/testsuite/gcc.dg/analyzer/torture/taint-read-index-3.c
new file mode 100644
index 00000000000..8eb6061a08b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/torture/taint-read-index-3.c
@@ -0,0 +1,52 @@
+// TODO: remove need for the taint option:
+/* { dg-additional-options "-fanalyzer-checker=taint" } */
+/* { dg-skip-if "" { *-*-* } { "-fno-fat-lto-objects" } { "" } } */
+
+struct raw_ep {
+  /* ...snip... */
+  int state;
+  /* ...snip... */
+};
+
+struct raw_dev {
+  /* ...snip... */
+  struct raw_ep eps[30];
+  int eps_num;
+  /* ...snip... */
+};
+
+int   __attribute__((tainted_args))
+simplified_raw_ioctl_ep_disable(struct raw_dev *dev, unsigned long value)
+{
+  int ret = 0, i = value;
+
+  if (i < 0 || i >= dev->eps_num) {
+    ret = -16;
+    goto out_unlock;
+  }
+  if (dev->eps[i].state == 0) { /* { dg-bogus "attacker-controlled" } */
+    ret = -22;
+    goto out_unlock;
+  }
+
+out_unlock:
+  return ret;
+}
+
+int   __attribute__((tainted_args))
+test_2(struct raw_dev *dev, int i)
+{
+  int ret = 0;
+
+  if (i < 0 || i >= dev->eps_num) {
+    ret = -16;
+    goto out_unlock;
+  }
+  if (dev->eps[i].state == 0) { /* { dg-bogus "attacker-controlled" } */
+    ret = -22;
+    goto out_unlock;
+  }
+
+out_unlock:
+  return ret;
+}