On Thu, 2011-12-08 at 16:37 +0100, Raphael Hertzog wrote: > I don't see the immediate need for this. It does this with temporary > files/dirs that it uses for example when it builds a package. But I don't > see why it should do this for non-temporary files...
Well, either that or delete the unsafe symlinks and issue a warning. > To make it executable. At first glance that seems like a security vulnerability, since it could allow an attacker to make an arbitrary file/dir executable. For the trivial case (if debian/rules is an unsafe symlink), I see that dpkg-source is not vulnerable to this issue. I didn't check any cases involving debian/ being an unsafe symlink, but I think that dpkg-source should check for that too. pabs@chianamo ~/tmp/foo $ ls -l ~/foo -rw------- 1 pabs pabs 0 Feb 12 09:27 /home/pabs/foo pabs@chianamo ~/tmp/foo $ dpkg-source -x foo_0.1.dsc dpkg-source: warning: extracting unsigned source package (foo_0.1.dsc) dpkg-source: info: extracting foo in foo-0.1 dpkg-source: info: unpacking foo_0.1.tar.gz dpkg-source: warning: foo-0.1/debian/rules is not a plain file pabs@chianamo ~/tmp/foo $ ls -l ~/foo -rw------- 1 pabs pabs 0 Feb 12 09:27 /home/pabs/foo pabs@chianamo ~/tmp/foo $ ls -l foo-0.1/debian/rules lrwxrwxrwx 1 pabs pabs 14 Feb 12 09:27 foo-0.1/debian/rules -> /home/pabs/foo I do note that debuild -S does change the permissions though, of course it also runs debian/rules clean, which would always execute arbitrary code so I guess that is safe if a bit weird. Personally I would suggest that dpkg-source should error out or warn when building a source package containing links to outside the source package, since the resulting symlink will likely be invalid on systems consuming the source package. pabs@chianamo ~/tmp/foo $ ls -l ~/foo -rw------- 1 pabs pabs 0 Feb 12 09:27 /home/pabs/foo pabs@chianamo ~/tmp/foo $ ls -l foo-0.1/debian/rules lrwxrwxrwx 1 pabs pabs 14 Feb 12 09:27 foo-0.1/debian/rules -> /home/pabs/foo pabs@chianamo ~/tmp/foo $ cd foo-0.1/ pabs@chianamo ~/tmp/foo/foo-0.1 $ debuild -S dpkg-buildpackage -rfakeroot -d -us -uc -j4 -i -ICVS -I.svn -S dpkg-buildpackage: source package foo dpkg-buildpackage: source version 0.1 dpkg-buildpackage: source changed by Paul Wise <[email protected]> dpkg-source -i -ICVS -I.svn --before-build foo-0.1 fakeroot debian/rules clean dpkg-source -i -ICVS -I.svn -b foo-0.1 dpkg-source: warning: no source format specified in debian/source/format, see dpkg-source(1) dpkg-source: info: using source format `1.0' dpkg-source: info: building foo in foo_0.1.tar.gz dpkg-source: warning: missing information for output field Standards-Version dpkg-source: info: building foo in foo_0.1.dsc dpkg-genchanges -S >../foo_0.1_source.changes dpkg-genchanges: warning: missing Section for source files dpkg-genchanges: including full source code in upload dpkg-source -i -ICVS -I.svn --after-build foo-0.1 dpkg-buildpackage: source only upload: Debian-native package pabs@chianamo ~/tmp/foo/foo-0.1 $ ls -l ~/foo -rwxr-xr-x 1 pabs pabs 0 Feb 12 09:27 /home/pabs/foo* > dpkg-source delegates extraction to tar. It can't easily cherry-pick > what to extract... I see, you could definitely do some checks after the fact though? > dpkg-source already has code to not extract new/supplementary files > over already unpacked symlinks so that unpacking a source package can't > write files outside of the newly created directory. > > What kind of bad impact do you expect ? The worst I can imagine is > something like "chmod -R" changing rights but apparently "chmod -R" > already ignores symlinks by default. > > We should still do something to avoid chmodding() a file outside of the > unpacked source tree but I don't know whether it's just ensuring that > debian and debian/rules are not symlinks, or a more heavy analyzis to get > rid of all symlinks pointing outside of the source package. You can't know what kind of commands a user or script will run on an unpacked source tree or whether the commands/script they are running is written with security in mind or not. Practising defence in depth is always a good idea, especially where people usually are not thinking about security issues, which I very much doubt is happening when people are processing source code in some way. As an example, say people are automatically using a source code checker, pretty-printer or comparison, automatically dumping output to the web. Presumably that would follow symlinks, maybe resulting in a nasty package causing arbitrary file leakage (think SSH private keys, proprietary source code, whatever). dpkg-source should definitely check both debian/ and debian/rules for unsafe symlinks before doing the chmod +x. I would also recommend removing any absolute or relative symlinks that point outside of the unpacked source tree and issuing a warning if there are any such unsafe symlinks. Alternatively you could fail noisily if there are any unsafe symlinks at all and delete the unpacked source tree. -- bye, pabs http://wiki.debian.org/PaulWise
signature.asc
Description: This is a digitally signed message part
