749379 - SecReview: [WebAPI] Expose JS API for settings app permissions

Status: VERIFIED FIXED

(mozilla.org :: Security Assurance: Review Request, task)

Description

[Curtis Koenig [:curtisk-use curtis.koenig+bzATgmail.com]]]

1. Who is/are the point of contact(s) for this review?
2. Please provide a short description of the feature / application (e.g. problem solved, use cases, etc.):
3. Please provide links to additional information (e.g. feature page, wiki) if available and not yet included in feature description:
4. Does this request block another bug? If so, please indicate the bug number
5. This review will be scheduled amongst other requested reviews. What is the urgency or needed completion date of this review?
6. Please answer the following few questions: (Note: If you are asked to describe anything, 1-2 sentences shall suffice.)
6a. Does this feature or code change affect Firefox, Thunderbird or any product or service the Mozilla ships to end users?
6b. Are there any portions of the project that interact with 3rd party services?
6c. Will your application/service collect user data? If so, please describe 
7. If you feel something is missing here or you would like to provide other kind of feedback, feel free to do so here (no limits on size):
8. Desired Date of review (if known from https://mail.mozilla.com/home/[e-mail]/Security%20Review.html) and whom to invite.

Comment 1

[Justin Lebar (not reading bugmail)]

Work on this bug has not begun, and nobody has stepped up to do the work.  I think a security review will need to wait until that happens.

Comment 2

[Curtis Koenig [:curtisk-use curtis.koenig+bzATgmail.com]]]

That is fine, we are not creating these bugs to track the sec-reviews and related activities for bugs marked sec-review-needed. This is so we can track resource utilization and do quarterly planning.

Comment 4

[Curtis Koenig [:curtisk-use curtis.koenig+bzATgmail.com]]]

I see that the bug this is blocking is resolved, are we ready for a review?

Comment 5

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

Yup, we are.

I've honestly lost track of how these things work these days. Is there documentation anywhere so that I know what to fill out and where?

Comment 6

[Curtis Koenig [:curtisk-use curtis.koenig+bzATgmail.com]]]

(In reply to Jonas Sicking (:sicking) from comment #5)
> Yup, we are.
> 
> I've honestly lost track of how these things work these days. Is there
> documentation anywhere so that I know what to fill out and where?

Please just answer the stuff in comment 0

Comment 7

[David Chan [:dchan]]

I reviewed the code and it looks fine. 

There are 2 components, a JSM which is imported / loaded in the parent process and a JS file which exists in the child / content as navigator.mozPermissionSettings. If the principal has ALLOW_ACTION for the "permissions" permission AND the pref "dom.mozPermissionSettings.enabled" is true, then the mozPermissionSettings object is returned (null otherwise). A page with this object can call get or set [1]. Get allows the page to determine what access level a particular page/app has for a permission. Set fires off a message to the parent process which sets the permission

Risk of exposing this interface is minimal in my opinion. The pref "dom.mozPermissionSettings.enabled" is only set to true on b2g, false on other platforms. The attack would be the user instaling a malicious app with the "permissions" setting then changing permissions of other installed apps. However the "permissions" permission is only granted to certified apps [2], which must met a much higher code standard bar (reviewed, signed, packaged) than other apps. 

Closing off the review as completed


[1] - http://mxr.mozilla.org/mozilla-central/source/dom/interfaces/permission/nsIDOMPermissionSettings.idl
[2] - http://mxr.mozilla.org/mozilla-central/source/dom/apps/src/PermissionsInstaller.jsm?rev=006e5f86f6a8#171

Comment 8

[Antonio Manuel Amaya Calvo (:amac)]

Sorry I missed this. This is related with bug 812289. As currently implemented, the PermissionSettings API doesn't enforce any restriction whatsoever on the permissions that can be granted to any app. In practice, this means that any app that has the permissions settings makes void the permissions matrix, the applications types, and the manifest permission declaration for apps.

Comment 9

[David Chan [:dchan]]

Thanks for filing the other bugs Antonio.

Compromise of the settings app was something I had dismissed since I thought the chance of that was low. You bring up a good point about dev_mode in the other bugs. I'll follow up there and leave this bug as is.