[Bug tree-optimization/104969] New: Likely a false positive of -D_FORTIFY_SOURCE=3

[Bug tree-optimization/104969] New: Likely a false positive of -D_FORTIFY_SOURCE=3

[marxin at gcc dot gnu.org]

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

            Bug ID: 104969
           Summary: Likely a false positive of -D_FORTIFY_SOURCE=3
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: marxin at gcc dot gnu.org
                CC: siddhesh at gcc dot gnu.org
  Target Milestone: ---

It's isolated from sratom package:

$ cat sratom.c
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>

int size = 3;
unsigned char data = 0xff;

int main()
{
    unsigned len = size * 2 + 1;
    char * str = __builtin_calloc(len, 1);

    for (uint32_t i = 0; i < size; ++i) {
      fprintf (stderr, "i=%i\n", i);
      snprintf((char*)str + (2 * i), len, "%02X", data);
    }

    fprintf (stderr, "R=%s\n", str);
}

$ gcc sratom.c -O2 -D_FORTIFY_SOURCE=3 && ./a.out
i=0
i=1
*** buffer overflow detected ***: terminated
Aborted (core dumped)

$ clang sratom.c -O2 -D_FORTIFY_SOURCE=3 && ./a.out
i=0
i=1
*** buffer overflow detected ***: terminated
Aborted (core dumped)

$ gcc-11 sratom.c -g -O2 -fsanitize=address,undefined && ./a.out 
i=0
i=1
i=2
R=FFFFFF

The original code is defective a bit as it wrongly assumes that
(char*)str + (2 * i) is at maximum 'len' big. It's actually len - (2 * i) big.
But it should be still valid code, am I right?

[Bug tree-optimization/104969] Likely a false positive of -D_FORTIFY_SOURCE=3

[marxin at gcc dot gnu.org]

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

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |12.0
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2022-03-17

[Bug tree-optimization/104969] Likely a false positive of -D_FORTIFY_SOURCE=3

[schwab@linux-m68k.org]

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

--- Comment #1 from Andreas Schwab <schwab@linux-m68k.org> ---
Passing a max len bigger than the available space is already an error.  The
whole point of snprintf is to never overflow no matter how large the output.

[Bug tree-optimization/104969] Likely a false positive of -D_FORTIFY_SOURCE=3

[siddhesh at gcc dot gnu.org]

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

--- Comment #2 from Siddhesh Poyarekar <siddhesh at gcc dot gnu.org> ---
(In reply to Martin Liška from comment #0)
> The original code is defective a bit as it wrongly assumes that
> (char*)str + (2 * i) is at maximum 'len' big. It's actually len - (2 * i)
> big. But it should be still valid code, am I right?

It doesn't overflow in this case, but specifying a length larger than the
actual buffer size is a standard violation.

"""
The snprintf() function shall be equivalent to sprintf(), with the addition of
the n argument which states the size of the buffer referred to by s. If n is
zero, nothing shall be written and s may be a null pointer. Otherwise, output
bytes beyond the n-1st shall be discarded instead of being written to the
array, and a null byte is written at the end of the bytes actually written into
the array.
"""

https://pubs.opengroup.org/onlinepubs/9699919799/functions/snprintf.html

[Bug tree-optimization/104969] Likely a false positive of -D_FORTIFY_SOURCE=3

[msebor at gcc dot gnu.org]

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

Martin Sebor <msebor at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |msebor at gcc dot gnu.org

--- Comment #3 from Martin Sebor <msebor at gcc dot gnu.org> ---
That's not the intended reading of the POSIX text.  But (outside of extensions
for behavior C leaves undefined) POSIX defers to C, so the authoritative text
is there.  C doesn't impose any requirement on the size argument.

That said, specifying a snprintf size that's bigger than the space in the
provided buffer is certainly asking for trouble, even more so than doing the
same with strncmp.  GCC should be enhanced to warn about that when possible
(pr83430 tracks the request), although I suspect that wouldn't help in this
case.

For the constant subset of instances Clang issues warning: 'snprintf' size
argument is too large; destination buffer has size 4, but size argument is 7
[-Wfortify-source].

[Bug tree-optimization/104969] Likely a false positive of -D_FORTIFY_SOURCE=3

[siddhesh at gcc dot gnu.org]

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

Siddhesh Poyarekar <siddhesh at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://sourceware.org/bugz
                   |                            |illa/show_bug.cgi?id=28989
         Resolution|---                         |INVALID
             Status|NEW                         |RESOLVED

--- Comment #4 from Siddhesh Poyarekar <siddhesh at gcc dot gnu.org> ---
Moving this to glibc, since gcc is working as expected.