Bug 14011 - GDB uses strcpy() with undefined behaviour, causing bug in CLI cd_command().
Summary: GDB uses strcpy() with undefined behaviour, causing bug in CLI cd_command().
Status: RESOLVED FIXED
Alias: None
Product: gdb
Classification: Unclassified
Component: cli (show other bugs)
Version: 7.4
: P2 normal
Target Milestone: 7.5
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-04-23 09:29 UTC by Fredrik Hederstierna
Modified: 2012-06-01 17:58 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Fredrik Hederstierna 2012-04-23 09:29:00 UTC
The C standard states that the behavior of strcpy() is undefined when the source and destination objects overlap.
Undefined behavior means it may work sometimes, or it may fail, or it may appear to succeed but manifest failure elsewhere in the program.

I got a failure running arm-elf-gdb-4.7.0 (compiled with GCC-4.6.1-9ubuntu3) with arguments

  arm-elf-gdb --cd=../../build/sniffer2/ sniffer2.elf

...
Reading symbols from /home/fredrikh/workspace/buile/sniffer2/sniffer2.elf...done.
(gdb)

Note that letter 'd' in 'build' is overwritten with letter 'e' in current_path.
The path to 'buile' is non-existing causing error.

I tracked down to the cd_command() function in CLI that was causing the bug.
It seems like the code is doing strcpy() on overlapping regions, to eliminate ".." paths, this causing an undefined behaviour.
GDB corrupted the dir-path replacing one letter:

The standard solution is to replace strcpy() with memmove(), and I submit a proposed patch that fixed the bug.



Index: gdb/cli/cli-cmds.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-cmds.c,v
retrieving revision 1.128
diff -r1.128 cli-cmds.c
420c420
<       strcpy (p, p + 2);
---
>       memmove(p, p + 2, strlen(p + 2) + 1);
439c439
<                 strcpy (q - 1, p + 3);
---
>                 memmove(q - 1, p + 3, strlen(p + 3) + 1);



I fear though that there might be more cases in the sources where strcpy() is used this way.
Maybe its a good idea to grep 'strcpy' and check that all uses are safe and non-overlapping.

Another idea is to use a custom gdb_strcpy() instead, that we know always copy from left-to-right, where we do define behaviour in the overlapping case.
Though is a danger to have dependencies on external C-lib implementation of string functions.

Thanks & Best Regards,

Fredrik Hederstierna
Securitas Direct AB
Malmoe Sweden
Comment 1 Fredrik Hederstierna 2012-04-23 10:41:30 UTC
Similar bug in 'source.c' found a month ago.

http://sourceware.org/ml/gdb-patches/2012-03/msg00264.html
/Fredrik
Comment 2 Yao Qi 2012-04-24 15:13:52 UTC
public list gdb-patches@sourceware.org is the place to review/discuss patches.  You'd better post your patch there, so maintainers can review and approve your patch.

Note that you should generate patch with `cvs diff -up', and you also need to create a ChangLog entry to describe your change in the patch.
Comment 3 Fredrik Hederstierna 2012-04-25 10:36:52 UTC
Ok, done.
http://sourceware.org/ml/gdb-patches/2012-04/msg00853.html
/Fredrik
Comment 4 Tom Tromey 2012-06-01 17:58:06 UTC
The fix was checked in.