[Inter App Communication API] child process defined fields used in security check in parent

RESOLVED INVALID

Status

RESOLVED INVALID
5 years ago
11 months ago

People

(Reporter: rfletcher, Unassigned)

Tracking

Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

5 years ago
dom/apps/src/InterAppCommService.js#814
receiveMessage() in the parent process gets a message and attempts to prevent a
compromised child process from sending commands to the parent in the following
code:

814   receiveMessage: function(aMessage) {
...
816     let message = aMessage.json;
817     let target = aMessage.target;
818 
819     // To prevent the hacked child process from sending commands to parent
820     // to do illegal connections, we need to check its manifest URL.
821     if (aMessage.name !== "child-process-shutdown" &&
822         kMessages.indexOf(aMessage.name) != -1) {
823       if (!target.assertContainApp(message.manifestURL)) {
824         if (DEBUG) {
825           debug("Got message from a process carrying illegal manifest URL.");
826         }
827         return null;
828       }
829     }

'message' is defined in aMessage.json, which is sent from the child. The parent
process then uses message.manifestURL to perform the check. Since the child
process sends/creates the message, it might be possible or a compromised child
process to send a crafted manifestURL string to bypass the check. Something
like:
var aMessage = {
  ...
    json: {
        manifestURL: 'child-process-defined-manifest-file'
    }
    ...
}

If the child process can control manifestURL via its message, the value should
not be used for security checking.
(Reporter)

Comment 1

5 years ago
CC'ing bent on pauljt's recommendation that he would know if this is a real issue or not.
(Reporter)

Comment 2

5 years ago
After doing some more inspection, it appears that this code is checking to see if aMessage.json.manifestURL is in fact the manifest URL for the application sending that message. Therefore, if I send mismatched manifest in an effort to bypass the security check it will fail as it should.

Will follow up and close once I've verified.

Updated

5 years ago
Component: General → DOM: Apps
Product: Firefox OS → Core
Version: unspecified → Trunk
(In reply to Rob Fletcher [:omerta] from comment #2)
> After doing some more inspection, it appears that this code is checking to
> see if aMessage.json.manifestURL is in fact the manifest URL for the
> application sending that message. Therefore, if I send mismatched manifest
> in an effort to bypass the security check it will fail as it should.

Correct, if the child sends anything other than the url we expect then we kill it. At least, that is definitely the intention.

Of course there could always be bugs so I'm glad you're digging in here. :)
(Reporter)

Updated

5 years ago
Blocks: 967653
(Reporter)

Comment 4

5 years ago
Verified this check actually does exactly what we were worried about. :)

Did however lead to bug 967653
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → INVALID

Updated

5 years ago
blocking-b2g: 1.4? → ---

Comment 5

4 years ago
Does this need to remain private? Is it waiting on bug 967653?
Flags: needinfo?(gene.lian)
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #5)
> Does this need to remain private? Is it waiting on bug 967653?

It's fine to make this bug open but I think keeping it private makes no harm.

Bug 967653 solves another similar issue but for AppStatus.
Flags: needinfo?(gene.lian)
Group: core-security

Updated

11 months ago
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.