Proper defense against symlink attacks in tar, unzip et al

Michael Conrad mconrad at intellitree.com
Mon Feb 19 17:40:04 UTC 2018


On 2/19/2018 11:15 AM, Denys Vlasenko wrote:
> On Mon, Feb 19, 2018 at 5:09 PM, Harald van Dijk <harald at gigawatt.nl> wrote:
>>>>> 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.
>>>> This wouldn't be safe, I think. Consider a tarball containing
>>>>
>>>>     a/
>>>>     b/
>>>>     a/a -> ../b
>>>>     a/a/a -> ../../c
>>>
>>> The link to "../../c" is not allowed - it fails criteria (b).
>> You wrote "it can be achieved just by looking at names", but that's not
>> enough here: you have to know that a/a/a is actually b/a, so only one level
>> deep in the -C directory, to know that ../../c points outside the -C
>> directory.
> Yes... but:
>
>> Since the a/a -> ../b symlink may not even be in this archive,
>> the only way to determine that is by lstat().
> I assume that tarball(s) are being unpacked into an empty directory.
> The case when someone already placed malicious symlinks there
> before unpacking would be a bug in whatever tool allowed _that_
> to happen.
>
> My goal here is to not allow tar (and unzip) to create such symlinks.

That might be considered a bug though.  There's lots of times a symlink 
points to a higher directory (or even starts with '/') and tar should be 
a reliable way to mirror a directory tree.

What if you do a test on the inode of the destination directory?  To 
determine whether it is valid, repeatedly append ".." and stat until you 
reach root or the unpack directory.  Then to speed it up, make a 
fixed-size LRU cache of directories you know are valid.  Each time you 
create a directory, add it to the cache, and then on average you 
probably only add one directory stat per unpacked file.  For more speed 
and less security, just change it to a cache of path names.

-Mike


More information about the busybox mailing list