782982 - Parent process should never abort upon receiving invalid data from child

Status: RESOLVED DUPLICATE of bug 777067

(Core :: IPC, defect, P3)

Description

[Justin Lebar (not reading bugmail)]

The child process can trivially cause the parent to abort by e.g. sending an invalid enum.  See for example bug 782971.

(Nearly) every instance of NS_ABORT_IF_FALSE in IPDL-generated code run on the parent must be removed, if I understand correctly.

I don't know if bug 746280 is the right metabug -- do we have a different one specifically for "sandboxing"-type stuff?

Comment 1

[Justin Lebar (not reading bugmail)]

> (Nearly) every instance of NS_ABORT_IF_FALSE

and NS_RUNTIMEABORT, etc.

Comment 2

[Justin Lebar (not reading bugmail)]

> See for example bug 782971.

This is actually an example of the parent process crashing itself by trying to send bad data.

But it's still not clear to me what the parent will do upon receiving a bad enum from the child.

Comment 3

[Benjamin Smedberg]

The parent aborting on bad data is a bug we should fix, but I don't think it's common. We've been pretty careful about the kinds of data/state manipulations that should and shouldn't abort. We fixed one case in plugins a while back, see bug 767775.

But whether there is a real bug here or not, I don't think that this needs to block basecamp: *common* aborts will show up in crash-stats, and uncommon ones are just unfortunate DOSes.

Comment 4

[Justin Lebar (not reading bugmail)]

> I don't think that this needs to block basecamp: *common* aborts will show up in crash-stats, and 
> uncommon ones are just unfortunate DOSes.

I understand that hardening the parent process so that a malicious child process cannot crash the phone is a basecamp blocker.  If it's not, then of course this bug should not block.

Comment 5

[Andrew Overholt [:overholt]]

We discussed this in triage and decided it is basically a meta bug which we don't block on.  Please open dependent bugs.

Comment 6

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

However, note that DoS-ing the parent process is not a blocker. Reading sensitive data or causing sensitive actions is a blocker, but just DoS-ing isn't.

Comment 7

[Josh Matthews [:jdm]]

(In reply to Justin Lebar [:jlebar] from comment #2)
> > See for example bug 782971.
> 
> This is actually an example of the parent process crashing itself by trying
> to send bad data.
> 
> But it's still not clear to me what the parent will do upon receiving a bad
> enum from the child.

Instead of aborting, it can just call FatalError and kill the child process.

Comment 8

[Doug Turner (:dougt)]

Do we need this bug?  Is there anything actionable we need to do?

Comment 9

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

We need to audit the places we abort on encountering bogus data (like byte sequences that claim to have a length in the billions) and kill the child process instead of aborting.

Comment 10

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Hey Christoph, would you consider this to be a dupe of the work you're doing and closable?

Comment 11

[Christoph Diehl [:posidron]]

Basically, yes. There will be component specific follow up bugs (at least from my side) where such conditions failed. Besides that, NS_ABORT_IF_FALSE got removed in https://bugzilla.mozilla.org/show_bug.cgi?id=1127201.