[Bug analyzer/106284] False positives from -Wanalyzer-tainted-array-index with optimized conditionals

2022-07-19 Thread cvs-commit at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106284

--- Comment #4 from CVS Commits  ---
The master branch has been updated by David Malcolm :

https://gcc.gnu.org/g:434d521d118fc7e7759b2b42bdddfa70caec637b

commit r13-1747-g434d521d118fc7e7759b2b42bdddfa70caec637b
Author: David Malcolm 
Date:   Tue Jul 19 09:53:39 2022 -0400

analyzer: log out-edge description in exploded_graph::process_node

I found this logging tweak very helpful when working on
PR analyzer/106284.

gcc/analyzer/ChangeLog:
* engine.cc (exploded_graph::process_node): Show any description
of the out-edge when logging it for consideration.

Signed-off-by: David Malcolm 

[Bug analyzer/106284] False positives from -Wanalyzer-tainted-array-index with optimized conditionals

2022-07-15 Thread dmalcolm at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106284

David Malcolm  changed:

   What|Removed |Added

 Status|ASSIGNED|RESOLVED
 Resolution|--- |FIXED

--- Comment #3 from David Malcolm  ---
Should be fixed on trunk for gcc 13 by the above patch.

[Bug analyzer/106284] False positives from -Wanalyzer-tainted-array-index with optimized conditionals

2022-07-15 Thread cvs-commit at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106284

--- Comment #2 from CVS Commits  ---
The master branch has been updated by David Malcolm :

https://gcc.gnu.org/g:0a8edfbd37d399d1103d86e134ba0a92f8c873c3

commit r13-1713-g0a8edfbd37d399d1103d86e134ba0a92f8c873c3
Author: David Malcolm 
Date:   Fri Jul 15 11:28:34 2022 -0400

analyzer: fix taint false positive on optimized range checks [PR106284]

PR analyzer/106284 reports a false positive from
-Wanalyzer-tainted-array-index seen on the Linux kernel
with a version of my patches from:
  https://gcc.gnu.org/pipermail/gcc-patches/2021-November/584372.html
in drivers/usb/class/usblp.c in function âusblp_set_protocolâ handling
usblp_ioctl on IOCNR_SET_PROTOCOL, which has:

  | 1337 | if (protocol < USBLP_FIRST_PROTOCOL || protocol >
USBLP_LAST_PROTOCOL)
  |  |~
  |  ||
  |  |(15) following âfalseâ branch...
  |..
  | 1341 | if (usblp->intf->num_altsetting > 1) {
  |  |
  |  || |
  |  || (16) ...to here
  |  |(17) following âtrueâ branch...
  | 1342 | alts = usblp->protocol[protocol].alt_setting;
  |  | 
  |  |  |
  |  |  (18) ...to here
  |  |  (19) use of attacker-controlled value
âargâ in array lookup without bounds checking

where "arg" is "protocol" (albeit from the caller frame, the ioctl
callback), and is clearly checked at (15).

The root cause is that at -O1 and above fold-const's build_range-check
can optimize range checks
  (c>=low) && (c<=high)
into
  (c-low>=0) && (c-low<=high-low)
and thus into a single check:
  (unsigned)(c - low) <= (unsigned)(high-low).

I initially attempted to fix this by detecting such conditions in
region_model::on_condition, and calling on_condition for both of the
implied conditions.  This turned out not to work since the current
sm_context framework doesn't support applying two conditions
simultaneously: it led to a transition from the old state to has_lb,
then a transition from the old state *again* to has_ub, thus leaving
the new state as has_ub, rather than the stop state.

Instead, this patch fixes things by special-casing it within
taint_state_machine::on_condition.

gcc/analyzer/ChangeLog:
PR analyzer/106284
* sm-taint.cc (taint_state_machine::on_condition): Handle range
checks optimized by build_range_check.

gcc/testsuite/ChangeLog:
PR analyzer/106284
* gcc.dg/analyzer/torture/taint-read-index-2.c: New test.

Signed-off-by: David Malcolm 

[Bug analyzer/106284] False positives from -Wanalyzer-tainted-array-index with optimized conditionals

2022-07-14 Thread dmalcolm at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106284

David Malcolm  changed:

   What|Removed |Added

 Ever confirmed|0   |1
 Status|UNCONFIRMED |ASSIGNED
   Last reconfirmed||2022-07-14

--- Comment #1 from David Malcolm  ---
I'm testing a fix for this.