Bug 78149 - missing warning on strncpy buffer overflow due to an excessive bound
Summary: missing warning on strncpy buffer overflow due to an excessive bound
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 7.0
: P3 normal
Target Milestone: ---
Assignee: Martin Sebor
URL:
Keywords: diagnostic, patch
Depends on:
Blocks:
 
Reported: 2016-10-28 18:43 UTC by Martin Sebor
Modified: 2016-12-08 00:22 UTC (History)
0 users

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2016-10-28 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Sebor 2016-10-28 18:43:56 UTC
GCC makes an effort to detect and diagnose with a "will always overflow destination buffer" warning calls to bounded checking functions such as strncpy whose bound is known and in excess of the size of the destination buffer.

But the warning is not issued when the bound whose exact value is not known is is in an invalid range (e.g., it's negative), or when the size of the destination object is not known but the bound is obviously too large to be meaningful (e.g., in excess of SIZE_MAX / 2).

Finally, no warning is issued for non-checking forms of the same functions.

In all these cases issuing a warning would be helpful in preventing the buffer overflow that inevitably occurs when the code is executed.

$ cat t.c && /home/msebor/build/gcc-git/gcc/xgcc -B/home/msebor/build/gcc-git/gcc -O2 -S -Wall -Wextra -Wpedantic t.c
char d [3];

char* strncpy (char*, const char*, __SIZE_TYPE__);

void __attribute__ ((noclone, noinline)) f (const char *s)
{
  strncpy (d, s, __SIZE_MAX__);
}

void __attribute__ ((noclone, noinline)) g0 (const char *s)
{
  __builtin___strncpy_chk (d, s, __SIZE_MAX__, __builtin_object_size (d, 1));
}

void __attribute__ ((noclone, noinline)) g1 (int max, const char *s)
{
  if (max > 0)
    max = -max;

  __builtin___strncpy_chk (d, s, max, __builtin_object_size (d, 1));
}

void __attribute__ ((noclone, noinline)) g2 (char *d, const char *s)
{
  __builtin___strncpy_chk (d, s, __SIZE_MAX__, __builtin_object_size (d, 1));
}
t.c: In function ‘g0’:
t.c:12:3: warning: call to __builtin___strncpy_chk will always overflow destination buffer
   __builtin___strncpy_chk (d, s, __SIZE_MAX__, __builtin_object_size (d, 1));
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Comment 1 Martin Sebor 2016-10-28 18:45:10 UTC
I'm working on this along with a patch for bug 78138.
Comment 2 Martin Sebor 2016-11-09 03:17:48 UTC
Patch posted for review:
https://gcc.gnu.org/ml/gcc-patches/2016-10/msg02308.html
Comment 3 Martin Sebor 2016-12-08 00:02:06 UTC
Author: msebor
Date: Thu Dec  8 00:01:33 2016
New Revision: 243419

URL: https://gcc.gnu.org/viewcvs?rev=243419&root=gcc&view=rev
Log:
PR c/53562 - Add -Werror= support for -D_FORTIFY_SOURCE / __builtin___memcpy_chk
PR middle-end/77784 - duplicate warning for snprintf when n > object size
PR middle-end/78149 - missing warning on strncpy buffer overflow due to an excessive bound
PR middle-end/78138 - missing warnings on buffer overflow with non-constant source length

gcc/c-family/ChangeLog:

	PR c/53562
	PR middle-end/77784
	PR middle-end/78149
	PR middle-end/78138
	* c.opt (-Wstringop-overflow): New option.

gcc/ChangeLog:

	PR middle-end/77784
	PR middle-end/78149
	PR middle-end/78138
	
	* builtins.c (expand_builtin_strcat, expand_builtin_strncat): New
	functions.
	(compute_dest_size, get_size_range, check_sizes, check_strncat_sizes)
	(check_memop_sizes): Same.
	(expand_builtin_memcpy): Call check memop_sizes.
	(expand_builtin_mempcpy): Same.
	(expand_builtin_memset): Same,
	(expand_builtin_bzero): Same.
	(expand_builtin_memory_chk): Call check_sizes.
	(expand_builtin_strcpy): Same.
	(expand_builtin_strncpy): Same.
	(maybe_emit_sprintf_chk_warning): Same.
	(expand_builtin): Handle strcat and strncat.
	(fini_object_sizes): Reset pointers.
	(compute_object_size): New function.
	* gimple-ssa-sprintf.c (pass_sprintf_length::handle_gimple_call):
	Avoid issuing warnings also issued during built-in expansion.
	* doc/invoke.texi (Warning Options): Document -Wstringop-overflow.

gcc/testsuite/ChangeLog:

	PR middle-end/77784
	PR middle-end/78149
	PR middle-end/78138

	* c-c++-common/Wsizeof-pointer-memaccess2.c: Adjust expected diagnostic.
	* g++.dg/ext/builtin-object-size3.C (bar): Same.
	* g++.dg/ext/strncpy-chk1.C: Same.
	* g++.dg/opt/memcpy1.C: Same.
	* g++.dg/torture/Wsizeof-pointer-memaccess1.C: Same.
	* gcc.c-torture/compile/pr55569.c: Disable -Wstringop-overflow.
	* gcc.dg/Wobjsize-1.c: Adjust expected diagnostic.
	* gcc.dg/attr-alloc_size.c: Same.
	* gcc.dg/builtin-stringop-chk-1.c: Adjust expected diagnostic.
	* gcc.dg/builtin-stringop-chk-2.c: Same.
	* gcc.dg/builtin-stringop-chk-4.c: New test.
	* gcc.dg/builtin-strncat-chk-1.c: Adjust expected diagnostic.
	* gcc.dg/memcpy-2.c: Same.
	* gcc.dg/pr40340-1.c: Same.
	* gcc.dg/pr40340-2.c (main): Same.
	* gcc.dg/pr40340-5.c (main): Same.
	* gcc.dg/torture/Wsizeof-pointer-memaccess1.c: Same.
	* gcc.dg/torture/pr71132.c: Disable -Wstringop-overflow.
	* gcc.dg/tree-ssa/builtin-sprintf-warn-1.c: Adjust text of expected
	warning.
	* gfortran.dg/char_length_3.f90: Prune expected warnings.
	* gfortran.dg/pr38868.f: Add expected warnings.


Added:
    trunk/gcc/testsuite/gcc.dg/builtin-stringop-chk-4.c
    trunk/gcc/testsuite/gcc.dg/builtin-stringop-chk-5.c
    trunk/gcc/testsuite/gcc.dg/builtin-stringop-chk-6.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/builtins.c
    trunk/gcc/c-family/ChangeLog
    trunk/gcc/c-family/c.opt
    trunk/gcc/doc/invoke.texi
    trunk/gcc/gimple-ssa-sprintf.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/c-c++-common/Wsizeof-pointer-memaccess2.c
    trunk/gcc/testsuite/g++.dg/ext/builtin-object-size3.C
    trunk/gcc/testsuite/g++.dg/ext/strncpy-chk1.C
    trunk/gcc/testsuite/g++.dg/opt/memcpy1.C
    trunk/gcc/testsuite/g++.dg/torture/Wsizeof-pointer-memaccess1.C
    trunk/gcc/testsuite/g++.dg/torture/Wsizeof-pointer-memaccess2.C
    trunk/gcc/testsuite/gcc.c-torture/compile/pr55569.c
    trunk/gcc/testsuite/gcc.dg/Wobjsize-1.c
    trunk/gcc/testsuite/gcc.dg/attr-alloc_size.c
    trunk/gcc/testsuite/gcc.dg/builtin-stringop-chk-1.c
    trunk/gcc/testsuite/gcc.dg/builtin-stringop-chk-2.c
    trunk/gcc/testsuite/gcc.dg/builtin-strncat-chk-1.c
    trunk/gcc/testsuite/gcc.dg/fstack-protector-strong.c
    trunk/gcc/testsuite/gcc.dg/memcpy-2.c
    trunk/gcc/testsuite/gcc.dg/pr40340-1.c
    trunk/gcc/testsuite/gcc.dg/pr40340-2.c
    trunk/gcc/testsuite/gcc.dg/pr40340-5.c
    trunk/gcc/testsuite/gcc.dg/torture/Wsizeof-pointer-memaccess1.c
    trunk/gcc/testsuite/gcc.dg/torture/pr71132.c
    trunk/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c
    trunk/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-3.c
    trunk/gcc/testsuite/gfortran.dg/char_length_3.f90
    trunk/gcc/testsuite/gfortran.dg/pr38868.f
Comment 4 Martin Sebor 2016-12-08 00:22:03 UTC
Fixed by 243419.