[PATCH] Correct exit codes for invalid tar archives

[Cristian Ionescu-Idbohrn]

On Tue, 19 Nov 2013, Denys Vlasenko wrote:
> On Fri, Nov 15, 2013 at 3:30 PM, Cristian Ionescu-Idbohrn <cristian.ionescu-idbohrn at axis.com> wrote:
> > This is what my work collegue proposes.  Works for us.
> >
> > Acceptable hack Denys?
> >
> > Signed-off-by: Magnus Rolf <Magnus.Rolf at axis.com>
> > ---
> >  busybox/archival/libarchive/get_header_tar.c | 17 +++++++----------
> >  1 file changed, 7 insertions(+), 10 deletions(-)
> >
> > diff --git a/busybox/archival/libarchive/get_header_tar.c b/busybox/archival/libarchive/get_header_tar.c
> > index b168653..54ddb19 100644
> > --- a/busybox/archival/libarchive/get_header_tar.c
> > +++ b/busybox/archival/libarchive/get_header_tar.c
> > @@ -190,17 +190,14 @@ char FAST_FUNC get_header_tar(archive_handle_t *archive_handle)
> >         /* to prevent misdetection of bz2 sig */
> >         *(aliased_uint32_t*)&tar = 0;
> >         i = full_read(archive_handle->src_fd, &tar, 512);
> > -       /* If GNU tar sees EOF in above read, it says:
> > -        * "tar: A lone zero block at N", where N = kilobyte
> > -        * where EOF was met (not EOF block, actual EOF!),
> > -        * and exits with EXIT_SUCCESS.
> > -        * We will mimic exit(EXIT_SUCCESS), although we will not mimic
> > -        * the message and we don't check whether we indeed
> > -        * saw zero block directly before this. */
> > -       if (i == 0) {
> > -               xfunc_error_retval = 0;
> > +       /* Since v1.19, GNU tar exits with code 2 when supplied an archive smaller
> > +        * than 512 bytes in reading mode (-x, -t).
> > +        * Previous tar versions silently ignored it, exiting with code 0.
> > +        */
> > +       if (i < 512) {
> > +               xfunc_error_retval = 2;
> >   short_read:
> > -               bb_error_msg_and_die("short read");
> > +               bb_error_msg_and_die("This does not look like a tar archive");
> >         }
>
> There are valid gzipped tarballs smaller than 512 bytes.

Yes, but, as I wrote, my understanding of the patch above is that it
only test the tar-ball (after possible decompression).  IOW, the 512
magic number has nothing to do with gzipped tar-balls.  Am I wrong?

> Your 1st example tries to unpack 0-byte file. In uncompressed case,
> such file can be interpreted as "valid" empty tarball with truncated
> EOF blocks.

Won't argue with that, as my tar-fu is very limited.

> The example when we misinterpret empty file as valid *tgz* file
> is a bug. I'm applying attached patch to fix that case.

True.  Still (after applying this patch) if I do:

	$ tar cf foo.tar foo

with a zero-bytes foo and hexdump the result (the uncompressed case),
I see a filename (foo), permissions, a date (maybe), owner and group,
and not a '"valid" empty tarball with truncated EOF blocks'.  In such
a case, GNU-tar shows:

	/bin/tar: This does not look like a tar archive
	/bin/tar: Exiting with failure status due to previous errors
	E: failed ($?=2) to check tarball '../foo'

	gzip: stdin: unexpected end of file
	/bin/tar: Child returned status 1
	/bin/tar: Error is not recoverable: exiting now
	E: failed ($?=2) to decompress and check tarbal '../foo'

where as bb-tar shows:

	tar: short read

	tar: invalid magic
	tar: short read
	E: failed ($?=1) to decompress and check tarbal '../foo'

(see my simple script, earlier in this thread; tar {tf|tzf} is
actually sufficient to demonstrate the point).

What about some tests under testsuite/tar to validate the proper
behaviour?  I'm willing to contribute such patch(es) if noone else
beats me to it.


Cheers,

-- 
Cristian