1436509 - Bring back sandboxed Address Sanitizer builds on Linux

Status: NEW

(Core :: Security: Process Sandboxing, enhancement, P3)

Description

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

In bug 1287971, we stopped trying to support sandboxing on Linux ASan builds because we'd started using Leak Sanitizer and it did a few things that we couldn't allow and still have a meaningful sandbox; see bug 1287971 comment #9 for some background.

But with pid namespace isolation, this might work: we'd have to let the process fork itself with weird flags and ptrace() arbitrary threads, but if it's contained in a pid namespace that should be relatively safe.

The other problem is that it blocks SIGSYS — using its own inlined syscall wrapper, so the interposition in SandboxHooks.cpp doesn't apply — so anything we'd normally broker will instead immediately kill the process, due to an unfortunate design decision in the original seccomp-bpf implementation.  And one of the things it does after blocking SIGSYS is call open() on files in procfs, again with an internal syscall wrapper.

It's possible to deal with that by allowing rt_sigprocmask only from a specific instance of the syscall instruction, and otherwise trapping into a signal handler where we can inspect and alter the masks.  And the Chromium seccomp-bpf compiler has functionality for this, but it's somewhat coupled to being used as a non-enforcing mode for the entire filter and is locked down to prevent being accidentally used outside of testing.  So this would require some refactoring and upstreaming to Chromium.

Another approach is to try to get a patch into upstream compiler-rt to add SIGSYS to the list of signals it doesn't block (and then *not* replace the existing handler… or we could prevent that with seccomp-bpf) (or, alternately, to make the procfs opening stuff aware of the sandboxing hooks that libsanitizer already has), but then we'd have to wait for that change to slowly diffuse back downstream, and we'd have to have a way of detecting whether the sanitizer runtime has it *before* doing sandboxing.  Trying to fix the kernel's handling of SECCOMP_RET_TRAP in the case of a blocked-but-not-ignored SIGSYS is also theoretically possible, but again it would take years to reach users.

Comment 1

[Christian Holler (:decoder)]

We use ASan's log_path option in fuzzing as well as in the ASan Nightly Project by now, which means that all parts of Firefox need to be able to directly write to the filesystem. This is currently essential functionality, so enabling the sandbox on ASan builds doesn't seem like a viable option to me unless we can disable it at build- and runtime.

Comment 2

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

(In reply to Christian Holler (:decoder) from comment #1)
> We use ASan's log_path option in fuzzing as well as in the ASan Nightly
> Project by now, which means that all parts of Firefox need to be able to
> directly write to the filesystem. This is currently essential functionality,
> so enabling the sandbox on ASan builds doesn't seem like a viable option to
> me unless we can disable it at build- and runtime.

We call __sanitizer_sandbox_on_notify, which in principle should cause the runtime to pre-acquire any external resources it needs, but that might not apply to log files.  I don't know if that approach *can* work for logs, because it would require every process to create an empty log file when it doesn't log anything, and something unsandboxed would have to be able to clean them up.

In any case, this probably isn't a problem for content processes: filesystem brokering should be transparent to the calling code, even if it uses inlined system calls like libsanitizer does; we'd just need to know the log file path to add it to the policy.

This is more of a problem for media plugins.  I'd prefer not to expose the full attack surface of the file broker to those, but it's much better than no sandboxing at all.

Comment 3

[Andrew McCreight [:mccr8]]

Maybe you saw this already, but I believe there's some way to make LSan not symbolicate, which would probably get rid of the thing that LSan needs privileges to do. We'd then have to do some external work to symbolicate, like we do for other kinds of stacks.

Comment 4

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

The LSan code I was having problems with was StopTheWorld: https://github.com/llvm-mirror/compiler-rt/blob/3ac94fe6137e/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc#L394

That gets called (via DoStopTheWorld; there's some complexity around locking and dl_iterate_phdr) from CheckForLeaks, and I'm left with the distinct impression that it's needed for the core of LSan, not just symbol lookup.


But trying to fork+exec a symbol lookup tool would be an additional problem.  There's alread a workaround (ifdef'ed) to make it not try that: https://searchfox.org/mozilla-central/rev/d160ac574a5798010eda4e33e91ee628aa268a33/security/sandbox/linux/SandboxFilter.cpp#357-358