829619 - Validate "Webapps:RegisterForMessages/UnregisterForMessages" in the parent process

Status: RESOLVED INVALID

(Core Graveyard :: DOM: Apps, defect)

Description

[Gene Lian (I already quit Mozilla)]

(quoting Paul from bug 820206, comment #1)
--------------------------------------------------
Webapps:RegisterForMessages
Webapps.js uses this to register for specific messages, but there is no check in the parent as to what messages are being listened for. I am not sure if this is an issue - would there be any benefit to the child to be able add itself as a listener for arbitrary messages (perhaps to intercept sensitive messages?)

(quoting Jonas from bug 820206, comment #45)
--------------------------------------------------
Ah, now I see what's going on here. I *think* that we are fine here as long as all message listening is managed on a per-childprocess basis (or per-application basis).

Comment 1

[Andrew Overholt [:overholt]]

koi? since it blocks bug 820202 but we don't usually block on meta bugs.

Comment 2

[Andrew Overholt [:overholt]]

Gene, would we hold 1.2 for this?

Comment 3

[Kai-Chih Hu [:khu]]

Looks like it's something related to comms team?

Comment 4

[[:fabrice] Fabrice Desré]

No, it's a platform issue. It's not even obvious that we have something to fix here.

Comment 5

[Gene Lian (I already quit Mozilla)]

+1 with Fabrice. This bug just highlight a potential security issue instead of a critical blocker. I'd suggest it's not really necessary to put it in the V1.2.

I think an easy solution is to let the 'Webapps:RegisterForMessages' message carry manifestURL and do the following check in the parent:

  receiveMessage: function receiveMessage(aMessage) {
    if (aMessage.name == Webapps:RegisterForMessages &&
        !aMessage.target.assertContainApp(aMessage.json.manifestURL)) {
      debug("Got message from a child process containing illegal manifest URL.");
      return null;
    }
  }

In this way, if any bad child wants to hack the 'Webapps:RegisterForMessages' message for other good child, the parent will stop that.

However, the bad child can still have chance to register messages for itself. Just like Jonas mentioned at comment #0, if side-effects would just happen on the bad child, then we don't need to worry about that. The current messages a bad child can register for are:

  "Webapps:Install:Return:OK"
  "Webapps:Uninstall:Return:OK"
  "Webapps:Uninstall:Broadcast:Return:OK
  "Webapps:OfflineCache"
  "Webapps:PackageEvent"
  "Webapps:CheckForUpdate:Return:OK"

Do they really sound dangerous?

Comment 6

[[:fabrice] Fabrice Desré]

(In reply to Gene Lian [:gene] from comment #5)

>   "Webapps:Install:Return:OK"
>   "Webapps:Uninstall:Return:OK"
>   "Webapps:Uninstall:Broadcast:Return:OK
>   "Webapps:OfflineCache"
>   "Webapps:PackageEvent"
>   "Webapps:CheckForUpdate:Return:OK"
> 
> Do they really sound dangerous?

They can all be used to know which apps the user has installed, so there can be privacy issues. Your mitigation sounds reasonable though even if not perfect.

Comment 7

[Andrew Overholt [:overholt]]

(In reply to Gene Lian [:gene] from comment #5)
> +1 with Fabrice. This bug just highlight a potential security issue instead
> of a critical blocker. I'd suggest it's not really necessary to put it in
> the V1.2.

Okay, I'll remove koi?.  Please feel free to re-nom for whatever release is appropriate.