546866 - Validator should flag enablePrivilege and codebase_principal_support

Status: VERIFIED FIXED

(addons.mozilla.org Graveyard :: Admin/Editor Tools, enhancement, P2)

Description

[Jesse Ruderman]

Addons shouldn't be giving web sites huge swaths of privileges, or encouraging users to make their browser less secure.

Also, Firefox is removing these features in bug 546848.

It's kinda scary how many addons use these according to https://mxr.mozilla.org/addons/.  Even a few non-experimental addons such as https://addons.mozilla.org/en-US/firefox/addon/6511 and https://addons.mozilla.org/en-US/firefox/addon/3688.

Comment 1

[chris hofmann]

wondering if this should retrigger a review of the addons that have changed this pref, or if its good enough to just wait for another update to the addon and a follow on submission.

Also wonder if we should reorganize and make a stronger statement about the risk of associated with flipping this pref in https://developer.mozilla.org/en/Bypassing_Security_Restrictions_and_Signing_Code

We hint at risks and say it should only be used for testing but some of that is buried at the bottom of the article.   Should we add a strong   ***WARNING*** message at the top of the article that explains the danger of doing any general purpose browsing with this pref set?

Comment 2

[chris hofmann]

The validator should check for explicit setting of the pref, but if it can it might also add some warnings if an addon does any recommending to users that manual pref setting is required/advised.  See this example.

https://mxr.mozilla.org/addons/source/9147/chrome/content3/Toolbar%20Permission.html?force=1

8 <h1>The handwriting tool doesn't work?</h2>
9 <p>The nciku Toolbar should have automatically been granted the necessary security priveliges when it was installed.</p>
10 <p>However, if this has failed (for example, if Windows Vista has prevented it from gaining these privileges), you will need to grant these manually as follows:</p>
11 
12 <ol>
13         <li>Enter "about:config" into Firefox's location bar. Press Enter.</li>
14         <li>Enter "signed.applets.codebase_principal_support" in the filter box.</li>
15         <li>If the item "signed.applets.codebase_principal_support" is false, double click it to set it to true.</li>
16         <li>Try the handwriting tool again. Enjoy.</li>
17 </ol>

Comment 3

[Daniel Veditz [:dveditz]]

https://mxr.mozilla.org/addons/search?string=enablePrivilege
"Too many hits, showing the first 1000" :-(

I guess we don't have to scour intranets, the "Dark Matter" is more visible than I thought.

Comment 4

[Boris Zbarsky [:bzbarsky]]

If it makes you feel any better, the first several I checked were mostly cargo-cult (as in, they're making the enablePrivilege call in chrome code, often with bogus comments like "need this to work with XPCOM").  A number seem to be cribbing from http://www.captain.at/ajax-file-upload.php but doing it in their chrome code....

There are some exceptions to that (e.g. the YSlow "UniversalBrowserRead so we can read our chrome:// logo into a JS string" thing), of course.

Comment 5

[chris hofmann]

as mentioned on IRC we also should figure out what should happen to addons out there right now that will get flagged by the valdiator when the validator gets changed?

back to the sandbox?
removed?
grandfathered for some grace time?

should the addons that have shipped with pref changing code be required to go back and reset the pref to make users safe again?

Comment 6

[Boris Zbarsky [:bzbarsky]]

My 2c:

1) Validators that are flagged should be grandfathered until we ship our next
   Firefox version (the one that drops enablePrivilege support).  Possibly past
   that if they can easily demonstrate that they only use the enablePrivilege
   code in older Geckos.  That said, we should encourage them to remove the code
   ASAP unless they really truly need it for some reason (unlikely, per
   comment 4).
2) Addons that have added such prefs should reset them, yes.  Note that we will
   be making the prefs no-ops going forward if all goes right, so this only
   affects downrev versions.

Comment 7

[Jesse Ruderman]

I'd prefer not to force addon authors to reset the pref.  It could lead to conflicts between extensions during the grace period, where an updated extension sets it to false while another relies on it being true.

Comment 8

[chris hofmann]

are users seeing conflicts in extensions worse than having a few hundred thousand people surfing to any page on web with codebase_principal_support=true for the next many months until they update to a version of firefox that no longer has support for that feature?

is this the right way to think about the trade off?

Comment 9

[Jorge Villalobos [:jorgev] (he/him)]

(In reply to comment #8)
> are users seeing conflicts in extensions worse than having a few hundred
> thousand people surfing to any page on web with codebase_principal_support=true
> for the next many months until they update to a version of firefox that no
> longer has support for that feature?
> 
> is this the right way to think about the trade off?

I don't think this is such a big security problem. The preference only enables websites to *ask* for additional permissions, and the user gets this big scary warning that defaults to "Deny". I agree with Jesse that forcing authors to reset the preference will cause more harm than good.

Patch for the validator coming up.

Comment 10

[Jorge Villalobos [:jorgev] (he/him)]

Attachment: Patch V1. Add validation for the preference and function, plus explanation in validation page

Here's the first version of the patch. Should be pretty straightforward.

Comment 11

[Jorge Villalobos [:jorgev] (he/him)]

Fixed, r64247.

Comment 12

[Jorge Villalobos [:jorgev] (he/him)]

You can verify here: https://preview.addons.mozilla.org/en-US/developers/versions/validate/55547/run#test-results-39336

Comment 13

[Stephen Donner [:stephend] Not actively reading bugmail]

Attachment: Post-fix screenshot

Comment 14

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED on https://preview.addons.mozilla.org/en-US/developers/versions/validate/55547/run.

Thanks, Jorge!

Comment 15

[Jorge Villalobos [:jorgev] (he/him)]

Reclassifying editor bugs and changing to a new whiteboard flag. Spam, spam, spam, spam...