tar segfaults (busybox 1.8.1)

[Denys Vlasenko]

On Friday 16 November 2007 05:19, Tito wrote:
> On Friday 16 November 2007 13:34:25 Alexander Griesser wrote:
> > tuxx at vi-edv003:~/busybox/busybox-1.8.1$ ./busybox tar cf test.tar a s d
> > *** buffer overflow detected ***: ./busybox terminated
> Hi,
> by compiling the latest svn i get this warning:
>
> CC      archival/tar.o
> archival/tar.c: In function ‘writeFileToTarball’:
> archival/tar.c:183: warning: call to __builtin___strcpy_chk will always
> overflow destination buffer AR      archival/lib.a
>   LINK    busybox_unstripped
> Trying libraries: crypt m
> Library crypt is needed
> Library m is needed
> Final link with: crypt m
>
>
> and after changing the line 183 of tar.c
>
> -	strcpy(hp->magic, "ustar  ");
> +	strcpy(hp->magic, "ustar");

This was intended:

http://en.wikipedia.org/wiki/Tar_(file_format)

USTAR format

Most modern tar programs read and write archives in the new USTAR (Uniform 
Standard Tape Archive) format...

Field Offset 	Field Size 	Field
0 	156 	(as in old format)
156 	1 	Type flag
157 	100 	(as in old format)
257 	6 	USTAR indicator "ustar"
263 	2 	USTAR version "00"
265 	32 	Owner user name
297 	32 	Owner group name
329 	8 	Device major number
337 	8 	Device minor number
345 	155 	Filename prefix

Example

The example below shows the ASCII dump of a header block from a tar file 
created using the GNU tar program. It was dumped with the od program. 
The "ustar" magic string followed by two spaces can be seen, meaning that the 
tar file is in GNU format, partially incompatible with the true USTAR 
standard (in POSIX.1-1988), which has the signature "ustar" followed by a NUL 
character.

0000000   e   t   c   /   p   a   s   s   w   d nul nul nul nul nul nul
0000020 nul nul nul nul nul nul nul nul nul nul nul nul nul nul nul nul
*
0000140 nul nul nul nul   0   1   0   0   6   4   4 nul   0   0   0   0
0000160   0   0   0 nul   0   0   0   0   0   0   0 nul   0   0   0   0
0000200   0   0   4   1   3   5   5 nul   1   0   1   5   5   0   6   1
0000220   1   0   5 nul   0   1   1   5   5   6 nul  sp   0 nul nul nul
0000240 nul nul nul nul nul nul nul nul nul nul nul nul nul nul nul nul
*
0000400 nul   u   s   t   a   r  sp  sp nul   r   o   o   t nul nul nul
0000420 nul nul nul nul nul nul nul nul nul nul nul nul nul nul nul nul
0000440 nul nul nul nul nul nul nul nul nul   r   o   o   t nul nul nul
0000460 nul nul nul nul nul nul nul nul nul nul nul nul nul nul nul nul
*
0001000


> there is no segfault due to the fact

Well, it's not a segfault, it's some kind of glibc buffer overflow check

> that the size of hp->magic is
>
> 	char magic[6];            /* 257-262 */

it is immediately followed by char version[2].
I cannot imagine how in hell glibc detects "buffer overflow" which is not 
really there.

> root at localhost:~/Desktop/busybox#  touch a s d
> root at localhost:~/Desktop/busybox#  ./busybox tar cf test.tar a s d
> root at localhost:~/Desktop/busybox# ls -la test.tar
> -rw-r--r-- 1 root root 2560 2007-11-16 14:15 test.tar
>
>
> I cannot say if this fix is correct or if maybe it should be
> done the other way by increasing the size of char magic
> to 8.

Yes, I think we can get rid of char version[2] and grow magic to 8 bytes

But, Alexander, can you confirm that replacing "ustar  " with "ustar"
*really* makes it go away? This is unbelievable.
--
vda