Closed Bug 672026 Opened 8 years ago Closed 8 years ago

code evaled from javascript component is not privileged

Categories

(Core :: JavaScript Engine, defect)

6 Branch
x86
All
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla8
Tracking Status
firefox6 + fixed
firefox7 + fixed
firefox8 + fixed

People

(Reporter: gk, Assigned: luke)

References

Details

(Keywords: regression, Whiteboard: [regression from bug 656171 ?] [khuey nom'd for add-on compat][qa-])

Attachments

(3 files, 1 obsolete file)

While starting an add-on that contains obfuscated JS code that in turn creates an instance of a XPCOM component I get the error: "Error: Permission denied to create instance of class." That does not happen if I include the code without obfuscation.

E.g. if I include: 

var x = Components.classes["@mozilla.org/filepicker;1"].
  createInstance(Components.interfaces.nsIFilePicker);

everything works fine. But if I replace that with the obfuscated code:

eval(function(p,a,c,k,e,d){e=function(c){return c};if(!''.replace(/^/,String)){while(c--)d[c]=k[c]||c;k=[function(e){return d[e]}];e=function(){return'\\w+'};c=1};while(c--)if(k[c])p=p.replace(new RegExp('\\b'+e(c)+'\\b','g'),k[c]);return p}('10 9=0.8["@7.6/5;1"].4(0.3.2);',10,11,'Components||nsIFilePicker|interfaces|createInstance|filepicker|org|mozilla|classes|x|var'.split('|'),0,{}))

I get "Error: Permission denied to create instance of class. CID={bd57cee8-1dd1-11b2-9fe7-95cf4709aea3}" in my error console.

This seems only to happen if I put it in my add-on. If I evaluate both the obfuscated and the not obfuscated code in the error console everything works as expected.

I can reproduce the problem both on Linux and Windows using Firefox 6 (and above). It is working with Firefox 5 and below.
Did you try with a nightly ?
Your testcase works correctly for me in latest nightly on linux.
Mmhhh... Yes I did and got the same results. How did you test the code? Did you put it into an extension? If so, where into a component/module or just chrome?
Okay. The problem does not occur if I put the code into chrome but is still reproducible (at least for me) if I have it in a component. I attached two minimal extensions showing the issue (hopefully). They are basically the same but one contains the obfuscated code and one the code without obfuscation. Regarding the basic obfuscated extension I get a new (but probably related) error:

Error: Permission denied to create wrapper for object of class XPCComponents_Classes
This has nothing to do with all the obsfucation stuff. This happens because of the eval. It looks like this has been caused by bug #656171 (unapplying the patch fixes the problem).
Assignee: nobody → general
Component: XPConnect → JavaScript Engine
Keywords: regression
QA Contact: xpconnect → general
Summary: Can't create instances of XPCOM components using obfuscated JS code → code evaled from javascript component is not privilegied.
Blocks: 656171
Ah, okay. That makes sense. Thanks for looking into it!
Status: UNCONFIRMED → NEW
Ever confirmed: true
Whiteboard: [regression from bug 656171 ?] [khuey's shotgun nom]
fwiw, I nominated this for tracking because it may break addons.
Thanks for the additional information, kyle. That's really helpful. We'll evaluate this at Monday's 2PM PT Beta triage session.
Whiteboard: [regression from bug 656171 ?] [khuey's shotgun nom] → [regression from bug 656171 ?] [khuey nom'd for add-on compat]
np, I should have been more verbose the first time
For context: bug 656171 switched PrincipalsForCompiledCode (back) to using the object-principals-finder (instead of cx->compartment->principals, which, until compartment-per-global happens, is wrong in case of document.domain).

In the case of the attached obfuscated addon, this JS is being run very early in startup (nsXREDirProvider::DoStartup calls NS_CreateServicesFromCategory(category="profile-after-change")) and JS_GetSecurityCallbacks(cx) points to nsScriptSecurityManager::Init()::securityCallbacks, which has a NULL findObject principals.  Thus, we get a NULL principals instead of cx->compartment->principals which is the system principals.  I think this craps out in CheckXPCPermissions where null doGetObjectPrincipal(aJSObject) denies access.

The naive "right" fix is to set findObjectPrincipals in nsScriptSecurityManager::Init().  Currently, its set in nsJSRuntime::Init(), but all that ObjectPrincipalsFinder does is call nsScriptSecurityManager::GetObjectPrincipal.  The only rub is getting the right static singleton which seems hackable.

Sound reasonable Blake?
Attached patch a fix (obsolete) — Splinter Review
What comment 11 says.  The attached obfuscated addon works.  Pushing to try.

Looking forward to mrbkap killing caps and findObjectPrincipals altogether...
Attachment #546628 - Flags: review?(mrbkap)
Comment on attachment 546628 [details] [diff] [review]
a fix

Looks good. Only comment is that we already have a gScriptSecMan (and associated nsScriptSecurityManager::GetScriptSecurityManager) that you can use instead of rolling your own sDefaultSecurityManager.
Attachment #546628 - Flags: review?(mrbkap) → review+
I'll ask for beta approval once try is green.
http://hg.mozilla.org/mozilla-central/rev/1a41c02a2612
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
It looks like this patch didn't actually get approval for the branches. Did you mistake tracking+ for approval+?
Summary: code evaled from javascript component is not privilegied. → code evaled from javascript component is not privileged
Oh dear, yes, that is exactly what happened.  I'm sorry for the mistake.  Can we request now and, if not approved, back out?
Attachment #546628 - Flags: approval-mozilla-beta?
Attachment #546628 - Flags: approval-mozilla-aurora?
Fennec Aurora is crashing on startup (on Android). This was the only Aurora checkin I see that could have done something. There were only 2 checkins into Aurora.
If this broke android why are all the tests green ...?
(In reply to comment #20)
> If this broke android why are all the tests green ...?

Looks like the Android crashes are related to cairo/pixman and don't crash all devices, just many.
Do the corresponding beta/nightly builds also crash?
(In reply to comment #23)
> Do the corresponding beta/nightly builds also crash?

nightly crashes early, for other reasons. no word on beta.
Did the crashes end up being bug 654049?
Oops, I saw that it was backed out b/c of Android, but I missed that it was a "Talos reression".
(In reply to comment #25)
> Did the crashes end up being bug 654049?

And it was never on Aurora
Attachment #546628 - Flags: approval-mozilla-beta?
Attachment #546628 - Flags: approval-mozilla-beta-
Attachment #546628 - Flags: approval-mozilla-aurora?
Attachment #546628 - Flags: approval-mozilla-aurora-
dougt and I were unable to get a local build to repro the problem or even get a tinderbox build under a debugger.  Fortunately, having spent a good bit of time considering the patch, I realized there is a significantly simpler and more attractive fix.
Assignee: general → luke
Attachment #546628 - Attachment is obsolete: true
Status: RESOLVED → REOPENED
Attachment #547573 - Flags: review?(mrbkap)
Resolution: FIXED → ---
Green on try (whatever that means ;-)
Attachment #547573 - Flags: review?(mrbkap) → review+
Comment on attachment 547573 [details] [diff] [review]
alternate, simpler fix

Requesting approval for aurora/beta (properly, this time :).  I think think the risk is pretty low risk and the reward is not breaking eval() for JS that run code early in startup.
Attachment #547573 - Flags: approval-mozilla-beta?
Attachment #547573 - Flags: approval-mozilla-aurora?
Can we verify that this doesn't break mobile first?
That's reasonable.  I built an opt build with this patch applied to mozilla-aurora tip:
  http://tbpl.mozilla.org/?tree=Try&rev=c9b41a83a0d1
Here's the apk link:
 http://stage.mozilla.org/pub/mozilla.org/firefox/try-builds/lwagner@mozilla.com-c9b41a83a0d1/try-android/fennec-7.0a2.en-US.eabi-arm.apk

Perhaps a kind soul with a device that was crashing before test out this build?
(In reply to comment #35)
>  http://stage.mozilla.org/pub/mozilla.org/firefox/try-builds/lwagner@mozilla.
> com-c9b41a83a0d1/try-android/fennec-7.0a2.en-US.eabi-arm.apk
> 
> Perhaps a kind soul with a device that was crashing before test out this
> build?

I ran this build without seeing any crashes on two Android devices (Galaxy Tab and T-Mobile G2).
Thanks!
Btw, for reference: it looks like the actual bug that caused the backed-out patch to crash has been tracked down: bug 672787 comment 18.
http://hg.mozilla.org/mozilla-central/rev/6c423d80fe27
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
Comment on attachment 547573 [details] [diff] [review]
alternate, simpler fix

Approved for releases/mozilla-aurora and releases/mozilla-beta. Please land asap
Attachment #547573 - Flags: approval-mozilla-beta?
Attachment #547573 - Flags: approval-mozilla-beta+
Attachment #547573 - Flags: approval-mozilla-aurora?
Attachment #547573 - Flags: approval-mozilla-aurora+
Verified on:
Mozilla/5.0 (Windows NT 6.1; rv:7.0) Gecko/20100101 Firefox/7.0
Mozilla/5.0 (X11; Linux i686 on x86_64; rv:7.0) Gecko/20100101 Firefox/7.0
Mozilla/5.0 (Windows NT 5.1; rv:7.0) Gecko/20100101 Firefox/7.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:7.0) Gecko/20100101 Firefox/7.0

I installed the add-on that contains obfuscated JS code from comment #4. After Firefox restarted, the add-on was enabled. No errors were displayed. Is this enough to verify this bug or do I need to do anything else?

I couldn't verify the fix on Fx8 because the add-on I mentioned above is not compatible with Aurora 8.0a2
qa- as no further verification necessary
Whiteboard: [regression from bug 656171 ?] [khuey nom'd for add-on compat] → [regression from bug 656171 ?] [khuey nom'd for add-on compat][qa-]
Closing bug per the above comments (comment #42 and #43).
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.