| 1 |
commit: 93d401570d4e54f732c0f821cdbb5ba2e1dee0f3 |
| 2 |
Author: Mike Frysinger <vapier <AT> gentoo <DOT> org> |
| 3 |
AuthorDate: Sun Sep 20 07:23:37 2015 +0000 |
| 4 |
Commit: Mike Frysinger <vapier <AT> gentoo <DOT> org> |
| 5 |
CommitDate: Sun Sep 20 07:23:37 2015 +0000 |
| 6 |
URL: https://gitweb.gentoo.org/proj/sandbox.git/commit/?id=93d40157 |
| 7 |
|
| 8 |
libsandbox: fix handling of dangling symlinks |
| 9 |
|
| 10 |
Make sure we properly check the target of symlinks even when the target |
| 11 |
does not exist. This caused problems in two ways: |
| 12 |
(1) It allowed code to bypass checks by writing through a symlink that |
| 13 |
was in a good location but pointed to a bad (non-existent) location. |
| 14 |
(2) It caused code to be wrongly rejected when it tried writing to a |
| 15 |
symlink in a bad location but pointed to a good location. |
| 16 |
|
| 17 |
In order to get this behavior, we need to use the new gnulib helpers |
| 18 |
added in the previous commit. They include functions which can look |
| 19 |
up the targets of symlinks even when the final path doesn't exist. |
| 20 |
|
| 21 |
URL: https://bugs.gentoo.org/540828 |
| 22 |
Reported-by: Rick Farina <zerochaos <AT> gentoo.org> |
| 23 |
Signed-off-by: Mike Frysinger <vapier <AT> gentoo.org> |
| 24 |
|
| 25 |
libsandbox/libsandbox.c | 23 ++++++++++++++++++----- |
| 26 |
tests/script-11.sh | 19 +++++++++++++++++++ |
| 27 |
tests/script-12.sh | 25 +++++++++++++++++++++++++ |
| 28 |
tests/script.at | 2 ++ |
| 29 |
4 files changed, 64 insertions(+), 5 deletions(-) |
| 30 |
|
| 31 |
diff --git a/libsandbox/libsandbox.c b/libsandbox/libsandbox.c |
| 32 |
index 3bd3794..57be731 100644 |
| 33 |
--- a/libsandbox/libsandbox.c |
| 34 |
+++ b/libsandbox/libsandbox.c |
| 35 |
@@ -240,7 +240,12 @@ static char *resolve_path(const char *path, int follow_link) |
| 36 |
else |
| 37 |
ret = realpath(path, filtered_path); |
| 38 |
|
| 39 |
- /* Maybe we failed because of funky anonymous fd symlinks. |
| 40 |
+ /* Handle broken symlinks. This can come up for a variety of reasons, |
| 41 |
+ * but we need to make sure that we resolve the path all the way to the |
| 42 |
+ * final target, and not just where the current link happens to start. |
| 43 |
+ * Latest discussion is in #540828. |
| 44 |
+ * |
| 45 |
+ * Maybe we failed because of funky anonymous fd symlinks. |
| 46 |
* You can see this by doing something like: |
| 47 |
* $ echo | ls -l /proc/self/fd/ |
| 48 |
* ....... 0 -> pipe:[9422999] |
| 49 |
@@ -248,11 +253,19 @@ static char *resolve_path(const char *path, int follow_link) |
| 50 |
* actual file paths for us to check against. #288863 |
| 51 |
* Don't look for any particular string as these are dynamic |
| 52 |
* according to the kernel. You can see pipe:, socket:, etc... |
| 53 |
+ * |
| 54 |
+ * Maybe we failed because it's a symlink to a path in /proc/ that |
| 55 |
+ * is a symlink to a path that longer exists -- readlink will set |
| 56 |
+ * ENOENT even in that case and the file ends in (deleted). This |
| 57 |
+ * can come up in cases like: |
| 58 |
+ * /dev/stderr -> fd/2 -> /proc/self/fd/2 -> /removed/file (deleted) |
| 59 |
*/ |
| 60 |
- if (!ret && !strncmp(filtered_path, "/proc/", 6)) { |
| 61 |
- char *base = strrchr(filtered_path, '/'); |
| 62 |
- if (base && strchr(base, ':')) |
| 63 |
- ret = filtered_path; |
| 64 |
+ if (!ret && errno == ENOENT) { |
| 65 |
+ ret = canonicalize_filename_mode(path, CAN_ALL_BUT_LAST); |
| 66 |
+ if (ret) { |
| 67 |
+ free(filtered_path); |
| 68 |
+ filtered_path = ret; |
| 69 |
+ } |
| 70 |
} |
| 71 |
|
| 72 |
if (!ret) { |
| 73 |
|
| 74 |
diff --git a/tests/script-11.sh b/tests/script-11.sh |
| 75 |
new file mode 100755 |
| 76 |
index 0000000..da9bbbf |
| 77 |
--- /dev/null |
| 78 |
+++ b/tests/script-11.sh |
| 79 |
@@ -0,0 +1,19 @@ |
| 80 |
+#!/bin/sh |
| 81 |
+# handle targets of dangling symlinks correctly #540828 |
| 82 |
+[ "${at_xfail}" = "yes" ] && exit 77 # see script-0 |
| 83 |
+ |
| 84 |
+# this should fail |
| 85 |
+mkdir subdir |
| 86 |
+ln -s subdir/target symlink |
| 87 |
+ |
| 88 |
+adddeny "${PWD}/subdir" |
| 89 |
+ |
| 90 |
+echo blah >symlink |
| 91 |
+# we should not be able to write through the symlink |
| 92 |
+if [ $? -eq 0 ] ; then |
| 93 |
+ exit 1 |
| 94 |
+fi |
| 95 |
+ |
| 96 |
+test -s "${SANDBOX_LOG}" |
| 97 |
+ |
| 98 |
+exit $? |
| 99 |
|
| 100 |
diff --git a/tests/script-12.sh b/tests/script-12.sh |
| 101 |
new file mode 100755 |
| 102 |
index 0000000..a80108b |
| 103 |
--- /dev/null |
| 104 |
+++ b/tests/script-12.sh |
| 105 |
@@ -0,0 +1,25 @@ |
| 106 |
+#!/bin/sh |
| 107 |
+# handle targets of dangling symlinks correctly #540828 |
| 108 |
+[ "${at_xfail}" = "yes" ] && exit 77 # see script-0 |
| 109 |
+ |
| 110 |
+# this should pass |
| 111 |
+mkdir subdir |
| 112 |
+ln -s subdir/target symlink |
| 113 |
+ |
| 114 |
+# make sure the log is in a writable location |
| 115 |
+SANDBOX_LOG="${PWD}/subdir/log" |
| 116 |
+ |
| 117 |
+( |
| 118 |
+# This clobbers all existing writable paths for this one write. |
| 119 |
+SANDBOX_WRITE="${PWD}/subdir" |
| 120 |
+echo pass >symlink |
| 121 |
+) |
| 122 |
+# we should be able to write through the symlink |
| 123 |
+if [ $? -ne 0 ] ; then |
| 124 |
+ exit 1 |
| 125 |
+fi |
| 126 |
+ |
| 127 |
+# and not gotten a sandbox violation |
| 128 |
+test ! -s "${SANDBOX_LOG}" |
| 129 |
+ |
| 130 |
+exit $? |
| 131 |
|
| 132 |
diff --git a/tests/script.at b/tests/script.at |
| 133 |
index 93e370a..f07a8f1 100644 |
| 134 |
--- a/tests/script.at |
| 135 |
+++ b/tests/script.at |
| 136 |
@@ -8,3 +8,5 @@ SB_CHECK(7) |
| 137 |
SB_CHECK(8) |
| 138 |
SB_CHECK(9, [wait errpipe... done OK!]) |
| 139 |
SB_CHECK(10) |
| 140 |
+SB_CHECK(11) |
| 141 |
+SB_CHECK(12) |
Archives