]> gcc.gnu.org Git - gcc.git/commit
analyzer: fix taint false positive on optimized range checks [PR106284]
authorDavid Malcolm <dmalcolm@redhat.com>
Fri, 15 Jul 2022 15:28:34 +0000 (11:28 -0400)
committerDavid Malcolm <dmalcolm@redhat.com>
Fri, 15 Jul 2022 15:28:34 +0000 (11:28 -0400)
commit0a8edfbd37d399d1103d86e134ba0a92f8c873c3
tree73b30ba0784edab6267d530d2bfbee244ce7dd11
parentb1d07b50d43e954b0c6a2b85079ce0fdf5e77ec5
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 <dmalcolm@redhat.com>
gcc/analyzer/sm-taint.cc
gcc/testsuite/gcc.dg/analyzer/torture/taint-read-index-2.c [new file with mode: 0644]
This page took 0.06715 seconds and 5 git commands to generate.