Note bug 777188 and bug 776825, which are basically prereqs for this.

Renom if you think we can't ship a v1 without this.

Per IRC conversations with a few other folks, I think the best course of action if there is disagreement on whether this blocks or not is to do the following: - Move the blocking-basecamp flag to ? for re-evaluation - Indicate a rationale for why you disagree

Olli, is this something you can do?

Some of this is done in https://bug776652.bugzilla.mozilla.org/attachment.cgi?id=651156

smaug, are we done with this task? what are the next steps?

I don't know if I can do anything more. More messageManager usage have been added, I suppose, and now that we have the permission manager and new API (did it land already?) in message manager to assert permissions, we should "just" add the necessary checks. I believe the owner of the each component which uses mm is the right person to add permission asserts.

Assigning to Lucas to find somebody to resolve this bug. Giving this a P1 priority since we don't know what issues are lying behind this bug.

Paul, Lucas said you'd love to do this :) If you can't take it on, just un-assign yourself. Thanks!

Note that what we really need here is penn-testing. We should assume that all the data that's coming in to the message manager is hostile data and then see where that data flows to see if it can be used to hack other processes.

Have you had a chance to look at this yet, Paul?

Andrew, just looking at this now. I don't think I will have the bandwidth to do this, but I will try to recruit help from the security assurance team.

Marking for C2, given this meets the criteria of known P1/P2 bugs at the end of C1.

Paul, Daniel, Curtis: What's the ETA for this?

This is the top of my list after 815105. I should have it completed by the end of the week.

I think I misinterpreted this bug from earlier comments - I thought the goal was audit permission checks based on comment 5 above. But I see that there is already bug 776834 (Apply security checks to sensitive DOM APIs) and bug 776652 (Apply appropriate security checks for sensitive IPDL protocols) So I am trying to work out what the threat scenario is here: is it a compromised child process (e.g. chrome JavaScript execution?) that then seeks to escalate privileges via messages? I.e. APIs which have security checks or data validation only in the child not the parent, or where the parent blindly trusts that data received? As an example, webapps.js checks the app type (installed/privileged/certified) requested prior to install, and only allows web unless the device in developer mode. A spurious message could just request 'certified' and as far as I can tell, this isnt chekced in the parent webapps.jsm (http://mxr.mozilla.org/mozilla-central/source/dom/apps/src/Webapps.jsm#677) So, with user interaction maybe a higher privileged app could be installed. Or maybe a directory name control with the app id parameter (used here http://mxr.mozilla.org/mozilla-central/source/dom/apps/src/Webapps.jsm#1160) Am I on the right path? Would this sort of issue be basecamp blocker? (I feel like I should be prioritizing reviews of APIs exposed to content and work on the permissions API rather than defence in depth at this point). Not sure if I am understanding this bug correctly though - if someone could spare 20 mins Monday afternoon to give me a bit of a grounding in message managers that would be helpful.

(In reply to Paul Theriault [:pauljt] from comment #16) > I think I misinterpreted this bug from earlier comments - I thought the goal > was audit permission checks based on comment 5 above. But I see that there > is already bug 776834 (Apply security checks to sensitive DOM APIs) and bug > 776652 (Apply appropriate security checks for sensitive IPDL protocols) > > So I am trying to work out what the threat scenario is here: is it a > compromised child process (e.g. chrome JavaScript execution?) that then > seeks to escalate privileges via messages? Exactly. The threat is an app which uses javascript to create a buffer overflow or similar to break the javascript sandbox and execute binary code directly against the CPU. B2G uses a sandbox which prevents on an OS level child processes from accessing almost any data, other than that it can use IPDL or message manager to send messages to the parent process. These messages could have side effects or return sensitive data. Hence we should in all cases assume that the child process has been hacked and not trust any data that it sends us. I.e. we should only allow child processes to take actions that we want to allow the apps in those child processes to take. > I.e. APIs which have security > checks or data validation only in the child not the parent, or where the > parent blindly trusts that data received? Exactly. For APIs that we only have security checks in the child process, we effectively have no security checks at all if the child process is hacked. Now, this might not be a huge deal. If we leak some non-sensitive data in this scenario it's something we can live with for the v1 release. For example if a hacker can fingerprint the user after hacking a child process, for example by enumerating the set of apps installed, it might not be a blocker. But it's something that we'd want to know about. > As an example, webapps.js checks the app type > (installed/privileged/certified) requested prior to install, and only allows > web unless the device in developer mode. A spurious message could just > request 'certified' and as far as I can tell, this isnt chekced in the > parent webapps.jsm > (http://mxr.mozilla.org/mozilla-central/source/dom/apps/src/Webapps.jsm#677) Isn't this code run in the parent process? > So, with user interaction maybe a higher privileged app could be installed. > Or maybe a directory name control with the app id parameter (used here > http://mxr.mozilla.org/mozilla-central/source/dom/apps/src/Webapps.jsm#1160) If these checks indeed happen in the child process, then that's a bad thing. > Am I on the right path? Would this sort of issue be basecamp blocker? (I > feel like I should be prioritizing reviews of APIs exposed to content and > work on the permissions API rather than defence in depth at this point). > > Not sure if I am understanding this bug correctly though - if someone could > spare 20 mins Monday afternoon to give me a bit of a grounding in message > managers that would be helpful. It sounds like you are on the right path indeed.

(In reply to Jonas Sicking (:sicking) from comment #17) > (In reply to Paul Theriault [:pauljt] from comment #16) > > I think I misinterpreted this bug from earlier comments - I thought the goal > > was audit permission checks based on comment 5 above. But I see that there > > is already bug 776834 (Apply security checks to sensitive DOM APIs) and bug > > 776652 (Apply appropriate security checks for sensitive IPDL protocols) > > > > So I am trying to work out what the threat scenario is here: is it a > > compromised child process (e.g. chrome JavaScript execution?) that then > > seeks to escalate privileges via messages? > > Exactly. The threat is an app which uses javascript to create a buffer > overflow or similar to break the javascript sandbox and execute binary code > directly against the CPU. > > B2G uses a sandbox which prevents on an OS level child processes from > accessing almost any data, other than that it can use IPDL or message > manager to send messages to the parent process. These messages could have > side effects or return sensitive data. > > Hence we should in all cases assume that the child process has been hacked > and not trust any data that it sends us. I.e. we should only allow child > processes to take actions that we want to allow the apps in those child > processes to take. > > > I.e. APIs which have security > > checks or data validation only in the child not the parent, or where the > > parent blindly trusts that data received? > > Exactly. For APIs that we only have security checks in the child process, we > effectively have no security checks at all if the child process is hacked. > > Now, this might not be a huge deal. If we leak some non-sensitive data in > this scenario it's something we can live with for the v1 release. For > example if a hacker can fingerprint the user after hacking a child process, > for example by enumerating the set of apps installed, it might not be a > blocker. > > But it's something that we'd want to know about. > > > As an example, webapps.js checks the app type > > (installed/privileged/certified) requested prior to install, and only allows > > web unless the device in developer mode. A spurious message could just > > request 'certified' and as far as I can tell, this isnt chekced in the > > parent webapps.jsm > > (http://mxr.mozilla.org/mozilla-central/source/dom/apps/src/Webapps.jsm#677) > > Isn't this code run in the parent process? I meant the checks were in webapps.js, and not the webapps.jsm. Webapps.jsm seems to run in both parent and child, since it has ppmm and cpmm, but for the install flow it seems to be the parent, hence the checks dont seem to be in the parent where they belong. Thats what I meant. > > > So, with user interaction maybe a higher privileged app could be installed. > > Or maybe a directory name control with the app id parameter (used here > > http://mxr.mozilla.org/mozilla-central/source/dom/apps/src/Webapps.jsm#1160) > > If these checks indeed happen in the child process, then that's a bad thing. > > > Am I on the right path? Would this sort of issue be basecamp blocker? (I > > feel like I should be prioritizing reviews of APIs exposed to content and > > work on the permissions API rather than defence in depth at this point). > > > > Not sure if I am understanding this bug correctly though - if someone could > > spare 20 mins Monday afternoon to give me a bit of a grounding in message > > managers that would be helpful. > > It sounds like you are on the right path indeed.

Raised 820202 to track the issues identified as part of this review. At a guess I am about 50% way through reviewing these messages.

I have finished a first pass through all of the APIs. The only ones I haven't looked deeply at are the RIL:* messages, since they have permissions checks and that is the main protection in this API. Making attachment public since the bugs raised are public, and these are not directly exploitable security vulnerabilities.

(In reply to Paul Theriault [:pauljt] from comment #21) > Created attachment 692264 [details] > Notes v2. > > I have finished a first pass through all of the APIs. The only ones I > haven't looked deeply at are the RIL:* messages, since they have permissions > checks and that is the main protection in this API. Making attachment public > since the bugs raised are public, and these are not directly exploitable > security vulnerabilities. Can we call this b-b+ meta bug resolved in that case? All mentioned bugs that we feel are needed for v1 should be b-b+ instead of this bug.

(In reply to Alex Keybl [:akeybl] from comment #22) > > Can we call this b-b+ meta bug resolved in that case? All mentioned bugs > that we feel are needed for v1 should be b-b+ instead of this bug. I'd be comfortable resolving this as such unless Paul feels like there's major work still outstanding.

Yes I think this is fine to close. I raised any issues or question under 820202 and the children of this bug should be reviewed on a case by case basis for blocking basecamp (although as I commented above, all of this is defence in depth controls, lower priority than directly exposed API security). Note that if it isn't in the spreadsheet, I didnt look at it - ie for any new system messages we should keep an eye on.

Paul: We shouldn't audit just the implementations of the APIs listed in the permissions matrix. We need to audit every user of the message manager. It's quite likely that there are non-B2G-specific pieces of code that uses the message manager and do harmful things in the parent process since most non-B2G code did not consider the child process as untrusted. I.e. the message manager was originally written for e10s desktop Firefox and e10s XUL fennec. Neither of which was running the child process in a sandbox.

The approach I used was to search for all files which have the string |parentprocessmessagemanager| to match the contractid, ie https://mxr.mozilla.org/mozilla-central/search?string=parentprocessmessagemanager I then went through each of these files and looked for where .addMessageListener was called to come up with a list message that would be listened for in the parent process. Does this sound like a comprehensive list of messages (again see the spreadsheet for detail). This included two places in Fennec: /mobile/xul/chrome/content/CapturePickerUI.js /mobile/xul/chrome/content/WebappsUI.js I didn't think these would be present in B2G so I hadn't had a strong look at them yet. I will do that now. (reopened for now, close when sure this is comprehensive.)

More info: /mobile/xul/chrome/content/CapturePickerUI.js This just load chrome://browser/content/CaptureDialog.xul which I dont think is present on B2G. In any case, only the title appears to be used, and it looks safe to me. /mobile/xul/chrome/content/WebappsUI.js This file references OpenWebapps.jsm which doesn't seem to be in the tree any more following bug 697383. If it IS still present this has the similar issues to bug 820206.

Yup, that sounds perfect. Sirry to cause a stir.

Removing myself from the sec-review flag. I think Paul finished this though

This was complete last year as above. I didn't close this since the dependent bugs were closed, but we can probably just keep 820202 open to track that work, so I am closing this.