The following commit has been merged in the master branch:
commit 2cb3d5d38fbe937280a8875b79a7247ac37a383d
Author: Raphaël Hertzog <[email protected]>
Date:   Fri Mar 5 18:19:33 2010 +0100

    Dpkg::Source::Patch: add more sanity checks on patches
    
    patch will happily accept filenames like "../../../../stuff" and modify
    files outside of the expected destination directory. To avoid problems
    we error out when we detect a filename that contains "/../". Any leading
    "../" is not a problem since patches are applied with -p1 and it's
    stripped.
    
    We also verify that the file to be modified is not accessed through a
    symlink as a compromised source package could also provide a symlink in
    the orig.tar.gz that points outside of the destination directory.

diff --git a/debian/changelog b/debian/changelog
index a1e85b7..d2f8ca5 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -67,6 +67,12 @@ dpkg (1.15.6) UNRELEASED; urgency=low
     spaces. Closes: #572030
   * dpkg-source does no longer fallback to other source formats if the
     requested one is not usable. Closes: #557459
+  * Modify dpkg-source to error out when it would apply patches containing
+    insecure paths (with "/../") and also error out when it would apply a
+    patch through a symlink. Those checks are required as patch will happily
+    modify files outside of the target directory and unpacking a source package
+    should not be able to have any side-effect outside of the target
+    directory. Fixes CVE-2010-0396.
 
   [ Guillem Jover ]
   * Handle argument parsing in dpkg-checkbuilddeps and dpkg-scanpackages
diff --git a/scripts/Dpkg/Source/Patch.pm b/scripts/Dpkg/Source/Patch.pm
index 18130ec..10f751c 100644
--- a/scripts/Dpkg/Source/Patch.pm
+++ b/scripts/Dpkg/Source/Patch.pm
@@ -322,8 +322,9 @@ sub analyze {
            error(_g("expected ^--- in line %d of diff `%s'"), $., $diff);
        }
         $_ = strip_ts($_);
-        if ($_ eq '/dev/null' or s{^(\./)?[^/]+/}{$destdir/}) {
+        if ($_ eq '/dev/null' or s{^[^/]+/}{$destdir/}) {
             $fn = $_;
+           error(_g("%s contains an insecure path: %s"), $diff, $_) if 
m{/\.\./};
         }
        if (/\.dpkg-orig$/) {
            error(_g("diff `%s' patches file with name ending .dpkg-orig"), 
$diff);
@@ -336,8 +337,9 @@ sub analyze {
            error(_g("line after --- isn't as expected in diff `%s' (line 
%d)"), $diff, $.);
        }
         $_ = strip_ts($_);
-        if ($_ eq '/dev/null' or s{^(\./)?[^/]+/}{$destdir/}) {
+        if ($_ eq '/dev/null' or s{^[^/]+/}{$destdir/}) {
             $fn2 = $_;
+           error(_g("%s contains an insecure path: %s"), $diff, $_) if 
m{/\.\./};
         } else {
             unless (defined $fn) {
                 error(_g("none of the filenames in ---/+++ are relative in 
diff `%s' (line %d)"),
@@ -363,6 +365,17 @@ sub analyze {
        if ($dirname =~ s{/[^/]+$}{} && not -d $dirname) {
            $dirtocreate{$dirname} = 1;
        }
+
+       # Sanity check, refuse to patch through a symlink
+       $dirname = $fn;
+       while (1) {
+           if (-l $dirname) {
+               error(_g("diff %s modifies file %s through a symlink: %s"),
+                     $diff, $fn, $dirname);
+           }
+           last unless $dirname =~ s{/[^/]+$}{};
+       }
+
        if (-e $fn and not -f _) {
            error(_g("diff `%s' patches something which is not a plain file"), 
$diff);
        }

-- 
dpkg's main repository


-- 
To UNSUBSCRIBE, email to [email protected]
with a subject of "unsubscribe". Trouble? Contact [email protected]

Reply via email to