Просмотр исходного кода

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@debian.org>
Guillem Jover лет назад: 12
Родитель
Сommit
a826511884
2 измененных файлов с 52 добавлено и 10 удалено
  1. 3 0
      debian/changelog
  2. 49 10
      scripts/Dpkg/Source/Patch.pm

+ 3 - 0
debian/changelog

@@ -17,6 +17,9 @@ dpkg (1.17.8) UNRELEASED; urgency=low
     in stable.
   * Enable failed test case reporting from the TAP::Harness, so that we get
     more meaningful reports on failure from the C test suite.
+  * 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@debian.org>. Fixes CVE-2014-0471.
 
   [ Updated programs translations ]
   * German (Sven Joachim).

+ 49 - 10
scripts/Dpkg/Source/Patch.pm

@@ -332,14 +332,53 @@ sub _getline {
     return $line;
 }
 
-# Strip timestamp
-sub _strip_ts {
-    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.*//);
+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;
 }
 
@@ -408,7 +447,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);
@@ -420,7 +459,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}) {