[Bug analyzer/106225] New: False positives from -Wanalyzer-tainted-divisor

[Bug analyzer/106225] New: False positives from -Wanalyzer-tainted-divisor

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106225

            Bug ID: 106225
           Summary: False positives from -Wanalyzer-tainted-divisor
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: analyzer
          Assignee: dmalcolm at gcc dot gnu.org
          Reporter: dmalcolm at gcc dot gnu.org
  Target Milestone: ---

-Wanalyzer-tainted-divisor seems to be using the wrong logic for determining if
a value has been checked for zeroness; consider:

#include <stdio.h>

struct st1
{
  int a;
  int b;
};

int test_checked_ne_zero (FILE *f)
{
  struct st1 s;
  fread (&s, sizeof (s), 1, f);
  if (s.b)
    return s.a / s.b;
  else
    return 0;
}

for which (with -fanalyzer -fanalyzer-checker=taint) trunk and gcc 12.1
erroneously emit:

<source>: In function 'test_checked_ne_zero':
<source>:14:16: warning: use of attacker-controlled value 's.b' as divisor
without checking for zero [CWE-369] [-Wanalyzer-tainted-divisor]
   14 |     return s.a / s.b;
      |            ~~~~^~~~~
  'test_checked_ne_zero': events 1-3
    |
    |   13 |   if (s.b)
    |      |      ^
    |      |      |
    |      |      (1) following 'true' branch...
    |   14 |     return s.a / s.b;
    |      |            ~~~~~~~~~
    |      |             |  |
    |      |             |  (3) use of attacker-controlled value 's.b' as
divisor without checking for zero
    |      |             (2) ...to here
    |

despite the check for zero at line 13.

https://godbolt.org/z/KK4K8h9z3

Reduced from false positive seen on Linux kernel in drivers/tty/vt/vt_ioctl.c:
(function vt_resizex).

[Bug analyzer/106225] False positives from -Wanalyzer-tainted-divisor

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106225

David Malcolm <dmalcolm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2022-07-07
             Status|UNCONFIRMED                 |ASSIGNED
     Ever confirmed|0                           |1

--- Comment #1 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
I'm testing a fix for this.

[Bug analyzer/106225] False positives from -Wanalyzer-tainted-divisor

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106225

--- Comment #2 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by David Malcolm <dmalcolm@gcc.gnu.org>:

https://gcc.gnu.org/g:897b3b31f0a94b8bac59c6061655c6a32646d0a0

commit r13-1562-g897b3b31f0a94b8bac59c6061655c6a32646d0a0
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Thu Jul 7 15:50:26 2022 -0400

    analyzer: fix false positives from -Wanalyzer-tainted-divisor [PR106225]

    gcc/analyzer/ChangeLog:
            PR analyzer/106225
            * sm-taint.cc (taint_state_machine::on_stmt): Move handling of
            assignments from division to...
            (taint_state_machine::check_for_tainted_divisor): ...this new
            function.  Reject warning when the divisor is known to be non-zero.
            * sm.cc: Include "analyzer/program-state.h".
            (sm_context::get_old_region_model): New.
            * sm.h (sm_context::get_old_region_model): New decl.

    gcc/testsuite/ChangeLog:
            PR analyzer/106225
            * gcc.dg/analyzer/taint-divisor-1.c: Add test coverage for various
            correct and incorrect checks against zero.

    Signed-off-by: David Malcolm <dmalcolm@redhat.com>

[Bug analyzer/106225] False positives from -Wanalyzer-tainted-divisor

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106225

--- Comment #3 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Fixed on trunk for gcc 13 by the above commit.  Keeping this open to backport
to gcc 12.

[Bug analyzer/106225] False positives from -Wanalyzer-tainted-divisor

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106225

--- Comment #4 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-12 branch has been updated by David Malcolm
<dmalcolm@gcc.gnu.org>:

https://gcc.gnu.org/g:71a4f739c218746df70612eeb844024d1fe206bb

commit r12-8638-g71a4f739c218746df70612eeb844024d1fe206bb
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Wed Jul 27 17:38:55 2022 -0400

    analyzer: fix false positives from -Wanalyzer-tainted-divisor [PR106225]

    (cherry picked from r13-1562-g897b3b31f0a94b)

    gcc/analyzer/ChangeLog:
            PR analyzer/106225
            * sm-taint.cc (taint_state_machine::on_stmt): Move handling of
            assignments from division to...
            (taint_state_machine::check_for_tainted_divisor): ...this new
            function.  Reject warning when the divisor is known to be non-zero.
            * sm.cc: Include "analyzer/program-state.h".
            (sm_context::get_old_region_model): New.
            * sm.h (sm_context::get_old_region_model): New decl.

    gcc/testsuite/ChangeLog:
            PR analyzer/106225
            * gcc.dg/analyzer/taint-divisor-1.c: Add test coverage for various
            correct and incorrect checks against zero.

    Signed-off-by: David Malcolm <dmalcolm@redhat.com>

[Bug analyzer/106225] False positives from -Wanalyzer-tainted-divisor

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106225

David Malcolm <dmalcolm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

--- Comment #5 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Backported to gcc 12, so marking as resolved.