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

[SCM] Debian package checker branch, master, updated. 2.5.11-269-g4fa9095



The following commit has been merged in the master branch:
commit 173523983e2173292b9c9b45631959e9a8be9a06
Author: Niels Thykier <niels@thykier.net>
Date:   Sun Apr 7 23:12:45 2013 +0200

    c/{cruft,symlinks}: Check for unsafe symlinks
    
    Signed-off-by: Niels Thykier <niels@thykier.net>

diff --git a/checks/cruft b/checks/cruft
index cf474e2..843c8fc 100644
--- a/checks/cruft
+++ b/checks/cruft
@@ -36,7 +36,7 @@ use constant BLOCKSIZE => 4096;
 use Lintian::Data;
 use Lintian::Relation ();
 use Lintian::Tags qw(tag);
-use Lintian::Util qw(fail is_ancestor_of);
+use Lintian::Util qw(fail is_ancestor_of resolve_pkg_path);
 
 use Cwd;
 use File::Find;
@@ -356,11 +356,32 @@ sub find_cruft {
             }
         }
     }
-    -f or return; # we just need normal files for the rest
-    if (-l && !is_ancestor_of($info->unpacked, $_)) {
-        # skip unsafe symlinks too
-        return;
+    if (-l) {
+        my $target = readlink($_);
+        my $dirname = dirname($_);
+        my $resolved = resolve_pkg_path($dirname, $target);
+        if (not $resolved) {
+            # skip unsafe symlinks too
+            tag 'source-contains-unsafe-symlink', $_;
+            return;
+        }
+        # This check may appear redundant, but it is not!
+        # resolve_pkg_path tells us that the link can "safely be
+        # resolved without escaping the root".  But it tells us
+        # nothing about the target of the link (which could be an
+        # unsafe symlink). Example
+        #
+        #  safe-symlink -> unsafe-symlink
+        #  unsafe-symlink -> ../../../../etc/passwd
+        #
+        # resolve_pkg_path would approve of "safe-symlink", but if we
+        # were to open it we would actually end up escaping the
+        # package root.
+        if (-e and not is_ancestor_of($info->unpacked, $_)) {
+            return;
+        }
     }
+    -f or return; # we just need normal files for the rest
     $basename = basename $name;
 
     unless ($warned->{$name}) {
diff --git a/checks/cruft.desc b/checks/cruft.desc
index 1891b6b..9ce3078 100644
--- a/checks/cruft.desc
+++ b/checks/cruft.desc
@@ -509,3 +509,12 @@ Info: The given source file is licensed under GFDL, but without any
  .
 Ref: http://wiki.debian.org/qa.debian.org/gfdlinvariant,
  http://www.debian.org/vote/2006/vote_001
+
+Tag: source-contains-unsafe-symlink
+Severity: serious
+Certainty: possible
+Info: The source contains an unsafe symlink.  If followed, the link
+ will escape the source root.
+ .
+ If it is a part of the package's testsuite, Lintian may have failed
+ to recognise it as a test.  In that case, please override the tag.
diff --git a/checks/symlinks b/checks/symlinks
index 005dcf1..21b8087 100644
--- a/checks/symlinks
+++ b/checks/symlinks
@@ -40,12 +40,20 @@ foreach my $file ($info->sorted_index) {
         my $path; # the target (from the pkg root)
         # Should not happen (too often) - but just in case
         next unless $target;
+        $path = $index_info->link_resolved;
+        if (not defined $path) {
+            # Unresolvable link
+            tag 'package-contains-unsafe-symlink', $file;
+            next;
+        }
+        # Links to the package root is always going to exist (although
+        # self-recursive and possibly not very useful)
+        next if $path eq '';
+
         # Skip usr/share/doc/<pkg> - we got a separate check for
         # that.
         next if $file eq "usr/share/doc/$pkg";
-        $path = $index_info->link_resolved;
-        # skip unresolvable links and links to "/"
-        next unless $path;
+
 
         # Check if the destination is in the package itself
         next if $info->index ($path) || $info->index ("$path/");
diff --git a/checks/symlinks.desc b/checks/symlinks.desc
index 74ec81e..39d6f2d 100644
--- a/checks/symlinks.desc
+++ b/checks/symlinks.desc
@@ -20,3 +20,10 @@ Info: The package contains a symlink with a target that
  appears to be a "failed" wildcard expansion.  Furthermore
  the target does not exists in the package or any of its
  direct dependencies (built from the same source).
+
+Tag: package-contains-unsafe-symlink
+Severity: serious
+Certainty: certain
+Info: The package contains an unsafe symlink.  If followed,
+ the link will escape the package root.
+Ref: policy 10.5
diff --git a/debian/changelog b/debian/changelog
index 5b67cf1..6d79fdc 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -22,7 +22,9 @@ lintian (2.5.12) UNRELEASED; urgency=low
       - missing-runtime-test-file
       - missing-runtime-tests-field
       - package-contains-broken-symlink-wildcard
+      - package-contains-unsafe-symlink
       - runtime-test-file-is-not-a-regular-file
+      - source-contains-unsafe-symlink
       - unknown-runtime-tests-feature
       - unknown-runtime-tests-field
       - unknown-runtime-tests-restriction
@@ -65,6 +67,8 @@ lintian (2.5.12) UNRELEASED; urgency=low
     + [NT] Correct description of an autotools tag.  Thanks
       to Alberto Garcia and Timo Juhani Lindfors for the
       report and patch.  (Closes: #703490)
+    + [NT] Check for unsafe symlinks (outside common testsuite
+      paths).
   * checks/debconf:
     + [NT] Fix several path traversal issues that could leak
       information about the host system.
@@ -159,6 +163,7 @@ lintian (2.5.12) UNRELEASED; urgency=low
       (Closes: #683737)
     + [NT] Demote certainty of package-contains-broken-symlink to
       wild-guess.
+    + [NT] Check for unsafe symlinks in binary packages.
   * checks/testsuite{,.desc}:
     + [NT] New check written by Nicolas Boulenguez to catch some
       mistakes with the new autopkgtest tests.
diff --git a/t/source/debian-source-dir-traversal-1/tags b/t/source/debian-source-dir-traversal-1/tags
index e69de29..455947e 100644
--- a/t/source/debian-source-dir-traversal-1/tags
+++ b/t/source/debian-source-dir-traversal-1/tags
@@ -0,0 +1 @@
+E: debian-source-dir-traversal-1 source: source-contains-unsafe-symlink git-patches
diff --git a/t/tests/cruft-unsafe-symlinks/desc b/t/tests/cruft-unsafe-symlinks/desc
new file mode 100644
index 0000000..8ee8f52
--- /dev/null
+++ b/t/tests/cruft-unsafe-symlinks/desc
@@ -0,0 +1,5 @@
+Testname: cruft-unsafe-symlinks
+Sequence: 6000
+Version: 1.0
+Description: General tests of unsafe symlinks
+Test-For: source-contains-unsafe-symlink
diff --git a/t/tests/cruft-unsafe-symlinks/pre_build b/t/tests/cruft-unsafe-symlinks/pre_build
new file mode 100755
index 0000000..a70c811
--- /dev/null
+++ b/t/tests/cruft-unsafe-symlinks/pre_build
@@ -0,0 +1,12 @@
+#!/bin/sh
+SOURCE_ROOT="$1"
+LINK_TARGET=non-existant/path/lintian-should-not-open
+
+
+mkdir -p "$SOURCE_ROOT/bad-symlinks" "$SOURCE_ROOT/tests/"
+ln -s "../../$LINK_TARGET" "$SOURCE_ROOT/bad-symlinks/relative-escape"
+ln -s "/../../$LINK_TARGET" "$SOURCE_ROOT/bad-symlinks/absolute-escape"
+
+# Lintian ignores these - not sure to what extend it makes sense
+ln -s "../../$LINK_TARGET" "$SOURCE_ROOT/tests/relative-escape-but-ignored"
+ln -s "/../../$LINK_TARGET" "$SOURCE_ROOT/tests/absolute-escape-but-ignored"
diff --git a/t/tests/cruft-unsafe-symlinks/tags b/t/tests/cruft-unsafe-symlinks/tags
new file mode 100644
index 0000000..7474d70
--- /dev/null
+++ b/t/tests/cruft-unsafe-symlinks/tags
@@ -0,0 +1,2 @@
+E: cruft-unsafe-symlinks source: source-contains-unsafe-symlink absolute-escape
+E: cruft-unsafe-symlinks source: source-contains-unsafe-symlink relative-escape
diff --git a/t/tests/files-symlinks/tags b/t/tests/files-symlinks/tags
index 15ba7e3..df7d272 100644
--- a/t/tests/files-symlinks/tags
+++ b/t/tests/files-symlinks/tags
@@ -1,4 +1,5 @@
 E: files-symlinks: compressed-symlink-with-wrong-ext usr/lib/lintian/data/comp-data ../../../share/lintian/data/comp-data.gz
+E: files-symlinks: package-contains-unsafe-symlink usr/share/lintian/data/pkg.conf
 E: files-symlinks: symlink-contains-spurious-segments usr/share/lintian/data/spurious ../../lintian-old/../lintian/data/data-file
 E: files-symlinks: symlink-has-too-many-up-segments usr/share/lintian/data/pkg.conf ../..//..//..//../etc/lintian/pkg.conf
 E: files-symlinks: symlink-should-be-absolute usr/share/lintian/data/pkg-old.conf ../../../../etc/lintian/pkg.conf
diff --git a/t/tests/symlinks-unsafe/debian/debian/rules b/t/tests/symlinks-unsafe/debian/debian/rules
new file mode 100644
index 0000000..2b12987
--- /dev/null
+++ b/t/tests/symlinks-unsafe/debian/debian/rules
@@ -0,0 +1,14 @@
+#!/usr/bin/make -f
+
+PKG:=$(shell dh_listpackages)
+LINK_DIR:=debian/$(PKG)/usr/share/lintian/bad-links
+LINK_TARGET:=non-existant/path/lintian-should-not-open
+
+%:
+	dh $@
+
+override_dh_link:
+	mkdir -p "$(LINK_DIR)"
+	ln -s ../../../../../$(LINK_TARGET) \
+		$(LINK_DIR)/relative-escape
+	ln -s /../$(LINK_TARGET) $(LINK_DIR)/absolute
diff --git a/t/tests/symlinks-unsafe/desc b/t/tests/symlinks-unsafe/desc
new file mode 100644
index 0000000..c1a7dc7
--- /dev/null
+++ b/t/tests/symlinks-unsafe/desc
@@ -0,0 +1,5 @@
+Testname: symlinks-unsafe
+Sequence: 6000
+Version: 1.0
+Description: General tests of unsafe symlinks
+Test-For: package-contains-unsafe-symlink
diff --git a/t/tests/symlinks-unsafe/tags b/t/tests/symlinks-unsafe/tags
new file mode 100644
index 0000000..f3610a2
--- /dev/null
+++ b/t/tests/symlinks-unsafe/tags
@@ -0,0 +1,3 @@
+E: symlinks-unsafe: package-contains-unsafe-symlink usr/share/lintian/bad-links/absolute
+E: symlinks-unsafe: package-contains-unsafe-symlink usr/share/lintian/bad-links/relative-escape
+E: symlinks-unsafe: symlink-has-too-many-up-segments usr/share/lintian/bad-links/relative-escape ../../../../../non-existant/path/lintian-should-not-open
diff --git a/testset/tags.filenames b/testset/tags.filenames
index 7fe8258..cc17520 100644
--- a/testset/tags.filenames
+++ b/testset/tags.filenames
@@ -13,6 +13,7 @@ E: filenames: lengthy-symlink usr/share/doc/filenames/version.txt.gz ../filename
 E: filenames: no-copyright-file
 E: filenames: non-standard-toplevel-dir files/
 E: filenames: package-contains-ancient-file usr/lib/perl5/foo/ancient.pm 1975-01-01
+E: filenames: package-contains-unsafe-symlink usr/lib/filenames/symlink2wrong
 E: filenames: package-installs-file-to-usr-x11r6 usr/X11R6/
 E: filenames: package-installs-file-to-usr-x11r6 usr/X11R6/bin/
 E: filenames: package-installs-file-to-usr-x11r6 usr/X11R6/bin/testxbin2

-- 
Debian package checker


Reply to: