Closed Bug 749379 Opened 11 years ago Closed 10 years ago

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

Categories

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

task
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: curtisk, Assigned: dchanm+bugzilla)

References

Details

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/ckoenig@mozilla.com/Security%20Review.html) and whom to invite.
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.
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.
No longer blocks: 707625
Blocks: 707625
Blocks: 770731
No longer blocks: 707625
Summary: SecReview: [WebAPI] Proper WebAPI permissions manager → SecReview: [WebAPI] Expose JS API for settings app permissions
I see that the bug this is blocking is resolved, are we ready for a review?
Flags: needinfo?(jonas)
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?
Flags: needinfo?(jonas)
(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
Flags: needinfo?(jonas)
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
Assignee: nobody → dchan+bugzilla
Status: NEW → RESOLVED
Closed: 10 years ago
Flags: needinfo?(jonas)
Resolution: --- → FIXED
Whiteboard: [pending secreview][needs info]
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.
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.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.