Tor Browser should run without a `/proc` filesystem.

Currently Tor Browser crashes immediately on startup if a proc filesystem is not mounted on /proc. This also affects the upstream firefox code, so it technically is a Mozilla bug.

too much recursion
Segmentation fault (core dumped)

/proc contains a large amount of information about the host system that can be used to fingerprint/identify users and additionally historically has been the source or part of many kernel security problems.

While this problem can be mitigated by a MAC system (eg: AppArmor) to constrain what Firefox can access under /proc, the ideal fix is for Firefox to support running without /proc, while degrading gracefully (there is no truly ubiquitous MAC system available on all common Linux distributions by default, and the problem is severe enough that it should be resolved correctly).

added TorBrowserTeam201804R component::applications/tor browser owner::pospeselr parent::20773 priority::very high resolution::fixed severity::normal sponsor::4 status::closed tbb-sandboxing type::defect labels

Do you know whether there is already an upstream bug for this?

Trac:
Cc: N/A to gk

AFAIK no, but I will admit that I didn't look very hard through bugzilla. It's probably a fairly uncommon use case, so I'm not sure how much upstream will care about it.

Trac:
Parent: N/A to #20773 (closed)

Trac:
Cc: gk to gk, intrigeri

I worked around this in sandboxed-tor-browser so I don't care about this anymore.

Upstream (Old): https://bugzilla.mozilla.org/show_bug.cgi?id=859782

Trac:
Status: new to assigned
Owner: tbb-team to pospeselr

Trac:
Cc: gk, intrigeri to gk, intrigeri, mcs

There are at least two issues that I know of that prevent running Firefox without /proc mounted.

The first is that Firefox uses /proc/self/task to see if it spawned any threads. The warning can be ignored on any kernel that supports SECCOMP_FILTER_FLAG_TSYNC (>= 3.17), but may result in "bad" if the kernel is old, and no, I do not remember what the bad is.

The second is that Firefox will crash with too much recursion if /proc is not mounted. The culprit there is that Firefox will query the stack size with pthread_attr_getstack() which will return a stack size of 0, if /proc is not mounted for the default thread (tid == pid).

Note that there may be other horrific things that happen, or other things that break without /proc, but I was not able to find any at the time that I cared about this. Finding and debugging such things is left as an exercise for the student. Fixing this properly probably requires upstream to care about this use case.

If SECCOMP_FILTER_FLAG_TSYNC isn't available and /proc/self/task can't be listed, the sandbox can't start. The process is already multithreaded, so we have to signal all the threads to tell them to apply seccomp, and we don't have access to the libc's internal list of threads (or the lock protecting it) so we have to ask the kernel via procfs.

The single-threadedness check, however, has been removed in Firefox 60, as part of https://bugzilla.mozilla.org/show_bug.cgi?id=1401062.

Trac:
Username: jld

Trac:
Cc: gk, intrigeri, mcs to gk, intrigeri, mcs, tbb-team

Note that there may be other horrific things that happen

The content process will SIGSEGV on JS heavy pages if pthread_attr_getstack() returns a NULL stackaddr.

I've identified all (hopefully) the callers in the parent firefox and the child plugin_container (nothing unique in plugin_container though that isn't called in the parent). The spawned glxtest also has lots of reads going to /proc as well.

For patch verification, what's the procedure for hiding /proc from a process in general?

Replying to jld:

If SECCOMP_FILTER_FLAG_TSYNC isn't available and /proc/self/task can't be listed, the sandbox can't start. The process is already multithreaded, so we have to signal all the threads to tell them to apply seccomp, and we don't have access to the libc's internal list of threads (or the lock protecting it) so we have to ask the kernel via procfs.

The single-threadedness check, however, has been removed in Firefox 60, as part of https://bugzilla.mozilla.org/show_bug.cgi?id=1401062.

That's actually #23915 (closed) and we should be good with that.

Replying to pospeselr:

I've identified all (hopefully) the callers in the parent firefox and the child plugin_container (nothing unique in plugin_container though that isn't called in the parent). The spawned glxtest also has lots of reads going to /proc as well.

For patch verification, what's the procedure for hiding /proc from a process in general?

I don't know but you could mess with Yawning's sandboxed-tor-browser (which brought this issue up in the first place), see: #20773 (closed) and the patch for it in 95857360ec7f84cf9f0a01855c15881c89919133.

From conversation with pospeselr:

I suspect that glibc relies on /proc only for the initial thread. It is possible that for the initial thread you can get the stack base address and size using getcontext(2). In particular, if pthread_getattr_np fails or pthread_attr_getstack returns null, and the thread in question is the process's initial thread, then it is possible that

ucontext_t uc;
void *base;
size_t size;

if (getcontext(&uc) == -1)
	err(1, "getcontext");
base = uc.uc_stack.ss_sp;
size = uc.uc_stack.ss_size;

will recover the stack base address and size that you want.

Replying to cypherpunks:

I suspect that glibc relies on /proc only for the initial thread.

Yes.

it is possible that

{{{ ucontext_t uc; void *base; size_t size;

if (getcontext(&uc) == -1) err(1, "getcontext"); base = uc.uc_stack.ss_sp; size = uc.uc_stack.ss_size; }}}

will recover the stack base address and size that you want.

99% sure this does not work without /proc mounted.

As mentioned by cypherpunks, the issue here only occurs on the main thread which you can verify with a little spelunking through the glibc source. The relevant query APIs work expected on non-main threads since pthreads populates the required info in memory.

Verifying a patch now on TreeHerder that fixes this issue by reading the !__libc_stack_end symbol (glibc sets this void* to the first stack frame during program init). Solution suggested by mozilla's jld, and the folks in #jsapi (those paying attention at least) didn't seem offended at the idea of doing so for the main thread.

Trac:
Keywords: N/A deleted, TorBrowserTeam201804 added
Priority: Medium to Very High

Trac:
0001-Bug-20283-Tor-Browser-should-run-without-a-proc-file.patch

js::GetNativeStackBaseImpl fix

Trac:
Status: assigned to needs_review

Trac:
Keywords: TorBrowserTeam201804 deleted, TorBrowserTeam201804R added

Audited all the remaining syscalls (was only catch'ing open and openat before) and discovered three more things which don't seem to be an issue for us. Here's the full list of what we're working with (on x64 Linux) roughly in order of invocation for future reference (open here covers both open and openat, didn't save that data unfortunately):

= Caller =	= syscall =	= Requested Path =	= Relevant Source
mozilla::HasUserNamespaceSupport()	access	"/proc/self/ns/user"	/security/sandbox/linux/SandboxInfo.cpp
			Failure here 'just' disables 'User Namespaces' (as shown under the Sandbox section of about:support), not sure what the security consequences of this is but tor browser boots and runs fine
selinuxfs_exists()	open	"/proc/filessytems"
			This call happens down in dlopen when opening libxul.so. Failure to open this file will result in selinxufs_exists() always reporting selinuxfs exists ( see selinuxfs_exists(void) but everything seems to go along even if it is a false positive)
mozilla::(anonymous namespace)::IsSingleThreaded ()	xstat	"/proc/self/task"	/security/sandbox/linux/SandboxInfo.cpp
			Failure to read and count number of files under this directory causes tor browser to guess and indicate that it is not single threaded
mozilla::ComputeProcessUptimeThread(void*)	open	"/proc/self/task/$tid/stat", "/proc/self/stat"	/mozglue/misc/TimeStamp_posix.cpp
			This code fails gracefully and sets uptime to 0
nsNotifyAddrListener::calculateNetworkId()	open	"/proc/net/route", "/proc/net/arp"	/netwerk/system/linux/nsNotifyAddrListener_Linux.cpp
			This code fails gracefully, network id is only used once for telemetry, otherwise this is dead code
js::GetNativeStackBaseImpl()	open	"/proc/self/maps"	/js/src/jsnativestack.cpp
			This code gets an address at the top of the usable stack, required for internal JS stuff or else bad things happen. Attached patch fixes this issue using !__libc_stack_end
mozilla::ThreadStackHelper::GetThreadStackBase()	open	"/proc/self/maps"	/xpcom/threads/ThreadStackHelper.cpp
			Similar to js::GetNativeStackBaseImpl() except the implementation isn't there for all platforms and the saved value isn't used for anything, fails gracefully
nsSystemInfo::Init()	open	"/proc/cpuino"	/xpcom/base/nsSystemInfo.cpp
			Fails gracefully, otherwise scrapes all your CPU info and stores it in the nsISystemInfo property bag for god knows what reasons
gfxFcPlatformFontList::FindGenericFamilies()	readlink	"/proc/self/exe"	/gfx/thebes/gfxFcPlatformFontList.cpp
			Fails gracefully when intercepting this syscall and returning error (-1)
nsLocalFile::GetDiskSpaceAvailable()	open	"/proc/self/mountinfo"	/xpcom/io/nsLocalFileUnix.cpp
			Fails gracefully when given file isn't found and a simpler estimate is used instead

Thanks for writing all those cases up in comment:22.

So, I think we could just avoid integrating the latest jsnativestack.cpp changes into esr52, given that the those did not get backported and thus might carry breakage risks. E.g. the fix for https://bugzilla.mozilla.org/show_bug.cgi?id=1423287 makes me sightly nervous due to touching the GNUC part, but my testing seems to be fine with it. (No, the try result done by dmajor does not change my nervousness factor as they very likely use a different toolchain (different GCC and binutils) versions than we)

However, that's for an alpha release and I think taking the patch especially after the Mozilla review is fine for that. Therefore: Applied to tor-browser-52.7.3esr-8.0-1 (as commit 90e16dd25b6eb199c016fc8b5e9478a3454d36e1) to get it into 8.0a6.

Nice work!

Trac:
Resolution: N/A to fixed
Status: needs_review to closed

Is this something we should try to uplift?

Replying to tom:

Is this something we should try to uplift?

It already has r+ in https://bugzilla.mozilla.org/show_bug.cgi?id=859782. Sure, if we could get this into beta, great! But I am not so optimistic.

Trac:
Sponsor: N/A to Sponsor4

closed

mentioned in issue #20781 (closed)

mentioned in issue #26576 (moved)

mentioned in issue #26615 (moved)

moved to tpo/applications/tor-browser#20283 (closed)

mentioned in issue tpo/applications/tor-browser#26576

mentioned in issue tpo/applications/tor-browser#26615 (closed)