Bug 895527 - potential buffer overflow uncovered by compiling with -O3 and FORTIFY_SOURCE
Summary: potential buffer overflow uncovered by compiling with -O3 and FORTIFY_SOURCE
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: gdb
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Jan Kratochvil
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2013-01-15 13:19 UTC by Gary Benson
Modified: 2013-01-15 16:09 UTC (History)
7 users (show)

Fixed In Version: gdb-7.5.1-34.fc18
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2013-01-15 14:39:57 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Sourceware 15017 0 None None None Never

Description Gary Benson 2013-01-15 13:19:13 UTC
I got this when compiling the latest Fedora source RPM (7.5.1-32). My default
CFLAGS is -O3 instead of the default -O2:

In function 'strncat',
    inlined from 'svr4_create_solib_event_breakpoints' at
../../gdb/solib-svr4.c:2076:
/usr/include/bits/string3.h:152: error: call to __builtin___strncat_chk might
overflow destination buffer


this correctly fails the build. The fix was simple, as the correct use of
strncat for the same data is one line below:
              if (with_prefix)
                strncat (name, "rtld_", sizeof (name));

              strncat (name, probe_info[i].name, sizeof (name) - sizeof ("rtld_"));

becomes

              if (with_prefix)
                strncat (name, "rtld_", sizeof (name) - sizeof("rtld_"));

              strncat (name, probe_info[i].name, sizeof (name) - sizeof ("rtld_"));


and this eliminates the warning/error.

Comment 3 Jan Kratochvil 2013-01-15 14:39:57 UTC
It appears already fixed to me:
gdb-dlopen-stap-probe-6of7.patch
char name[32] = { '\0' };
if (with_prefix) 
  strncat (name, "rtld_", sizeof (name));
strncat (name, probe_info[i].name, sizeof (name) - sizeof ("rtld_"));

I tried to rebuild gdb-7.5.1-35.fc18 with -O3 on F-18 and it is OK.

IIRC I have sent you a notice it failed with -O2 but it was a long time ago.

Comment 4 Gary Benson 2013-01-15 15:29:23 UTC
Does F-18 have -DFORTIFY_SOURCE?

I do remember a previous failure but I don't recall what.

Both those strncat calls need a "- 1" to allow space for the trailing NULL, so the patch I've pushed is more correct.  Either way, it's not an exploitable problem as I understand it, since all the input arguments (all probe_info[i].name) come from within GDB, and the 32 byte buffer is big enough for all of them.

Comment 5 Jan Kratochvil 2013-01-15 15:39:03 UTC
(In reply to comment #4)
> Does F-18 have -DFORTIFY_SOURCE?

Yes, by rpmbuild, I was testing the O3 variant as:

gcc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4  -m64 -mtune=generic -O3    -I. -I../../gdb -I../../gdb/common -I../../gdb/config -DLOCALEDIR="\"/usr/share/locale\"" -DHAVE_CONFIG_H -I../../gdb/../include/opcode -I../../gdb/../opcodes/..  -I../bfd -I../../gdb/../bfd -I../../gdb/../include -I../libdecnumber -I../../gdb/../libdecnumber  -I../../gdb/gnulib/import -Ibuild-gnulib/import   -DTUI=1  -I/usr/include/python2.7 -I/usr/include/python2.7 -Wall -Wdeclaration-after-statement -Wpointer-arith -Wformat-nonliteral -Wno-pointer-sign -Wno-unused -Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts -Wmissing-prototypes -Wdeclaration-after-statement -Wno-unused -Werror -c -o solib-svr4.o -MT solib-svr4.o -MMD -MP -MF .deps/solib-svr4.Tpo ../../gdb/solib-svr4.c


> Both those strncat calls need a "- 1" to allow space for the trailing NULL,

There is sizeof(string) instead of strlen(string) to count for that trailing '\0' (that is not NULL).


> since all the input arguments (all probe_info[i].name) come from within GDB,
> and the 32 byte buffer is big enough for all of them.

Yes, I agree keeping the compiler quiet is the only purpose of this topic.

Comment 6 Gary Benson 2013-01-15 16:09:03 UTC
Oh, I see, you used sizeof ("rtld_") in the second call.  I agree now that what you have is correct.  I'm going to keep what I have in GIT though for when I submit the patches, as I think it's clearer what is going on.


Note You need to log in before you can comment on or make changes to this bug.