Bug 85484 - missing -Wstringop-overflow for strcpy with a string of non-const length
Summary: missing -Wstringop-overflow for strcpy with a string of non-const length
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 8.0
: P3 normal
Target Milestone: 10.0
Assignee: Martin Sebor
URL:
Keywords: diagnostic
Depends on:
Blocks: strlen Wstringop-overflow
  Show dependency treegraph
 
Reported: 2018-04-20 15:48 UTC by Martin Sebor
Modified: 2023-12-30 20:59 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work: 10.0
Known to fail: 8.3.0, 9.2.0
Last reconfirmed: 2019-11-08 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Sebor 2018-04-20 15:48:26 UTC
The buffer overflow reported in bug 85476 can be reduced to the test case below.    The overflow is quite obvious and should be easy to detect at compile-time via the -Wstringop-overflow warning yet GCC does not detect it, either with or without -D_FORTIFY_SOURCE.  That's because __builtin_object_size that -Wstringop-overflow relies on only deals with constant sizes.  But the non-constant string length and allocation size are both readily available in the tree-ssa-strlen pass and so the bug can easily be detected there.

$ cat x.c && gcc -O2 -S -Wall -Wextra -Wpedantic -fdump-tree-strlen=/dev/stdout x.c
void f (char*);

void g (const char *s)
{
  unsigned n = __builtin_strlen (s);
  char *d = __builtin_alloca (n);   // off-by-one error (should be n + 1)
  __builtin_strcpy (d, s);          // missing -Wstringop-overflow
  f (d);
}

void h (const char *s)
{
  unsigned n = __builtin_strlen (s);
  char *d = __builtin_alloca (n);
  __builtin___strcpy_chk (d, s, __builtin_object_size (d, 1));
  f (d);
}


;; Function g (g, funcdef_no=0, decl_uid=1960, cgraph_uid=0, symbol_order=0)

g (const char * s)
{
  char * d;
  long unsigned int _1;
  long unsigned int _8;
  long unsigned int _10;

  <bb 2> [local count: 1073741825]:
  _1 = __builtin_strlen (s_3(D));
  _8 = _1 & 4294967295;
  d_5 = __builtin_alloca (_8);
  _10 = _1 + 1;
  __builtin_memcpy (d_5, s_3(D), _10);
  f (d_5);
  return;

}



;; Function h (h, funcdef_no=1, decl_uid=1965, cgraph_uid=1, symbol_order=1)

h (const char * s)
{
  char * d;
  long unsigned int _1;
  long unsigned int _2;
  long unsigned int _9;

  <bb 2> [local count: 1073741825]:
  _1 = __builtin_strlen (s_4(D));
  _9 = _1 & 4294967295;
  d_6 = __builtin_alloca (_9);
  _2 = _1 + 1;
  __builtin_memcpy (d_6, s_4(D), _2);
  f (d_6);
  return;

}
Comment 1 Martin Sebor 2019-10-15 21:43:48 UTC
Still no progress in GCC 10.  The missing warning part is also being tracked in bug 91582.
Comment 2 Martin Sebor 2019-11-08 22:47:33 UTC
My WIP patch for pr91582 detects both of these bugs:

pr85484.c: In function ‘g’:
pr85484.c:7:3: warning: ‘__builtin_strcpy’ writing one too many bytes into a region of a size that depends on ‘strlen’ [-Wstringop-overflow=]
    7 |   __builtin_strcpy (d, s);          // missing -Wstringop-overflow
      |   ^~~~~~~~~~~~~~~~~~~~~~~
pr85484.c:6:13: note: at offset 0 to an object with size at most 4294967295 allocated by ‘__builtin_alloca’ here
    6 |   char *d = __builtin_alloca (n);   // off-by-one error (should be n + 1)
      |             ^~~~~~~~~~~~~~~~~~~~
pr85484.c: In function ‘h’:
pr85484.c:15:3: warning: ‘__builtin_strcpy’ writing one too many bytes into a region of a size that depends on ‘strlen’ [-Wstringop-overflow=]
   15 |   __builtin___strcpy_chk (d, s, __builtin_object_size (d, 1));
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
pr85484.c:14:13: note: at offset 0 to an object with size at most 4294967295 allocated by ‘__builtin_alloca’ here
   14 |   char *d = __builtin_alloca (n);
      |             ^~~~~~~~~~~~~~~~~~~~
Comment 3 Martin Sebor 2020-04-22 21:52:56 UTC
Fixed for GCC 10 via r278983.