User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Firefox/24.0 (Beta/Release) Build ID: 20130917102605 Steps to reproduce: See the attached code. Basically, It uses an iterator to set a wrapper protected object as key in a for-in loop, and then uses a proxy to catch the stringified version of the object. One instance of this problem can be found in InstallTrigger.install, but there are probably many other palces in the code. Expected results: Privileged code should never iterate over content objects like this: for (var key in object) { var value = object[key]; // ... }

So the example does not really work when hosted on https://bugzilla.mozilla.org because of mixed content blocking, but it should, when loading as local file or from a non-https URL.

Alright, we need to get serious about this. As-filed, this bug is equivalent to bug 924329. But gabor@molnar makes an excellent point that this is trick is likely repeatable on other APIs that use __exposedProps__. We're in serious danger of being caught with our pants down here. First, we need a task force, probably led by the security team (dchan?), to go through all of the uses of __exposedProps__ in our tree ([1]) and determine which ones of them are likely to be vulnerable. This is basically any API where content can somehow pass an object to chrome (via a function or defining it on an object) where the object is not treated with extreme caution. This applies to |this|-bindings as well. Once we've identified the __exposedProps__ cases in which chrome interacts with content objects, we need to move forward on converting those to WebIDL. Fabrice, how go your efforts to convert the WebApps stuff to WebIDL? Additionally, we need to move on bug 923904, which will seal up our WebIDL story. Andrew, do you have the cycles to take a crack at it? [1] http://mxr.mozilla.org/mozilla-central/search?string=__exposedProps__

Sure, I'll start working on it next week.

(In reply to Bobby Holley (:bholley) from comment #2) > Once we've identified the __exposedProps__ cases in which chrome interacts > with content objects, we need to move forward on converting those to WebIDL. > Fabrice, how go your efforts to convert the WebApps stuff to WebIDL? I have a wip patch, still fighting with some of the events stuff. Another use case we have is Web Activities. The data that is transfered from app to app is a jsval and we don't enforce anything. I'm not sure what we can do there.

Okay, I'll get this prioritized for our sprint next week.

(In reply to Fabrice Desré [:fabrice] from comment #4) > Another use case we have is Web Activities. The data that is transfered from > app to app is a jsval and we don't enforce anything. I'm not sure what we > can do there. What does the JSVal represent conceptually? Can we at least guarantee that it can cloned without any special hooks (that is to say, it can be represented in JSON modulo cycles)? If so, WebIDL |object| or |any| should be fine here once we fix bug 923904.

The jsval holds anything that can be moved around doing a structured clone (we use blobs a lot for instance). So it looks like |object| or |any| will be fine for us.

Note that currently, Blob and co are only hooked up for PostMessage. Hooking it up for the general case might involve some thinking about lifetimes, since I'm pretty sure we just serialize raw XPCOM pointers at the moment. I don't know if we addref them or not, or what the invariants are.

I'm putting together the pieces for a coordinated effort on this - I'll make an announcement on security-group tomorrow.

Setting this to sec-moderate, because comment 2 says it is similar to bug 924329, which is moderate.

Cross-origin location reads are sec-high.

I'm hoping that bug 930091 will fix this.

Also note that it is somewhat easier to use this one than bug 924329, since in this case there's no extension installation popup.

Comment 12 says bug 930091 should fix this, and bholley is working on that, so I'm assigning this to him.

(In reply to Andrew McCreight [:mccr8] from comment #14) > Comment 12 says bug 930091 should fix this, and bholley is working on that, > so I'm assigning this to him. No, it won't. See bug 930091 comment 4. The closest thing we have to tracking the work for this is bug 929542.

Sorry, I didn't read things closely enough. Thanks for the explanation.

It's not clear at present whether this exploit is actually unique from bug 924329 in Gecko today (i.e. whether some other API is exploitable). This is a pretty good trick though, and certainly widens the thread surface.

bounty- this bug and nominating 924329 for bounty.

Assigning to bholley because I think he's tracking whatever work is going on to handle this.

I'm doing a talk about JavaScript security at the end of May. Do you think I could mention this issue with for loops in the talk? This is really generic stuff, and also really rare in the codebase: only 1 instance found to date, and I guess that's going to be fixed by then. Please let me know if this is problematic.

(In reply to Gábor Molnár from comment #20) > I'm doing a talk about JavaScript security at the end of May. Do you think I > could mention this issue with for loops in the talk? This is really generic > stuff, and also really rare in the codebase: only 1 instance found to date, > and I guess that's going to be fixed by then. Please let me know if this is > problematic. Please don't. We're working on it like crazy, but Mozilla is still in a pretty vulnerable place with respect to JS-implemented APIs. A lot of APIs have been converted to WebIDL (see bug 981845), but most of it won't hit release before the end of may. And bug 924329 isn't even fixed yet. :-( I'm hoping that the Slaughterhouse project (the initiative to harden Mozilla against attacks like this) will be complete sometime this Fall. But before then, getting too public about this stuff has the potential to bring a lot of harm. I care about this initiative and am working on it daily (mostly in the wrapper department, but trying to push along the other teams as well).

Thanks for the quick answer. I appreciate the security team's work on this and the related issues. I'll look for some long-time fixed bugs as examples for the talk instead of this one so that this issue won't be uncovered.

Thanks Gábor! I really appreciate that, with all of your helpful research in this area. If you have time, I'd be curious to hear what you think of the proposal in bug 987111 comment 0.

Bug 856067 will fix this.

(In reply to Bobby Holley (:bholley) from comment #24) > Bug 856067 will fix this. (Specifically, because the Proxy will become opaque to code that doesn't trust it).

With those three dependencies landed, I am ready to call this fixed. Go Slaughterhouse.

Awesome!

For posterity: This bug was a more general version of the attack in bug 924329, but the only weak API it identified was the same one that was explicitly fixed in bug 924329. There was a wider class of APIs that this attack _could_ have applied to, and auditing those APIs would have been hard (so we didn't do it). So I transformed this bug into a tracking bug for the general infrastructural issues that would make attacks like this possible. Those holes are now plugged.

Bug 1036214 just got backported to aurora. \o/

Confirmed issues in Fx24. Verified fixed on Fx32.0.3 and Fx33.

Could you open up this bug?

Yep. Thanks again Gábor for the awesome work! http://bholley.net/blog/2016/the-right-fix.html

Thanks fixing it the right way, great job! :)