[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#563773: lintian: false positive if a patch in debian/patches is a symlink



-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256


Hi

I have devised a patch for this; currently Lintian ignores all patches
outside debian/, with this it will accept one-level of indirection.
That is symlink -> patch (but not symlink -> symlink -> patch).
  The patch can be extended to do that, but I could not wrap my head
around during it correctly without relying on unpacked being present.
Since failure here would give us another CVE I decided to keep it simple.

That being said, I would still like a double check on this before I
apply it.

Thank you in advance,
~Niels

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBCAAGBQJN25rcAAoJEAVLu599gGRCCj8P/0CNi1y2DrFtlynBUYx9Y7O2
AH4Gc8ArQDd5jJOaFFVWMSDYBaEt80+VD4765WEzNgAojxGFoCxdjuagzFVGkw49
Uzvq+QwvT3PreVDZ4rXRzcRc+4VLjNwna3EyU9wFkgVA+xtOCFbFr6o5A+f7SsOP
QucBbPAAXVNMTMW0tnKQgBKxH/XtNemOlba3NPfvUjwMdc+xoYKEv1bKkBTVuM5D
0gXU2HRDOMH2XmyX+MTpZhMiB58c5fDHeMzGcrm4TkhAvs//l8J+wyKaKH8i0H8K
jFcRgK5IGP1DZhaIsBI++Rylaaa3DEwlhqT+aTVXO0i526cPN3ne2fORLmkAaSt1
j8LB9hqX+DmcXa8nSbCbAy9WCwffYAljYmC7XzD+do8yyo6RUi81MQTRtA7KIUkc
baBcxawDJ+7Uye3/dnsxH9T7AgUd+0W/6NbYuW1z1XZnnwa81f+GVrkl7gC1GI34
q+2BKCq0ElGD/F48LgkCA/qFUYzz5CKJSmAfPFZ3crc2GRrf1QuGBh8HFcY+Y7ej
eydvhc1U+2GIJr3vuiAOmJQwZL/0pokRqGJuiVRAx6WKEYLO4j75CNYUSn5GVlci
Kvrewn50uAlGI0dw0r3ytJTzJQWLNOoXQrAljkddPHOr2kwmnoocGUWNGGhdLcNg
TQt2wz72iTuPFVB3OmlV
=g9Ta
-----END PGP SIGNATURE-----
>From b54102d5c418368833e977e17d6a4dfee593f0ab Mon Sep 17 00:00:00 2001
From: Niels Thykier <niels@thykier.net>
Date: Tue, 24 May 2011 13:36:25 +0200
Subject: [PATCH] Allow quilt patches to be symlinks outside debian/ in some
 cases

Allow patches directly in debian/patches/ to be symlinks to other
files, if it is a valid target in the source package.  The logic
in this patch does not handle symlinks to symlinks or if the
original patch is outside debian/patches (e.g. p/my-patch.patch
in the series file)
---
 checks/patch-systems                               |   76 ++++++++++++++++----
 debian/changelog                                   |    4 +
 .../debian/debian/patches/series                   |    3 +
 t/tests/patch-systems-quilt-general/tags           |    1 +
 .../debian/debian/control.in                       |   15 ++++
 .../debian/debian/patches/series                   |    1 +
 .../debian/debian/source/format                    |    1 +
 .../patch-systems-quilt-source-symlink-patch/desc  |    6 ++
 .../pre_build                                      |    5 ++
 .../upstream/README                                |    1 +
 .../upstream/source.patch                          |    6 ++
 11 files changed, 106 insertions(+), 13 deletions(-)
 create mode 100644 t/tests/patch-systems-quilt-source-symlink-patch/debian/debian/control.in
 create mode 100644 t/tests/patch-systems-quilt-source-symlink-patch/debian/debian/patches/series
 create mode 100644 t/tests/patch-systems-quilt-source-symlink-patch/debian/debian/source/format
 create mode 100644 t/tests/patch-systems-quilt-source-symlink-patch/desc
 create mode 100755 t/tests/patch-systems-quilt-source-symlink-patch/pre_build
 create mode 100644 t/tests/patch-systems-quilt-source-symlink-patch/tags
 create mode 100644 t/tests/patch-systems-quilt-source-symlink-patch/upstream/README
 create mode 100644 t/tests/patch-systems-quilt-source-symlink-patch/upstream/source.patch

diff --git a/checks/patch-systems b/checks/patch-systems
index 515561c..72ec09f 100644
--- a/checks/patch-systems
+++ b/checks/patch-systems
@@ -109,7 +109,7 @@ sub run {
 							tag "dpatch-missing-description", $patch_file;
 						}
 					}
-					check_patch($patch_file);
+					check_patch($patch_file, "debfiles/patches/$patch_file");
 				}
 			}
 		}
@@ -147,15 +147,33 @@ sub run {
 
 				# Check each patch.
 				foreach my $patch_file (@patches) {
-					next if ( -l "debfiles/patches/$patch_file" );
-					unless (realpath("debfiles/patches/$patch_file") =~ m,^\Q$cwd\E/debfiles/,) {
-					    next;
+					my $patch_path;
+					if ( $patch_file =~ m,/,o) {
+					    # The patch itself is in another directory.
+					    my $path = resolve_patch('debian/patches', $patch_file);
+					    next unless $path; # cannot be safely resolved - bail
+					    ## FIXME: According to #562155, quilt does not correctly
+					    ##   handle ../ patches, perhaps we should warn about that?
+					    $patch_path = $path;
+					} else {
+					    # Straight forward simple patch in d/patches/
+					    $patch_path = "debfiles/patches/$patch_file";
+					    if ( -l $patch_path ) {
+						my $target = readlink("debfiles/patches/$patch_file");
+						my $path = resolve_patch('debian/patches', $target);
+						next unless $path;
+						$patch_path = $path;
+					    }
 					}
-					if (! -r "debfiles/patches/$patch_file") {
+					# Bail if this is a link - the logic above is not strong enough to
+					# handle a link to a link or a "../../path/to/symlink" in
+					# d/patches/series
+					next if -l $patch_path;
+					if (! -r $patch_path) {
 						tag "quilt-series-references-non-existent-patch", $patch_file;
 						next;
 					}
-					if (open(PATCH_FILE, '<', "debfiles/patches/$patch_file")) {
+					if (open(PATCH_FILE, '<', $patch_path)) {
 						my $has_description = 0;
 						while (<PATCH_FILE>) {
 							# stop if something looking like a patch starts:
@@ -169,7 +187,7 @@ sub run {
 							tag "quilt-patch-missing-description", $patch_file;
 						}
 					}
-					check_patch($patch_file);
+					check_patch($patch_file, $patch_path);
 				}
 			}
 		}
@@ -217,8 +235,8 @@ sub run {
 }
 
 # Checks on patches common to all build systems.
-sub check_patch($) {
-	my $patch_file = shift;
+sub check_patch {
+	my ($patch_file, $patch_path) = @_;
 
 	# Use -p1 to strip off the first layer of directory in case the parent
 	# directory in which the patches were generated was named "debian".
@@ -226,17 +244,49 @@ sub check_patch($) {
 	# in the debian/* directory, but as of 2010-01-01, all cases where the
 	# first level of the patch path is "debian/" in the archive are false
 	# positives.
-	open(DIFFSTAT, "-|", 'diffstat', '-p1', '-l', "debfiles/patches/$patch_file")
-	  or fail("can't fork diffstat");
+	open(DIFFSTAT, '-|', 'diffstat', '-p1', '-l', $patch_path)
+	  or fail("cannot fork diffstat: $!");
 	while (<DIFFSTAT>) {
 		chomp;
-		if (m|^(\./)?debian/|) {
-			tag "patch-modifying-debian-files", $patch_file, $_;
+		if (m|^(?:\./)?debian/|o) {
+			tag 'patch-modifying-debian-files', $patch_file, $_;
 		}
 	}
 	close(DIFFSTAT) or fail("cannot close pipe to diffstat on $patch_file: $!");
 }
 
+# Resolve a patch in $dir pointing to $target
+#  - resolve_patch($dir, $target);
+#  - resolve_patch('debian/patches', 'p/my-patch.diff');
+#     gives 'debfiles/patches/p/my-patch.diff'
+#  - resolve_patch('debian/patches', '../../my-patch.diff');
+#     gives 'unpacked/my-patch.diff' (if unpacked is present)
+#
+#  Returns a path into debfiles/ if the target is inside debian/
+#  Returns a path into unpacked/ if the target is outside debian/
+#   but can safely be resolved AND unpacked is a directory.
+#  Returns a non-truth value otherwise.
+sub resolve_patch {
+    my ($dir, $target) = @_;
+    my $path = resolve_pkg_path($dir, $target);
+    return '' unless $path; # cannot be safely resolved - bail
+    if ($path =~ m,^debian/,o){
+	# The patch is inside debian/, so we can find it in
+	# debfiles
+	$path =~ s,debian/,debfiles/,o;
+    } else {
+	# The patch is in the upstream part, which we cannot check
+	# unless 'unpacked' is here.
+	# - Usually it will be since debfiles depend on unpacked
+	#   but if we are re-run in a lab without being allowed to use unpacked
+	#   it might have been auto-removed.
+	return '' unless -d 'unpacked/';
+	$path = "unpacked/$path";
+    }
+    return $path;
+}
+
+
 1;
 
 # Local Variables:
diff --git a/debian/changelog b/debian/changelog
index 459ae78..380941f 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -6,6 +6,10 @@ lintian (2.5.1) UNRELEASED; urgency=low
   * checks/java:
     + [NT] Sort the jar files by name, so they are checked in the same
       order.
+  * checks/patch-systems:
+    + [NT] Check symlinked patches that can safely be resolved.  This
+      allows the use of symlinks in debian/patches/ to point to a patch
+      in the upstream part of the source package.  (Closes: #563773)
 
   * collection/unpacked:
     + [NT] Added support for unpacking source packages using libdpkg-perl,
diff --git a/t/tests/patch-systems-quilt-general/debian/debian/patches/series b/t/tests/patch-systems-quilt-general/debian/debian/patches/series
index edea10a..9406f3d 100644
--- a/t/tests/patch-systems-quilt-general/debian/debian/patches/series
+++ b/t/tests/patch-systems-quilt-general/debian/debian/patches/series
@@ -3,4 +3,7 @@ some-other-file -p0
 some-nonexistent-patch
 # this is to test for directory traversals
 # 'debian-patch' is a file in the laboratory
+#  - older versions of Lintian would read the
+#    file from the laboratory rather than
+#    realizing that the patch does not exist.
 ../../debian-patch
diff --git a/t/tests/patch-systems-quilt-general/tags b/t/tests/patch-systems-quilt-general/tags
index 38e023f..403605f 100644
--- a/t/tests/patch-systems-quilt-general/tags
+++ b/t/tests/patch-systems-quilt-general/tags
@@ -1,4 +1,5 @@
 E: patch-systems-quilt-general source: patch-modifying-debian-files some-file debian/some-file
+E: patch-systems-quilt-general source: quilt-series-references-non-existent-patch ../../debian-patch
 E: patch-systems-quilt-general source: quilt-series-references-non-existent-patch some-nonexistent-patch
 W: patch-systems-quilt-general source: dpatch-build-dep-but-no-patch-list
 W: patch-systems-quilt-general source: more-than-one-patch-system
diff --git a/t/tests/patch-systems-quilt-source-symlink-patch/debian/debian/control.in b/t/tests/patch-systems-quilt-source-symlink-patch/debian/debian/control.in
new file mode 100644
index 0000000..07c8950
--- /dev/null
+++ b/t/tests/patch-systems-quilt-source-symlink-patch/debian/debian/control.in
@@ -0,0 +1,15 @@
+Source: {$srcpkg}
+Priority: extra
+Section: {$section}
+Maintainer: {$author}
+Standards-Version: {$standards_version}
+Build-Depends: debhelper (>= 7.0.50~), quilt
+
+Package: {$srcpkg}
+Architecture: {$architecture}
+Depends: $\{shlibs:Depends\}, $\{misc:Depends\}
+Description: {$description}
+ This is a test package designed to exercise some feature or tag of
+ Lintian.  It is part of the Lintian test suite and may do very odd
+ things.  It should not be installed like a regular package.  It may
+ be an empty package.
diff --git a/t/tests/patch-systems-quilt-source-symlink-patch/debian/debian/patches/series b/t/tests/patch-systems-quilt-source-symlink-patch/debian/debian/patches/series
new file mode 100644
index 0000000..860a818
--- /dev/null
+++ b/t/tests/patch-systems-quilt-source-symlink-patch/debian/debian/patches/series
@@ -0,0 +1 @@
+upstream-patch-symlink.patch
diff --git a/t/tests/patch-systems-quilt-source-symlink-patch/debian/debian/source/format b/t/tests/patch-systems-quilt-source-symlink-patch/debian/debian/source/format
new file mode 100644
index 0000000..163aaf8
--- /dev/null
+++ b/t/tests/patch-systems-quilt-source-symlink-patch/debian/debian/source/format
@@ -0,0 +1 @@
+3.0 (quilt)
diff --git a/t/tests/patch-systems-quilt-source-symlink-patch/desc b/t/tests/patch-systems-quilt-source-symlink-patch/desc
new file mode 100644
index 0000000..cac9934
--- /dev/null
+++ b/t/tests/patch-systems-quilt-source-symlink-patch/desc
@@ -0,0 +1,6 @@
+Testname: patch-systems-quilt-source-symlink-patch
+Sequence: 6000
+Type: non-native
+Version: 1.0-1
+Test-Against: quilt-series-references-non-existent-patch
+Description: Test for valid symlink quilt patches
diff --git a/t/tests/patch-systems-quilt-source-symlink-patch/pre_build b/t/tests/patch-systems-quilt-source-symlink-patch/pre_build
new file mode 100755
index 0000000..1d8457d
--- /dev/null
+++ b/t/tests/patch-systems-quilt-source-symlink-patch/pre_build
@@ -0,0 +1,5 @@
+#!/bin/sh
+
+# generate the symlink
+DIR="$1"
+ln -s ../../source.patch "$DIR/debian/patches/upstream-patch-symlink.patch"
diff --git a/t/tests/patch-systems-quilt-source-symlink-patch/tags b/t/tests/patch-systems-quilt-source-symlink-patch/tags
new file mode 100644
index 0000000..e69de29
diff --git a/t/tests/patch-systems-quilt-source-symlink-patch/upstream/README b/t/tests/patch-systems-quilt-source-symlink-patch/upstream/README
new file mode 100644
index 0000000..e845566
--- /dev/null
+++ b/t/tests/patch-systems-quilt-source-symlink-patch/upstream/README
@@ -0,0 +1 @@
+README
diff --git a/t/tests/patch-systems-quilt-source-symlink-patch/upstream/source.patch b/t/tests/patch-systems-quilt-source-symlink-patch/upstream/source.patch
new file mode 100644
index 0000000..79bf224
--- /dev/null
+++ b/t/tests/patch-systems-quilt-source-symlink-patch/upstream/source.patch
@@ -0,0 +1,6 @@
+Description: Wanna-be patch from upstream
+
+--- /dev/null	2011-05-24 08:55:59.948027011 +0200
++++ b/source-file.txt	2011-05-24 11:58:50.466426869 +0200
+@@ -0,0 +1 @@
++Patch me, patch me!
-- 
1.7.4.4

Attachment: 0001-Allow-quilt-patches-to-be-symlinks-outside-debian-in.patch.sig
Description: Binary data


Reply to: