[Bug cli/14011] New: GDB uses strcpy() with undefined behaviour, causing bug in CLI cd_command().

[Bug cli/14011] New: GDB uses strcpy() with undefined behaviour, causing bug in CLI cd_command().

[fredrik.hederstierna@securitas-direct.com]

http://sourceware.org/bugzilla/show_bug.cgi?id=14011

             Bug #: 14011
           Summary: GDB uses strcpy() with undefined behaviour, causing
                    bug in CLI cd_command().
           Product: gdb
           Version: 7.4
            Status: NEW
          Severity: normal
          Priority: P2
         Component: cli
        AssignedTo: unassigned@sourceware.org
        ReportedBy: fredrik.hederstierna@securitas-direct.com
    Classification: Unclassified


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

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug cli/14011] GDB uses strcpy() with undefined behaviour, causing bug in CLI cd_command().

[fredrik.hederstierna@securitas-direct.com]

http://sourceware.org/bugzilla/show_bug.cgi?id=14011

--- Comment #1 from Fredrik Hederstierna <fredrik.hederstierna@securitas-direct.com> 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

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug cli/14011] GDB uses strcpy() with undefined behaviour, causing bug in CLI cd_command().

[qiyao at gcc dot gnu.org]

http://sourceware.org/bugzilla/show_bug.cgi?id=14011

Yao Qi <qiyao at gcc dot gnu.org> changed:

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

--- Comment #2 from Yao Qi <qiyao at gcc dot gnu.org> 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.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug cli/14011] GDB uses strcpy() with undefined behaviour, causing bug in CLI cd_command().

[fredrik.hederstierna@securitas-direct.com]

http://sourceware.org/bugzilla/show_bug.cgi?id=14011

--- Comment #3 from Fredrik Hederstierna <fredrik.hederstierna@securitas-direct.com> 2012-04-25 10:36:52 UTC ---
Ok, done.
http://sourceware.org/ml/gdb-patches/2012-04/msg00853.html
/Fredrik

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug cli/14011] GDB uses strcpy() with undefined behaviour, causing bug in CLI cd_command().

[tromey at redhat dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=14011

Tom Tromey <tromey at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
                 CC|                            |tromey at redhat dot com
         Resolution|                            |FIXED
   Target Milestone|---                         |7.5

--- Comment #4 from Tom Tromey <tromey at redhat dot com> 2012-06-01 17:58:06 UTC ---
The fix was checked in.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.