strncpy() core dump when I use Libtar in ubuntu12.04

Question

I trace the coredump in the encode.c:33.

The source code like this:

if (t->options & TAR_GNU) 
    strncpy(t->th_buf.magic, "ustar  ", 8);    // here is the coredump                                                                                                                                 
else
    .... 

the function call stack like this:

0x4064d73a in strncpy (__len=8, __src=0x40663b34 "ustar  ", __dest=0x4104e5ed "") at /usr/include/i386-linux-gnu/bits/string3.h:121
0x4024e342 in __strncpy_chk () from /lib/i386-linux-gnu/libc.so.6
0x4024ee1a in __chk_fail () from /lib/i386-linux-gnu/libc.so.6
0x40250065 in __fortify_fail () from /lib/i386-linux-gnu/libc.so.6
0x401b739a in ?? () from /lib/i386-linux-gnu/libc.so.6
0x4017d825 in abort () from /lib/i386-linux-gnu/libc.so.6
0x4017a1df in raise () from /lib/i386-linux-gnu/libc.so.6
0x40064424 in __kernel_vsyscall ()
<signal handler called>
sig_coredump (sig=6) at mpm_common.c:1207

the t->th_buf.magic is sturct of tar_header, the defination:

struct tar_header{
    ...;
    char magic[6];
    char version[2];
    ...;
}

I pretty sure strncpy can use in this way.

And in my case, the t->th_buf has malloc already.

In gdb:

(gdb) p t->th_buf
$5 = {name = "/TARFILE.C", '\000' <repeats 89 times>, mode = "100644 ", 
      uid = "    41", gid = "    41 ", size = "     207114 ", mtime = "12115070475 ", 
      chksum = "\000\000\000\000\000\000\000", typeflag = 48 '0', linkname = '\000' <repeats 99 times>, 
      magic = "\000\000\000\000\000", version = "\000",
      ...

Answer 1

Although you are trying to copy eight bytes into a buffer that only has room for six, that wouldn't normally be a problem.

The reason it's not usually a problem is that there is a two-byte character array immediately following that is perfectly capable of taking the buffer overflow:

struct tar_header{
    ...;
    char magic[6];
    char version[2];     // Can absorb extra two chars easily.
    ...;
}

---

The real problem with strncpy is that most people don't understand how it works.

If the buffer being copied is larger than the size you specify, it basically stops at the size and copies no more, including the null-terminator at the end of the string.

Hence strncpy can end up giving you something that is not a C string and, if you subsequently try to use it as a C string (such as passing it to strlen), all hell may break loose.

That's why I prefer to use strlen check followed by strcpy, assuming the data can't change between the two parts. That way, you're guaranteed to end up with a C string (or you know about it before hand).

---

However, that's also not the specific problem here.

Within your call stack, there is a call to a checking function __strncpy_chk which goes something like this:

char *
__strncpy_chk (s1, s2, n, s1len)
     char *s1;
     const char *s2;
     size_t n;
     size_t s1len;
{
    char c;
    char *s = s1;

    if (__builtin_expect (s1len < n, 0))
        __chk_fail ();
    :
    rest of function to copy the stuff.
}

Hence it's checking, in advance, whether the source string is too long for the length you've specified, and failing before it even attempts the copy. This is basically doing the same steps I mention earlier but forcing a crash rather than you knowing about it in your own code.

That's where your error is coming from, a bit of extra security in the runtime library.

Keep in mind that this extra check is only in the debug code, not in the production ready code. That way, you can confirm that you're doing the right thing during development without it slowing down your code in the field.

Answer 2

magic is only 6 chars and you are writing 8 to it.

Answer 3

You are trying to copy 8 bytes:

strncpy(t->th_buf.magic, "ustar  ", 8); 

into a buffer with allocated space for 6

char magic[6];

Not going to work.