commit 95857360ec7f84cf9f0a01855c15881c89919133 Author: Yawning Angel <yawning@schwanenlied.me> Date: Mon Jul 3 17:27:35 2017 +0000 Bug 20773: Stop mounting /proc in the Tor Browser container. Wasn't so bad to figure out, though upstream still should fall back gracefully, since the only reason why it crashes horribly is because pthread_attr_getstack() returns a stack size of 0 for the default thread when /proc is missing. In our case we can safely fall back to querying RLIMIT_STACK, because the sandbox containerization explicity sets it to something large, but relatively sane. --- ChangeLog | 1 + README.md | 12 ++-- .../internal/sandbox/application.go | 1 + src/tbb_stub/tbb_stub.c | 76 ++++++++++++++++++++++ 4 files changed, 83 insertions(+), 7 deletions(-) diff --git a/ChangeLog b/ChangeLog index b42022a..51f693a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,6 @@ Changes in version 0.0.9 - UNRELEASED: * Bug 22712: Suppress ATK Bridge initialization which will never work. + * Bug 20773: Stop mounting /proc in the Tor Browser container. * Fix the build being broken on Debian Jessie due to #22648. * Remove the undocumented command line options that enable unsafe behavior. diff --git a/README.md b/README.md index dc4fc18..6cad572 100644 --- a/README.md +++ b/README.md @@ -41,18 +41,16 @@ Things that the sandbox breaks: Places where the sandbox could be better: - * More about the host system is exposed than neccecary, primarily because - Firefox crashes without `/proc`. + * The updater container still mounts `/proc`. + * PulseAudio is likely unsafe without a protocol filter like X11. + * X11 is still X11, and despite mitigations is likely still unsafe. Upstream Bugs: - * Tor Browser should run without a `/proc` filesystem. - (https://bugs.torproject.org/20283) + * Tor Browser should run without a `/proc` filesystem, worked around in + the worst possible way. (https://bugs.torproject.org/20283) * OpenGL software rendering is broken on certain Linux systems. (https://bugs.torproject.org/20866) - * On certain systems, Firefox crashes with an X11 error due to the lack of - MIT-SHM support. This appears to be a race condition, and still occurs - despite mitigations. (https://bugzilla.mozilla.org/show_bug.cgi?id=1271100) Notes: diff --git a/src/cmd/sandboxed-tor-browser/internal/sandbox/application.go b/src/cmd/sandboxed-tor-browser/internal/sandbox/application.go index d3162dd..a160b67 100644 --- a/src/cmd/sandboxed-tor-browser/internal/sandbox/application.go +++ b/src/cmd/sandboxed-tor-browser/internal/sandbox/application.go @@ -69,6 +69,7 @@ func RunTorBrowser(cfg *config.Config, manif *config.Manifest, tor *tor.Tor) (pr h.stderr = logger h.seccompFn = installTorBrowserSeccompProfile h.fakeDbus = true + h.mountProc = false // Gtk+ and PulseAudio. hasAdwaita := h.appendGtk2Theme() diff --git a/src/tbb_stub/tbb_stub.c b/src/tbb_stub/tbb_stub.c index e7ca733..84a14e8 100644 --- a/src/tbb_stub/tbb_stub.c +++ b/src/tbb_stub/tbb_stub.c @@ -37,6 +37,9 @@ #define _GNU_SOURCE /* Fuck *BSD and Macintoys. */ +#include <sys/types.h> +#include <sys/syscall.h> +#include <sys/resource.h> #include <sys/socket.h> #include <sys/un.h> #include <arpa/inet.h> @@ -47,11 +50,13 @@ #include <stdbool.h> #include <stdio.h> #include <stdlib.h> +#include <unistd.h> static pthread_once_t stub_init_once = PTHREAD_ONCE_INIT; static int (*real_connect)(int, const struct sockaddr *, socklen_t) = NULL; static int (*real_socket)(int, int, int) = NULL; static void *(*real_dlopen)(const char *, int) = NULL; +static int (*real_pthread_attr_getstack)(const pthread_attr_t *, void **, size_t *); static struct sockaddr_un socks_addr; static struct sockaddr_un control_addr; @@ -228,6 +233,73 @@ pa_start_child_for_read(const char *name, const char *argv1, pid_t *pid) return -1; } +/* Firefox will crash if pthread_attr_getstack doesn't return a sensible stack + * size, which will happen if /proc is missing, since glibc grovels through + * /proc/self/maps to determine this information for the default thread. + * + * See: glibc/nptl/pthread_getattr_np.c + */ +int +pthread_attr_getstack(const pthread_attr_t *attr, void **stackaddr, size_t *stacksize) +{ + int ret; + + ret = real_pthread_attr_getstack(attr, stackaddr, stacksize); + if (ret != 0) { + fprintf(stderr, "WARN: pthread_attr_getstack(%p, %p, %p) = %d\n", attr, stackaddr, stacksize, ret); + return ret; + } + +#if 0 + fprintf(stderr, "tbb_stub: pthread_attr_getstack(%p, %p, %p) = %d\n", attr, stackaddr, stacksize, ret); + fprintf(stderr, "tbb_stub: stackaddr: %p\n", stackaddr); + fprintf(stderr, "tbb_stub: stacksize: %ld\n", *stacksize); +#endif + + /* If we got a sensible value for the stack size, then return. */ + if (*stacksize != 0) { + return ret; + } else { + /* Otherwise, we should be the initial thread (pid == tid). */ + pid_t tid = syscall(__NR_gettid); + pid_t pid = getpid(); + + if (tid != pid) { + fprintf(stderr, "ERROR: Got a 0 stack size when pid = %d != tid = %d\n", pid, tid); + abort(); + } + } + + /* First try pthread_attr_getstacksize(), which works on glibc 2.25. */ + ret = pthread_attr_getstacksize(attr, stacksize); + if (ret != 0 || *stacksize == 0) { + /* Fall back to getrlimit(). */ + struct rlimit rl; + + ret = getrlimit(RLIMIT_STACK, &rl); + if (ret != 0) { + fprintf(stderr, "ERROR: Failed to query rlimit: %d", ret); + abort(); + } + + /* So, the main reason why glibc digs through proc is so that it can + * return the current committed stack size, and not the upper bound on + * the stack size. But without /proc, there's no good way to get this + * information. + * + * This is probably ok, pthread_attr_getstacksize() without proc mounted + * appears to do the same thing. + */ + *stacksize = rl.rlim_cur; + } + +#if 0 + fprintf(stderr, "tbb_stub: Fallback stacksize: %ld\n", *stacksize); +#endif + + return ret; +} + /* Initialize the stub. */ static void stub_init(void) @@ -257,6 +329,10 @@ stub_init(void) fprintf(stderr, "ERROR: Failed to find `socket()` symbol: %s\n", dlerror()); goto out; } + if ((real_pthread_attr_getstack = dlsym(RTLD_NEXT, "pthread_attr_getstack")) == NULL) { + fprintf(stderr, "ERROR: Failed to find `pthread_attr_getstack()` symbol: %s\n", dlerror()); + goto out; + } /* Initialize the SOCKS target address. */ socks_addr.sun_family = AF_LOCAL;