[gentoo-commits] proj/sandbox:master commit in: libsandbox/, tests/

["Mike Frysinger" <vapier@gentoo.org>]

commit:     93d401570d4e54f732c0f821cdbb5ba2e1dee0f3
Author:     Mike Frysinger <vapier <AT> gentoo <DOT> org>
AuthorDate: Sun Sep 20 07:23:37 2015 +0000
Commit:     Mike Frysinger <vapier <AT> gentoo <DOT> org>
CommitDate: Sun Sep 20 07:23:37 2015 +0000
URL:        https://gitweb.gentoo.org/proj/sandbox.git/commit/?id=93d40157

libsandbox: fix handling of dangling symlinks

Make sure we properly check the target of symlinks even when the target
does not exist.  This caused problems in two ways:
(1) It allowed code to bypass checks by writing through a symlink that
was in a good location but pointed to a bad (non-existent) location.
(2) It caused code to be wrongly rejected when it tried writing to a
symlink in a bad location but pointed to a good location.

In order to get this behavior, we need to use the new gnulib helpers
added in the previous commit.  They include functions which can look
up the targets of symlinks even when the final path doesn't exist.

URL: https://bugs.gentoo.org/540828
Reported-by: Rick Farina <zerochaos <AT> gentoo.org>
Signed-off-by: Mike Frysinger <vapier <AT> gentoo.org>

 libsandbox/libsandbox.c | 23 ++++++++++++++++++-----
 tests/script-11.sh      | 19 +++++++++++++++++++
 tests/script-12.sh      | 25 +++++++++++++++++++++++++
 tests/script.at         |  2 ++
 4 files changed, 64 insertions(+), 5 deletions(-)

diff --git a/libsandbox/libsandbox.c b/libsandbox/libsandbox.c
index 3bd3794..57be731 100644
--- a/libsandbox/libsandbox.c
+++ b/libsandbox/libsandbox.c
@@ -240,7 +240,12 @@ static char *resolve_path(const char *path, int follow_link)
 		else
 			ret = realpath(path, filtered_path);
 
-		/* Maybe we failed because of funky anonymous fd symlinks.
+		/* Handle broken symlinks.  This can come up for a variety of reasons,
+		 * but we need to make sure that we resolve the path all the way to the
+		 * final target, and not just where the current link happens to start.
+		 * Latest discussion is in #540828.
+		 *
+		 * Maybe we failed because of funky anonymous fd symlinks.
 		 * You can see this by doing something like:
 		 *		$ echo | ls -l /proc/self/fd/
 		 *		.......	0 -> pipe:[9422999]
@@ -248,11 +253,19 @@ static char *resolve_path(const char *path, int follow_link)
 		 * actual file paths for us to check against. #288863
 		 * Don't look for any particular string as these are dynamic
 		 * according to the kernel.  You can see pipe:, socket:, etc...
+		 *
+		 * Maybe we failed because it's a symlink to a path in /proc/ that
+		 * is a symlink to a path that longer exists -- readlink will set
+		 * ENOENT even in that case and the file ends in (deleted).  This
+		 * can come up in cases like:
+		 * /dev/stderr -> fd/2 -> /proc/self/fd/2 -> /removed/file (deleted)
 		 */
-		if (!ret && !strncmp(filtered_path, "/proc/", 6)) {
-			char *base = strrchr(filtered_path, '/');
-			if (base && strchr(base, ':'))
-				ret = filtered_path;
+		if (!ret && errno == ENOENT) {
+			ret = canonicalize_filename_mode(path, CAN_ALL_BUT_LAST);
+			if (ret) {
+				free(filtered_path);
+				filtered_path = ret;
+			}
 		}
 
 		if (!ret) {

diff --git a/tests/script-11.sh b/tests/script-11.sh
new file mode 100755
index 0000000..da9bbbf
--- /dev/null
+++ b/tests/script-11.sh
@@ -0,0 +1,19 @@
+#!/bin/sh
+# handle targets of dangling symlinks correctly #540828
+[ "${at_xfail}" = "yes" ] && exit 77 # see script-0
+
+# this should fail
+mkdir subdir
+ln -s subdir/target symlink
+
+adddeny "${PWD}/subdir"
+
+echo blah >symlink
+# we should not be able to write through the symlink
+if [ $? -eq 0 ] ; then
+	exit 1
+fi
+
+test -s "${SANDBOX_LOG}"
+
+exit $?

diff --git a/tests/script-12.sh b/tests/script-12.sh
new file mode 100755
index 0000000..a80108b
--- /dev/null
+++ b/tests/script-12.sh
@@ -0,0 +1,25 @@
+#!/bin/sh
+# handle targets of dangling symlinks correctly #540828
+[ "${at_xfail}" = "yes" ] && exit 77 # see script-0
+
+# this should pass
+mkdir subdir
+ln -s subdir/target symlink
+
+# make sure the log is in a writable location
+SANDBOX_LOG="${PWD}/subdir/log"
+
+(
+# This clobbers all existing writable paths for this one write.
+SANDBOX_WRITE="${PWD}/subdir"
+echo pass >symlink
+)
+# we should be able to write through the symlink
+if [ $? -ne 0 ] ; then
+	exit 1
+fi
+
+# and not gotten a sandbox violation
+test ! -s "${SANDBOX_LOG}"
+
+exit $?

diff --git a/tests/script.at b/tests/script.at
index 93e370a..f07a8f1 100644
--- a/tests/script.at
+++ b/tests/script.at
@@ -8,3 +8,5 @@ SB_CHECK(7)
 SB_CHECK(8)
 SB_CHECK(9, [wait errpipe... done OK!])
 SB_CHECK(10)
+SB_CHECK(11)
+SB_CHECK(12)