]> git.ipfire.org Git - thirdparty/gcc.git/commit - gcc/analyzer/sm.h
analyzer: fix taint handling of switch statements [PR106321]
authorDavid Malcolm <dmalcolm@redhat.com>
Tue, 19 Jul 2022 13:53:39 +0000 (09:53 -0400)
committerDavid Malcolm <dmalcolm@redhat.com>
Tue, 19 Jul 2022 13:53:39 +0000 (09:53 -0400)
commit2c044ff123ee573b7cd63d88f544091b7aeeb8f6
treeeaad74eba8c3f7f6d16675bcb3f73df3b0122f1f
parent434d521d118fc7e7759b2b42bdddfa70caec637b
analyzer: fix taint handling of switch statements [PR106321]

PR analyzer/106321 reports false positives from
-Wanalyzer-tainted-array-index on switch statements, seen e.g.
in the Linux kernel in drivers/vfio/pci/vfio_pci_core.c, where
vfio_pci_core_ioctl has:

    |  744 |                 switch (info.index) {
    |      |                 ~~~~~~  ~~~~~~~~~~
    |      |                 |           |
    |      |                 |           (8) ...to here
    |      |                 (9) following ‘case 0 ... 5:’ branch...
    |......
    |  751 |                 case VFIO_PCI_BAR0_REGION_INDEX ... VFIO_PCI_BAR5_REGION_INDEX:
    |      |                 ~~~~
    |      |                 |
    |      |                 (10) ...to here

and then a false complaint about "use of attacker-controlled value
‘info.index’ in array lookup without upper-bounds checking", where
info.index has clearly had its bounds checked by the switch/case.

It turns out that when I rewrote switch handling for the analyzer in
r12-3101-g8ca7fa84a3af35, I removed notifications to state machines
about the constraints on cases.

This patch fixes that oversight by adding a new on_bounded_ranges vfunc
for region_model_context, called on switch statement edges, which calls
a new state_machine vfunc.  It implements it for the "taint" state
machine, so that it updates the "has bounds" flags at out-edges for
switch statements, based on whether the bounds from the edge appear to
actually constrain the switch index.

gcc/analyzer/ChangeLog:
PR analyzer/106321
* constraint-manager.h (bounded_ranges::get_count): New.
(bounded_ranges::get_range): New.
* engine.cc (impl_region_model_context::on_bounded_ranges): New.
* exploded-graph.h (impl_region_model_context::on_bounded_ranges):
New decl.
* region-model.cc (region_model::apply_constraints_for_gswitch):
Potentially call ctxt->on_bounded_ranges.
* region-model.h (region_model_context::on_bounded_ranges): New
vfunc.
(noop_region_model_context::on_bounded_ranges): New.
(region_model_context_decorator::on_bounded_ranges): New.
* sm-taint.cc: Include "analyzer/constraint-manager.h".
(taint_state_machine::on_bounded_ranges): New.
* sm.h (state_machine::on_bounded_ranges): New.

gcc/testsuite/ChangeLog:
PR analyzer/106321
* gcc.dg/analyzer/torture/taint-read-index-2.c: Add test coverage
for switch statements.

Signed-off-by: David Malcolm <dmalcolm@redhat.com>
gcc/analyzer/constraint-manager.h
gcc/analyzer/engine.cc
gcc/analyzer/exploded-graph.h
gcc/analyzer/region-model.cc
gcc/analyzer/region-model.h
gcc/analyzer/sm-taint.cc
gcc/analyzer/sm.h
gcc/testsuite/gcc.dg/analyzer/torture/taint-read-index-2.c