1078838 - Lock down Linux content process clone(2) flags

Status: RESOLVED FIXED

(Core :: Security, defect)

Description

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

We should restrict clone(2) in sandboxed processes to the specific set of flags used by pthread_create(3).  Advantages: reduce attack surface in general, block things like CLONE_NEWUSER in particular, and prevent creating new thread groups (so that one SIGKILL will terminate all tasks that the content process could have created).

This is already in place for GMP, because so far it's supported only on GNU/Linux and glibc's thread creation clone flags haven't changed in a decade.  Desktop content would work the same way.

On B2G, it varies.  We should be able to specialize by Android API level:

 <=17: CLONE_VM | CLONE_FS | CLONE_FILES | CLONE_SIGHAND | CLONE_THREAD | CLONE_SYSVSEM | CLONE_DETACHED
(https://android.googlesource.com/platform/bionic/+/40eabe24e4e3ae8ebe437f1f4e43cf39cbba2e9e)
18-20: CLONE_VM | CLONE_FS | CLONE_FILES | CLONE_SIGHAND | CLONE_THREAD | CLONE_SYSVSEM
(https://android.googlesource.com/platform/bionic/+/70b24b1cc2a1a4436b1fea3f8b76616fdcb27224)
(https://android.googlesource.com/platform/bionic/+/877ec6d90418ff1d6597147d355a2229fdffae7e)
 >=21: CLONE_VM | CLONE_FS | CLONE_FILES | CLONE_SIGHAND | CLONE_THREAD | CLONE_SYSVSEM | CLONE_SETTLS | CLONE_PARENT_SETTID | CLONE_CHILD_CLEARTID

Comment 1

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

Attachment: bug1078838-cloneflags-hg0.diff

This seems to work.  I should probably try it on a few more phones first.  And it'd be good for someone who knows Android versioning better than I do to look at that part of it.

Comment 2

[Michael Wu [:mwu]]

Comment on attachment 8506474 [details] [diff] [review]
bug1078838-cloneflags-hg0.diff

Review of attachment 8506474 [details] [diff] [review]:
-----------------------------------------------------------------

::: security/sandbox/linux/SandboxFilter.cpp
@@ +79,5 @@
> +  // value has changed a few times, but it's more or less a function
> +  // of Android releases.
> +  static const int flags_common = CLONE_VM | CLONE_FS | CLONE_FILES |
> +    CLONE_SIGHAND | CLONE_THREAD | CLONE_SYSVSEM;
> +#ifdef ANDROID

If we care about Firefox for Android, we can't decide this at compile time. Up to you.

Another alternative is to just use the same flags across all versions.
It looks like CLONE_DETACHED has no effect on modern Linux kernels, so it shouldn't hurt to include that.

Comment 3

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

(In reply to Michael Wu [:mwu] from comment #2)
> If we care about Firefox for Android, we can't decide this at compile time.
> Up to you.

We're not using this on Android (yet?), and I don't know if any non-B2G Android devices have kernel support yet.  But that's a good point; at minimum there should be an #ifdef/#error for that.

> Another alternative is to just use the same flags across all versions.
> It looks like CLONE_DETACHED has no effect on modern Linux kernels, so it
> shouldn't hurt to include that.

I'd have to check the value against all three possibilities, but that's not the worst thing in the world.  (Once I can import the Chromium bpf_dsl library (or reinvent that wheel, but I'd rather not), there will be support for masking off some bits and doing equality tests on the rest.)

Comment 4

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

Attachment: bug1078838-cloneflags-hg1.diff

Let's just check for any of the known values on Android.  Simpler, less chance of somehow accidentally breaking something, and no meaningful increase in attack surface.

Tested on keon and flame-kk, and with desktop GMP tests on x86_64.

Comment 5

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

https://hg.mozilla.org/integration/b2g-inbound/rev/6ab760222a4e

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/6ab760222a4e