Closed Bug 564048 Opened 12 years ago Closed 12 years ago
Nix security checks in ns
http://mxr.mozilla.org/mozilla-central/source/modules/libpref/src/nsPrefBranch.cpp#791 All of the Get/SetFooPref APIs call through getValidatedPrefName(), which checks if the pref is "capability.*" and goes out to the script security manager if so. Which then checks that the caller has "CapabilityPreferencesAccess": http://mxr.mozilla.org/mozilla-central/source/caps/src/nsScriptSecurityManager.cpp#3271 It baffles me what the point of this is. The only place this makes sense is if the caller is content; but nowadays we already require UniversalXPConnect in order to call into the prefservice at all, right? And we have wrappers to enforce that. Which subsumes this check. sicking suggests that we kill this off, and replace the checks with assertions that we do indeed have UniversalXPConnect. That seems like a pretty simple thing to do. Objections?
I believe this was a belt-and-braces measure to make sure that someone who accidentally managed to get his hands on a prefbranch couldn't permanently escalate their permissions.
I say nuke it then. We have lots of belts and braces protections in xpconnect and wrapperland these days. And capabilities are going away.
Removes nsISecurityPref and nsIPrefSecurityCheck, which nsPrefBranch and nsScriptSecurityManager implement (resp.), and all related code. This also removes a CapabilityPreferencesAccess localization string from dom/locales/en-US/chrome/security/caps.properties. I'm not sure whether we want to keep that or not...
Comment on attachment 447358 [details] [diff] [review] patch v1 Yay!!!
Attachment #447358 - Flags: review?(jonas) → review+
Attachment #447358 - Flags: superreview?(jst) → superreview+
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
(In reply to comment #1) > I believe this was a belt-and-braces measure to make sure that someone who > accidentally managed to get his hands on a prefbranch couldn't permanently > escalate their permissions. Or even navigator.preference, although you nixed that simultaneously.
Depends on: 568069
This landing was mismerged in the following changeset: http://hg.mozilla.org/mozilla-central/rev/155d97b3f8c9 What's needed to fix this?
I filed bug 635665 on the mismerge.
You need to log in before you can comment on or make changes to this bug.