Re: Security bug: tar allows to overwrite arbitrary file when extracting

[Eli Zaretskii]

> From: Paul Eggert <address@hidden>
> Date: Fri, 29 Jun 2001 18:14:10 -0700 (PDT)
> 
> > Date: Thu, 28 Jun 2001 11:30:48 +0200 (CEST)
> > From: Mikulas Patocka <address@hidden>
> > 
> > Here is output of strace -o tr tar xvf xploit.tar
> 
> Thanks.  Can you please try the following patch?
> 
> --- tar-1.13.19/src/extract.c Fri Jan 12 21:59:29 2001
> +++ tar-1.13.19-fix/src/extract.c     Fri Jun 29 17:53:37 2001
> @@ -850,9 +850,9 @@ extract_archive (void)
>       break;
>  
>        if (absolute_names_option
> -       || (ISSLASH (current_link_name
> -                    [FILESYSTEM_PREFIX_LEN (current_link_name)])
> -           && ! contains_dot_dot (current_link_name)))
> +       || ! (ISSLASH (current_link_name
> +                      [FILESYSTEM_PREFIX_LEN (current_link_name)])
> +             || contains_dot_dot (current_link_name)))
>       {
>         while (status = symlink (current_link_name, CURRENT_FILE_NAME),
>                status != 0)

I don't have the code handly, all I have is this snippet.  So I might
be talking nonsense, but the above (both the old and the new code)
looks wrong for systems where FILESYSTEM_PREFIX_LEN is non-zero.

I believe this code wants to catch the cases such as "d:/foo/bar",
which are DOS-style absolute file names.  If so, it is not the right
way to do that, for these reasons:

  - names such as d:foo/bar should also be considered absolute in this
    context;

  - a name such as ab/foo/bar will be handled by this as an absolute
    name (assuming that FILESYSTEM_PREFIX_LEN is 2 on DOSish systems).

I'd suggest instead to define a macro IS_ABSOLUTE which will return
non-zero if its argument is an absolute file names, and then define it
as appropriate for each filesystem.  That's what other GNU packages
do.  There's no need to bother the mainline code with such ugly
details.