[Bug analyzer/106229] New: False positives from -Wanalyzer-tainted-array-index with unsigned char index

[Bug analyzer/106229] New: False positives from -Wanalyzer-tainted-array-index with unsigned char index

[dmalcolm at gcc dot gnu.org]

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

            Bug ID: 106229
           Summary: False positives from -Wanalyzer-tainted-array-index
                    with unsigned char index
           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: ---

Consider:

struct s_12
{
  unsigned char idx;
  char buf[256];
};

char __attribute__((tainted_args))
test_12(struct s_12 s)
{
  return s.buf[s.idx];
}

Currently with trunk and gcc 12 this gives:

<source>: In function 'test_12':
<source>:10:15: warning: use of attacker-controlled value 's.idx' in array
lookup without bounds checking [CWE-129] [-Wanalyzer-tainted-array-index]
   10 |   return s.buf[s.idx];
      |          ~~~~~^~~~~~~
  'test_12': event 1
    |
    |    8 | test_12(struct s_12 s)
    |      | ^~~~~~~
    |      | |
    |      | (1) function 'test_12' marked with '__attribute__((tainted_args))'
    |
    +--> 'test_12': events 2-3
           |
           |    8 | test_12(struct s_12 s)
           |      | ^~~~~~~
           |      | |
           |      | (2) entry to 'test_12'
           |    9 | {
           |   10 |   return s.buf[s.idx];
           |      |          ~~~~~~~~~~~~
           |      |               |
           |      |               (3) use of attacker-controlled value 's.idx'
in array lookup without bounds checking

https://godbolt.org/z/ozhWdb78G

However, given that s.idx is unsigned char, it must be within the valid range,
and so the warning is unhelpful.

See on Linux kernel in drivers/tty/vt/keyboard.c where ioctls use a
user-supplied index to access the key_maps array:

include/linux/keyboard.h:extern unsigned short *key_maps[MAX_NR_KEYMAPS];
include/uapi/linux/keyboard.h:#define MAX_NR_KEYMAPS    256

but the index is unsigned char, so must be within range.

[Bug analyzer/106229] False positives from -Wanalyzer-tainted-array-index with unsigned char index

[cvs-commit at gcc dot gnu.org]

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

--- Comment #1 from GCC 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:ce27b66d952127b7abd0f8cceacb79eb6ecf71db

commit r14-7266-gce27b66d952127b7abd0f8cceacb79eb6ecf71db
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Mon Jan 15 19:01:21 2024 -0500

    analyzer: fix false +ves from -Wanalyzer-tainted-array-index with unsigned
char index [PR106229]

    gcc/analyzer/ChangeLog:
            PR analyzer/106229
            * analyzer.h (compare_constants): New decl.
            * constraint-manager.cc (compare_constants): Make non-static.
            * sm-taint.cc: Add include "fold-const.h".
            (class concrete_range): New.
            (get_possible_range): New.
            (index_can_be_out_of_bounds_p): New.
            (region_model::check_region_for_taint): Reject
            -Wanalyzer-tainted-array-index if the type of the value makes it
            impossible for it to be out-of-bounds of the array.

    gcc/testsuite/ChangeLog:
            PR analyzer/106229
            * c-c++-common/analyzer/taint-index-pr106229.c: New test.

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

[Bug analyzer/106229] False positives from -Wanalyzer-tainted-array-index with unsigned char index

[dmalcolm at gcc dot gnu.org]

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

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

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

--- Comment #2 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Should be fixed on trunk for GCC 14 by the above patch.

Given that taint checking requires explicit opt-in in earlier releases, I don't
plan to backport this.

Marking this as resolved.

[Bug analyzer/106229] False positives from -Wanalyzer-tainted-array-index with unsigned char index

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |14.0
           Keywords|                            |diagnostic