1285768 - Seccomp sandbox violation: sys_getppid called in content process of Firefox desktop

Status: RESOLVED FIXED

(Core :: Security: Process Sandboxing, defect)

Description

[Julian Hector [:tedd] [:jhector]]

Crash reports show that sys_getppid is called in the content process:

https://crash-stats.mozilla.com/search/?product=Firefox&reason=~SIGSYS&address=0x6e&_sort=-date&_facets=signature&_columns=date&_columns=signature&_columns=product&_columns=version&_columns=build_id&_columns=platform#crash-reports

Comment 1

[Julian Hector [:tedd] [:jhector]]

Attachment: Add sys_getppid to seccomp whitelist. r=gcp

Try push for build: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d6dfb303806d

Comment 2

[Gian-Carlo Pascutto [:gcp]]

Do we know what the underlying requester is? The stacks look a bit broken, it might be gfx (GL) but not sure.

getppid looks like the kind of function that we'd rather block.

Comment 3

[Gian-Carlo Pascutto [:gcp]]

Comment on attachment 8769502 [details] [diff] [review]
Add sys_getppid to seccomp whitelist. r=gcp

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

I'd like to hold off and discuss this one.

Comment 4

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

Comment on attachment 8769502 [details] [diff] [review]
Add sys_getppid to seccomp whitelist. r=gcp

getppid seems pretty harmless.  There's probably a copy of the pid already in the address space for one reason or another (if not our IPC layer, then maybe in glibc's structures somewhere), and the implementation is relatively simple, so it wouldn't be a concern from an attack surface point of view.

Once we're using pid namespaces, getppid() will return 0 and that might cause some problems, but I don't think that needs to block this patch.

Comment 5

[Julian Hector [:tedd] [:jhector]]

Comment on attachment 8769502 [details] [diff] [review]
Add sys_getppid to seccomp whitelist. r=gcp

I agree with :jld, I don't think this is harmful. So far we only received 3 crash reports where this was an issue. I would be fine with landing this, but also don't mind waiting and investigating.

Comment 6

[Gian-Carlo Pascutto [:gcp]]

Comment on attachment 8769502 [details] [diff] [review]
Add sys_getppid to seccomp whitelist. r=gcp

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

Alright then. My main concern is that it's trying to get the parent pid for "something", and that "something" is probably "something" we don't want to allow either.

Comment 7

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

I took a look at fglrx: it seems to be using getppid() so it can scan the parent process's command line for the names of profiling tools.

So here's another idea: we could make getppid() return 0 instead of the actual ppid, to pre-break anything that will break in a pid namespace leader (bug 1151624), where getppid() really does return 0.

Comment 8

[Julian Hector [:tedd] [:jhector]]

Attachment: Let getppid() return 0 to simulate pid namespaces. r=gcp

Implements the behavior mentioned in Comment 7.

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3196cc327244

Comment 9

[Julian Hector [:tedd] [:jhector]]

Try in Comment 8.

Please check in after Bug 1286185 (to avoid merge conflict)

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/414ef1361cd2
Let getppid() return 0 to simulate pid namespaces. r=gcp

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/414ef1361cd2