Bug#737160: [uupdate] symlink directory traversal

[Jakub Wilk]

* James McCoy <jamessan at debian.org>, 2014-02-21, 22:53:
>>A malicious .orig.tar file can trick uupdate into patching files 
>>outside the source package directory. Proof of concept:
>
>Thanks for the report and PoC.
>
>Looking into it some, below is my understanding of the issue and 
>concerns on fixing it.
>
>First, this is only a problem for 1.0 format source packages, since 
>unpacking a 3.0 format's diff tarball will replace a, potentially 
>malicious, symlink in upstream's source with the corresponding 
>directory in the diff tarball.

Indeed.

>With it constrained to 1.0 format, the problem exists for any file the 
>diff.gz is adding (or possibly, but much less likely, modifying) where 
>one of the directories in the path is a symlink pointing outside of the 
>upstream source tree.

That's right.

>We basically need to add the following just inside the if on line 730:
>
>    for link in $(find -type l); do
>        resolved="$(readlink -f "$link")"
>        if ! expr "$resolved" : "$(pwd)" >/dev/null; then
>            complain loudly
>        fi
>    done
>
>The problem with the above is that it's not robust in the face of paths 
>which contain whitespace.  That means, at best, some paths aren't 
>properly detected and therefore are still subject to original issue.

More (minor) problems I see:
- Special characters in the regexp are not escaped. At least "." could 
be legitimately part of cwd.
- TOCTOU problem: what the symlink resolves to can change between the 
readlink call and unpacking. This is because the symlink could point to 
another symlink, owned by malicious local user.
- The check probably shouldn't complain about symlinks that weren't 
going to be patched anyway.

>If someone more familiar with the inrticacies of handling this sort of 
>scenario in (ba)sh has an idea on how to properly implement this, I'm 
>all ears.

Perhaps a more viable way would be to construct a temporary new source 
package, and let dpkg-source deal with all the corner cases of unpacking 
it?

Or maybe patch(1) should (have an option to) avoid following directory 
symlinks.

-- 
Jakub Wilk