Bug 515361 - tftp FORTIFY_SOURCE strcpy crash
Summary: tftp FORTIFY_SOURCE strcpy crash
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: tftp
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Jiri Skala
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: F12Blocker, F12FinalBlocker
TreeView+ depends on / blocked
 
Reported: 2009-08-03 21:10 UTC by Warren Togami
Modified: 2014-11-09 22:31 UTC (History)
4 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2009-08-05 14:31:53 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Warren Togami 2009-08-03 21:10:00 UTC
tftp-0.49-3.fc11 working binary
tftp-0.49-4.fc12 broken binary

tftp> get /path/to/file.txt
*** buffer overflow detected ***: tftp terminated
======= Backtrace: =========
/lib/libc.so.6(__fortify_fail+0x4d)[0xcdf1ad]
/lib/libc.so.6[0xcdd27a]
/lib/libc.so.6(__strcpy_chk+0x44)[0xcdc554]
tftp[0x8049025]
tftp[0x8049be2]
tftp[0x804a807]
tftp[0x804b39f]
/lib/libc.so.6(__libc_start_main+0xe6)[0xbfeb26]
tftp[0x8048f51]
======= Memory map: ========
00498000-004cd000 r-xp 00000000 00:10 5125       /lib/libreadline.so.6.0
004cd000-004d1000 rw-p 00035000 00:10 5125       /lib/libreadline.so.6.0
004d1000-004d2000 rw-p 00000000 00:00 0 
00708000-00728000 r-xp 00000000 00:10 4127       /lib/ld-2.10.90.so
00728000-00729000 r--p 0001f000 00:10 4127       /lib/ld-2.10.90.so
00729000-0072a000 rw-p 00020000 00:10 4127       /lib/ld-2.10.90.so
00755000-00760000 r-xp 00000000 00:10 4150       /lib/libnss_files-2.10.90.so
00760000-00761000 r--p 0000a000 00:10 4150       /lib/libnss_files-2.10.90.so
00761000-00762000 rw-p 0000b000 00:10 4150       /lib/libnss_files-2.10.90.so
00890000-00891000 r-xp 00000000 00:00 0          [vdso]
009fd000-00a13000 r-xp 00000000 00:10 4441       /lib/libtinfo.so.5.7
00a13000-00a16000 rw-p 00015000 00:10 4441       /lib/libtinfo.so.5.7
00aa7000-00ad1000 r-xp 00000000 00:10 4103       /lib/libgcc_s-4.4.1-20090725.so.1
00ad1000-00ad2000 rw-p 00029000 00:10 4103       /lib/libgcc_s-4.4.1-20090725.so.1
00be8000-00d5c000 r-xp 00000000 00:10 4134       /lib/libc-2.10.90.so
00d5c000-00d5e000 r--p 00174000 00:10 4134       /lib/libc-2.10.90.so
00d5e000-00d5f000 rw-p 00176000 00:10 4134       /lib/libc-2.10.90.so
00d5f000-00d62000 rw-p 00000000 00:00 0 
08048000-0804e000 r-xp 00000000 00:10 7932       /usr/bin/tftp
0804e000-0804f000 rw-p 00005000 00:10 7932       /usr/bin/tftp
0804f000-0806f000 rw-p 00000000 00:00 0 
08f0c000-08f2d000 rw-p 00000000 00:00 0          [heap]
b7ea4000-b7eab000 r--s 00000000 00:10 4434       /usr/lib/gconv/gconv-modules.cache
b7eab000-b80ab000 r--p 00000000 00:10 5848       /usr/lib/locale/locale-archive
b80ab000-b80ac000 rw-p 00000000 00:00 0 
b80b1000-b80b4000 rw-p 00000000 00:00 0 
bfba1000-bfbb6000 rw-p 00000000 00:00 0          [stack]
Aborted

gdb backtrace is not useful for FORTIFY crashes in identifying which strcpy is at fault.

http://gcc.gnu.org/ml/gcc-patches/2009-06/msg00419.html
jakub says this is likely effected by this compiler change.  If the binary is built with -D_FORTIFY_SOURCE=1 instead of 2 and it works, then this is confirmation that this is the issue.  I have not successfully built it with -D_FORTIFY_SOURCE=1 yet.

Comment 1 Warren Togami 2009-08-05 01:43:25 UTC
tftp-hpa-0.49/tftp/tftp.c

static int
makerequest(int request, const char *name,
            struct tftphdr *tp, const char *mode)
{
    char *cp;

    tp->th_opcode = htons((u_short) request);
    cp = (char *)&(tp->th_stuff);
    strcpy(cp, name);
    cp += strlen(name);
    *cp++ = '\0';
    strcpy(cp, mode);
    cp += strlen(mode);
    *cp++ = '\0';
    return (cp - (char *)tp);
}

The first strcpy() is where it crashes.

Comment 2 Warren Togami 2009-08-05 02:03:49 UTC
This function pre-processed.

static int
makerequest(int request, const char *name,
            struct tftphdr *tp, const char *mode)
{
    char *cp;

    tp->th_opcode = (__extension__ ({ register unsigned short int __v, __x = ((u_short) request); if (__builtin_constant_p (__x)) __v = ((((__x) >> 8) & 0xff) | (((__x) & 0xff) << 8)); else __asm__ ("rorw $8, %w0" : "=r" (__v) : "0" (__x) : "cc"); __v; }));
    cp = (char *)&(tp->th_u.tu_stuff);
    strcpy(cp, name);
    cp += strlen(name);
    *cp++ = '\0';
    strcpy(cp, mode);
    cp += strlen(mode);
    *cp++ = '\0';
    return (cp - (char *)tp);
}

/usr/include/arpa/tftp.h contains the definition of struct tftphdr.

struct  tftphdr {
        short   th_opcode;                      /* packet type */
        union {
                unsigned short  tu_block;       /* block # */
                short   tu_code;                /* error code */
                char    tu_stuff[1];            /* request packet stuff */
        } __attribute__ ((__packed__)) th_u;
        char    th_data[1];                     /* data or error string */
} __attribute__ ((__packed__));

#define th_block        th_u.tu_block
#define th_code         th_u.tu_code
#define th_stuff        th_u.tu_stuff
#define th_msg          th_data

jakub, do you need any more?

Comment 3 Jakub Jelinek 2009-08-05 07:22:17 UTC
th_u isn't the last field in the struct, so flexible array member and similar exception definitely doesn't apply here.  You can use memcpy instead of strcpy (memcpy is always allowed to cross field boundaries), after all you are computing strlen (name) immediately afterwards anyway, so just trplacing
strcpy(cp, name); cp += strlen(name); *cp++ = '\0'; with size_t name_len = strlen(name) + 1; memcpy(cp, name, name_len); cp += name_len; and similarly for mode should work.


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