Proper defense against symlink attacks in tar, unzip et al

[Harald van Dijk]

On 19/02/2018 15:47, Denys Vlasenko 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. Since the a/a -> ../b symlink may not even be in this 
archive, the only way to determine that is by lstat().

Cheers,
Harald van Dijk