[Bug 10651] New: tar: check for unsafe symlinks is overly strict

[bugzilla at busybox.net]

https://bugs.busybox.net/show_bug.cgi?id=10651

            Bug ID: 10651
           Summary: tar: check for unsafe symlinks is overly strict
           Product: Busybox
           Version: 1.28.x
          Hardware: All
                OS: Linux
            Status: NEW
          Severity: normal
          Priority: P5
         Component: Other
          Assignee: unassigned at busybox.net
          Reporter: bb at gigawatt.nl
                CC: busybox-cvs at busybox.net
  Target Milestone: ---

To counter bug #8411, busybox tar no longer allows unsafe symlinks to be
created. Unfortunately, it is now so strict that safe symlinks are no longer
accepted either. Ironically, the very first failure that I got after upgrading
to busybox 1.28 was for a tarball containing along with other files:

bin/busybox
usr/bin/ar -> ../../bin/busybox

At the very least, symlinks to files are safe no matter what they start with.
There was no need to block this. It cannot be used to modify any files
inappropriately by tar, because a subsequent attempt to extract there would
delete the symlink before creating the file.

But actually, I think the whole logic for checking unsafe symlinks is
unnecessary. It's not the symlink that's unsafe, it's a subsequent attempt to
use that symlink to create or overwrite a file where the user shouldn't be able
to modify anything. The initial patch handled this for extraction into empty
directories by delaying the symlink creation. This is sufficient if the pattern
is 1) create temp directory 2) extract into temp directory 3) validate contents
of temp directory 4) remove temp directory if contents are invalid.

But indeed, this is not enough if multiple untrusted tarballs are extracted
into the same directory. I do not know if that is supposed to be safe. It
currently isn't with GNU tar, but it may be reasonable to expect that to be
safe.

One way to handle that would be, on an attempt to extract a/b/c, to require
that a and a/b are truly directories, not symlinks. This is doable by using
openat(), opening all path components individually with O_NOFOLLOW. This way,
if any component is a symlink, an error is raised. If the path components are
cached, then for the common case where a large number of files appear in the
same directory, it might even reduce path lookups.

Would something like this be acceptable, or does this have other security
implications that I'm not seeing?

-- 
You are receiving this mail because:
You are on the CC list for the bug.