Proper defense against symlink attacks in tar, unzip et al

Denys Vlasenko vda.linux at googlemail.com
Sun Feb 18 20:01:39 UTC 2018


Bug 10651 - tar: check for unsafe symlinks is overly strict
https://bugs.busybox.net/show_bug.cgi?id=10651

| (In reply to Harald van Dijk from comment #6)
| 1: Keep the current check, but modify it to allow all symlinks to
files to be extracted.

(1) What if symlink comes first in the tarball?

(2) Is it ok to extract symlink to e.g. "/etc/passwd"? For tar, it
_should be ok_ since newly extracted files unlink target filename,
then open it with O_CREAT|O_EXCL, thus /etc/passwd can't be
overwritten thru this symlink - but it is still feels iffy. Basically,
_tar_ will not compromise security - but it may unknowingly create a
setup for subsequent writes to the "file" to break it.

| OR:
|
| 2: Remove the current check, allow all symlinks to be extracted, but
add a check that requires all path components during extraction to be
real directories, not symlinks.
| Option 2 is difficult to implement, but potentially safer since it
also protects against symlinks created without the use of tar. An
added benefit is that it would never error out when simply creating an
archive from directory A and extracting it into directory B,
regardless of what symlinks A might contain, so it would have fewer
false positives.

Opt 2 is good. A slight downside is that there is no open flag in
Linux which tells open() to fail if any path component is a symlink.
O_NOFOLLOW only checks last component. We need to check each one. We
will need to lstat() every path component in sequence.

Let's also brainstorm option 3:
Allow symlinks which
(a) start with one or more "../";
(b) never end up on a higher level of directory tree:
"../dir/.." is ok,
"../../usr/bin/.." is ok,
"../file" is not ok,
"../../dir/file" is not ok;
and (c) they also should not leave tarred tree:
symlink "sbin/mkswap" to "../bin/busybox" is ok, but
symlink "mkswap" to "../bin/busybox" is not ok (it hops "above" the tree).

This sounds a bit complicated, but it can be achieved just by looking
at names, no multiple lstat() calls for every open() needed.

With only these symlinks allowed, we know that if we untar to an empty
directory or to a formerly empty directory with another tarball
untarred, any pathname we open, whilst it can contain components which
are symlinks, they nevertheless can not allow new opens to "leave" the
tree and create files elsewhere.


More information about the busybox mailing list