Last Comment Bug 823691 - We should catch bad permissions in the manifest during validation
: We should catch bad permissions in the manifest during validation
Status: VERIFIED FIXED
:
Product: Marketplace
Classification: Server Software
Component: Validation (show other bugs)
: 1.0
: All All
: -- normal (vote)
: 2013-01-03
Assigned To: Matt Basta [:basta]
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-12-20 13:13 PST by krupa raj[:krupa]
Modified: 2013-01-03 11:56 PST (History)
2 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments

Description krupa raj[:krupa] 2012-12-20 13:13:25 PST
I found a packaged app which upon installation would throw this JS error

12-20 20:48:24.533 E/GeckoConsole(  109): [JavaScript Error: "PermissionsInstaller.jsm: 'device-storage' is not a valid Webapps permission name." {file: "resource://gre/modules/PermissionsInstaller.jsm" line: 122}]

We should be more stringent during validation and not allow bad persmissions in the manifest. 

http://mxr.mozilla.org/mozilla-central/source/dom/apps/src/PermissionsTable.jsm#67
Comment 1 Wil Clouser [:clouserw] 2012-12-27 14:16:30 PST
I like this idea.  Jonas - is there any reason not to keep a list of the permissions in the validator and reject apps asking for something not on the list?
Comment 2 Jonas Sicking (:sicking) No longer reading bugmail consistently 2012-12-27 16:13:00 PST
Nope, it would be great if we enforce this on the marketplace side! We should definitely not sign any package where there are permissions that don't (yet) have a meaning.
Comment 3 Matt Basta [:basta] 2012-12-27 20:28:36 PST
The validator originally did this, but we decided about three months ago not to because the docs don't keep up with the implementation (and we're generally slow to update the validator), meaning folks were getting spurious warnings that the permissions they were using weren't valid permissions.

Are you sure that we should reverse this? Is the cost of being a few weeks out of date worth the extra strictness in the long run?
Comment 4 Jonas Sicking (:sicking) No longer reading bugmail consistently 2012-12-27 20:41:20 PST
The set of permissions is now pretty stable. Definitely stable enough that being a few weeks out won't be a big deal.

We really should never sign an app which contains permissions that we don't understand since that can have unknown security implications for the end user. So if we don't catch this in the validator, we should definitely make sure it gets caught during review.

Note You need to log in before you can comment on or make changes to this bug.