Proper defense against symlink attacks in tar, unzip et al

[Denys Vlasenko]

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.