[Reproducible-commits] [dpkg] 15/17: Dpkg::Source::Patch: Correctly parse C-style diff filenames

[Holger Levsen]

This is an automated email from the git hooks/post-receive script.

holger pushed a commit to annotated tag 1.16.13
in repository dpkg.

commit 73203c0bc4af425300fdcddc4be72624855798e7
Author: Guillem Jover <guillem at debian.org>
Date:   Tue Apr 15 08:15:44 2014 +0200

    Dpkg::Source::Patch: Correctly parse C-style diff filenames
    
    We need to strip the surrounding quotes, and unescape any escape
    sequence, so that we check the same files that the patch program will
    be using, otherwise a malicious package could overpass those checks,
    and perform directory traversal attacks on source package unpacking.
    
    Fixes: CVE-2014-0471
    
    Reported-by: Jakub Wilk <jwilk at debian.org>
---
 debian/changelog             |  3 +++
 scripts/Dpkg/Source/Patch.pm | 62 +++++++++++++++++++++++++++++++++++++-------
 2 files changed, 55 insertions(+), 10 deletions(-)

diff --git a/debian/changelog b/debian/changelog
index 397c0f5..975d955 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -20,6 +20,9 @@ dpkg (1.16.13) UNRELEASED; urgency=low
     setting package selections. Suggested by Klaus Ita <koki.eml at gmail.com>.
   * Recognize «start-stop-daemon -C» as documented. Closes: #719746
     Reported by Brian S. Julin <bri at abrij.org>.
+  * Correctly parse C-style diff filenames in Dpkg::Source::Patch, to avoid
+    directory traversal attempts from hostile source packages when unpacking
+    them. Reported by Jakub Wilk <jwilk at debian.org>. Fixes CVE-2014-0471.
 
   [ Updated scripts translations ]
   * Fix a typo in the German scripts translation.
diff --git a/scripts/Dpkg/Source/Patch.pm b/scripts/Dpkg/Source/Patch.pm
index da4f7c2..d9b084e 100644
--- a/scripts/Dpkg/Source/Patch.pm
+++ b/scripts/Dpkg/Source/Patch.pm
@@ -310,6 +310,56 @@ sub _fail_not_same_type {
     $self->register_error();
 }
 
+my %ESCAPE = ((
+    'a' => "\a",
+    'b' => "\b",
+    'f' => "\f",
+    'n' => "\n",
+    'r' => "\r",
+    't' => "\t",
+    'v' => "\cK",
+    '\\' => "\\",
+    '"' => "\"",
+), (
+    map { sprintf('%03o', $_) => chr($_) } (0..255)
+));
+
+sub _unescape {
+    my ($diff, $str) = @_;
+
+    if (exists $ESCAPE{$str}) {
+        return $ESCAPE{$str};
+    } else {
+        error(_g("diff %s patches file with unknown escape sequence \\%s"),
+              $diff, $str);
+    }
+}
+
+# Fetch the header filename ignoring the optional timestamp
+sub _fetch_filename {
+    my ($diff, $header) = @_;
+
+    # Strip any leading spaces.
+    $header =~ s/^\s+//;
+
+    # Is it a C-style string?
+    if ($header =~ m/^"/) {
+        $header =~ m/^"((?:[^\\"]|\\.)*)"/;
+        error(_g('diff %s patches file with unbalanced quote'), $diff)
+            unless defined $1;
+
+        $header = $1;
+        $header =~ s/\\([0-3][0-7]{2}|.)/_unescape($diff, $1)/eg;
+    } else {
+        # Tab is the official separator, it's always used when
+        # filename contain spaces. Try it first, otherwise strip on space
+        # if there's no tab
+        $header =~ s/\s.*// unless $header =~ s/\t.*//;
+    }
+
+    return $header;
+}
+
 # check diff for sanity, find directories to create as a side effect
 sub analyze {
     my ($self, $destdir, %opts) = @_;
@@ -332,14 +382,6 @@ sub analyze {
         }
         return $line;
     }
-    sub strip_ts { # Strip timestamp
-        my $header = shift;
-        # Tab is the official separator, it's always used when
-        # filename contain spaces. Try it first, otherwise strip on space
-        # if there's no tab
-        $header =~ s/\s.*// unless ($header =~ s/\t.*//);
-        return $header;
-    }
     sub intuit_file_patched {
 	my ($old, $new) = @_;
 	return $new unless defined $old;
@@ -389,7 +431,7 @@ sub analyze {
 	unless(s/^--- //) {
 	    error(_g("expected ^--- in line %d of diff `%s'"), $., $diff);
 	}
-        $path{'old'} = $_ = strip_ts($_);
+	$path{'old'} = $_ = _fetch_filename($diff, $_);
 	$fn{'old'} = $_ if $_ ne '/dev/null' and s{^[^/]*/+}{$destdir/};
 	if (/\.dpkg-orig$/) {
 	    error(_g("diff `%s' patches file with name ending .dpkg-orig"), $diff);
@@ -401,7 +443,7 @@ sub analyze {
 	unless (s/^\+\+\+ //) {
 	    error(_g("line after --- isn't as expected in diff `%s' (line %d)"), $diff, $.);
 	}
-        $path{'new'} = $_ = strip_ts($_);
+	$path{'new'} = $_ = _fetch_filename($diff, $_);
 	$fn{'new'} = $_ if $_ ne '/dev/null' and s{^[^/]*/+}{$destdir/};
 
 	unless (defined $fn{'old'} or defined $fn{'new'}) {

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/reproducible/dpkg.git