From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from lists.gentoo.org (pigeon.gentoo.org [208.92.234.80]) by finch.gentoo.org (Postfix) with ESMTP id 7A6071384AE for ; Sun, 20 Sep 2015 08:15:41 +0000 (UTC) Received: from pigeon.gentoo.org (localhost [127.0.0.1]) by pigeon.gentoo.org (Postfix) with SMTP id 3AF7D21C0BC; Sun, 20 Sep 2015 08:15:33 +0000 (UTC) Received: from smtp.gentoo.org (smtp.gentoo.org [140.211.166.183]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by pigeon.gentoo.org (Postfix) with ESMTPS id 296AE21C0BA for ; Sun, 20 Sep 2015 08:15:32 +0000 (UTC) Received: from oystercatcher.gentoo.org (unknown [IPv6:2a01:4f8:202:4333:225:90ff:fed9:fc84]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by smtp.gentoo.org (Postfix) with ESMTPS id 46372340887 for ; Sun, 20 Sep 2015 08:15:31 +0000 (UTC) Received: from localhost.localdomain (localhost [127.0.0.1]) by oystercatcher.gentoo.org (Postfix) with ESMTP id 1CAC220B for ; Sun, 20 Sep 2015 08:15:28 +0000 (UTC) From: "Mike Frysinger" To: gentoo-commits@lists.gentoo.org Content-Transfer-Encoding: 8bit Content-type: text/plain; charset=UTF-8 Reply-To: gentoo-dev@lists.gentoo.org, "Mike Frysinger" Message-ID: <1442733817.93d401570d4e54f732c0f821cdbb5ba2e1dee0f3.vapier@gentoo> Subject: [gentoo-commits] proj/sandbox:master commit in: libsandbox/, tests/ X-VCS-Repository: proj/sandbox X-VCS-Files: libsandbox/libsandbox.c tests/script-11.sh tests/script-12.sh tests/script.at X-VCS-Directories: libsandbox/ tests/ X-VCS-Committer: vapier X-VCS-Committer-Name: Mike Frysinger X-VCS-Revision: 93d401570d4e54f732c0f821cdbb5ba2e1dee0f3 X-VCS-Branch: master Date: Sun, 20 Sep 2015 08:15:28 +0000 (UTC) Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-Id: Gentoo Linux mail X-BeenThere: gentoo-commits@lists.gentoo.org X-Archives-Salt: 7d448d9e-98b5-42fb-a540-6264098c63f7 X-Archives-Hash: fc317882908752052111592a787287d9 commit: 93d401570d4e54f732c0f821cdbb5ba2e1dee0f3 Author: Mike Frysinger gentoo org> AuthorDate: Sun Sep 20 07:23:37 2015 +0000 Commit: Mike Frysinger gentoo 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 gentoo.org> Signed-off-by: Mike Frysinger 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)