[gentoo-commits] proj/sandbox:master commit in: libsandbox/wrapper-funcs/

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

commit:     26ad6af1a4f246bda3cd7a19a24c1767ec9c835e
Author:     Mike Frysinger <vapier <AT> gentoo <DOT> org>
AuthorDate: Mon Dec  3 06:15:15 2012 +0000
Commit:     Mike Frysinger <vapier <AT> gentoo <DOT> org>
CommitDate: Mon Dec 24 08:01:36 2012 +0000
URL:        http://git.overlays.gentoo.org/gitweb/?p=proj/sandbox.git;a=commit;h=26ad6af1

libsandbox: fall back to tracing set*id programs

If we are non-root and run a set*id program, the ldso will ignore our
LD_PRELOAD (rightly so).  Unfortunately, this opens up the ability to
run set*id apps that modify things and sandbox cannot catch it.

Instead, force ptracing of these ELFs.  While the kernel will disallow
the set*id aspect when running, for the most part, that shouldn't be a
problem if it was already safe.

URL: http://bugs.gentoo.org/442172
Reported-by: Nikoli <nikoli <AT> lavabit.com>
Signed-off-by: Mike Frysinger <vapier <AT> gentoo.org>

---
 libsandbox/wrapper-funcs/__wrapper_exec.c |   23 ++++++++++++++++++-----
 1 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/libsandbox/wrapper-funcs/__wrapper_exec.c b/libsandbox/wrapper-funcs/__wrapper_exec.c
index 0ffc08a..b7c7dfa 100644
--- a/libsandbox/wrapper-funcs/__wrapper_exec.c
+++ b/libsandbox/wrapper-funcs/__wrapper_exec.c
@@ -31,7 +31,7 @@ static void sb_check_exec(const char *filename, char *const argv[])
 	fd = open(filename, O_RDONLY|O_CLOEXEC);
 	if (fd == -1)
 		return;
-	if (stat(filename, &st))
+	if (fstat(fd, &st))
 		goto out_fd;
 	if (st.st_size < sizeof(Elf64_Ehdr))
 		goto out_fd;
@@ -47,6 +47,17 @@ static void sb_check_exec(const char *filename, char *const argv[])
 	      elf[EI_CLASS] != ELFCLASS64))
 		goto out_mmap;
 
+	/* If we are non-root but attempt to execute a set*id program,
+	 * our LD_PRELOAD trick won't work.  So skip the static check.
+	 * This might break some apps, but it shouldn't, and is better
+	 * than doing nothing since it might mean `mount` or `umount`
+	 * won't get caught if/when they modify things. #442172
+	 *
+	 * Only other option is to code a set*id sandbox helper that
+	 * gains root just to preload libsandbox.so.  That unfortunately
+	 * could easily open up people to root vulns.
+	 */
+	if (getuid() == 0 || !(st.st_mode & (S_ISUID | S_ISGID))) {
 #define PARSE_ELF(n) \
 ({ \
 	Elf##n##_Ehdr *ehdr = (void *)elf; \
@@ -60,10 +71,12 @@ static void sb_check_exec(const char *filename, char *const argv[])
 		if (phdr[p].p_type == PT_INTERP) \
 			goto done; \
 })
-	if (elf[EI_CLASS] == ELFCLASS32)
-		PARSE_ELF(32);
-	else
-		PARSE_ELF(64);
+		if (elf[EI_CLASS] == ELFCLASS32)
+			PARSE_ELF(32);
+		else
+			PARSE_ELF(64);
+#undef PARSE_ELF
+	}
 
 	do_trace = trace_possible(filename, argv, elf);
 	/* Now that we're done with stuff, clean up before forking */