Last Comment Bug 624621 - (CVE-2011-1187) HTTP Redirections and remote content can be read by javascript errors.
(CVE-2011-1187)
: HTTP Redirections and remote content can be read by javascript errors.
Status: VERIFIED FIXED
[sg:moderate][qa+] embargoed: cross-b...
: regression
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- major (vote)
: ---
Assigned To: Luke Wagner [:luke]
:
Mentors:
: 387476 (view as bug list)
Depends on: 637572
Blocks: CVE-2008-5507
  Show dependency treegraph
 
Reported: 2011-01-10 20:09 PST by Eduardo Vela N
Modified: 2012-05-18 13:26 PDT (History)
25 users (show)
bzbarsky: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
wontfix
wontfix
-
wontfix
+
fixed
-
wontfix
.x+


Attachments
add JS engine API for originPrincipals (40.45 KB, patch)
2011-06-28 10:53 PDT, Luke Wagner [:luke]
bzbarsky: feedback+
Details | Diff | Review
add JS engine API for originPrincipals (v2) (39.56 KB, patch)
2011-09-26 15:34 PDT, Luke Wagner [:luke]
no flags Details | Diff | Review
add JS engine API for originPrincipals (v2 rebased) (41.08 KB, patch)
2011-10-18 14:09 PDT, Luke Wagner [:luke]
no flags Details | Diff | Review
add JS engine API for originPrincipals (v3) (55.39 KB, patch)
2011-10-19 17:58 PDT, Luke Wagner [:luke]
no flags Details | Diff | Review
add JS engine API for originPrincipals (v4: not crash edition) (55.36 KB, patch)
2011-10-21 11:37 PDT, Luke Wagner [:luke]
no flags Details | Diff | Review
add JS engine API for originPrincipals (v4, rebased on 2087537890f0) (54.46 KB, patch)
2011-12-07 17:21 PST, Luke Wagner [:luke]
mrbkap: review+
bzbarsky: feedback+
Details | Diff | Review
part 2. Add an origin principal argument on nsIScriptContext::EvaluateString and pass that through to the JS engine. (11.39 KB, patch)
2011-12-07 21:56 PST, Boris Zbarsky [:bz] (Out June 25-July 6)
mrbkap: review+
Details | Diff | Review
part 3. Use the pre-redirect filename as the script filename and the channel principal as the origin principal, and base our cross-origin check on the origin principal. (10.28 KB, patch)
2011-12-07 21:57 PST, Boris Zbarsky [:bz] (Out June 25-July 6)
mrbkap: review+
Details | Diff | Review
data for test (5 bytes, text/plain)
2012-04-25 18:22 PDT, Daniel Veditz [:dveditz]
no flags Details

Description Eduardo Vela N 2011-01-10 20:09:30 PST
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.13) Gecko/20101203 Firefox/3.6.13
Build Identifier: 

<script>
window.SyntaxError.prototype.__defineGetter__('name',function(){var e=this;for(var j in e){if(j!='name')console.log(j,e[j])}});
</script>
<script src="http://google.com/"></script>

Reproducible: Always

Steps to Reproduce:
1. execute the afore mentioned code
2. see the console
3. see that fileName includes the final URL of the redirection
Actual Results:  
fileName leaks

Expected Results:  
fileName shouldn't leak, or should say the URL in the <script src="..." attribute
Comment 1 Eduardo Vela N 2011-01-10 20:15:19 PST
<script>ReferenceError.prototype.__defineGetter__('name', function(){
   e=this;for(var j in e){if(j!='name')alert(j+':'+e[j])}
});</script>
<script src="http://jsbin.com/ujoho6/2"></script>

that code will also leak the contents of the remote site.
Comment 2 Eduardo Vela N 2011-01-10 20:19:34 PST
Acknowledgement should go to Daniel Divricean.
Comment 3 Eduardo Vela N 2011-01-10 20:38:06 PST
Opera, Safari, Chrome are vulnerable.

IE is vulnerable as well via window.onerror.
Comment 4 Eduardo Vela N 2011-01-10 20:42:24 PST
Opera is tracking this as DSK-325495
Comment 5 Jonas Sicking (:sicking) PTO Until July 5th 2011-01-10 21:08:32 PST
Johnny, you might know our error handling code well enough to know what's going on here?
Comment 6 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-01-10 21:41:46 PST
What's going on is that nsScriptLoader::EvaluateScript passes aRequest->mFinalURI as the script filename when evaluating the script.  And JS exceptions include the filename of the script in the data they expose.  So that means that as long as you can get hold of exceptions from the script you know what URI the script was loaded from.  We've closed this hole for window.onerror by sanitizing the information the onerror handler gets when the exception is thrown from an off-site script.  But the code in comment 0 or comment 1 gets hold of exceptions by overriding the "name" getter on the exception's prototype object.

Note that we're using mFinalURI here due to the fix for bug 461735.  The fundamental issue in that bug is that we need to know that the script was cross-origin to avoid giving too much information to the onerror handler.

Fundamentally, as long as we expose the script filename to the web site at all, and only have the single field to work with for both the "filename" and the concept of "where did this script come from", this is a no-win situation.  If I had my way, then in the redirect case, we should be using the pre-redirect URI as the filename for reporting purposes, and the post-redirect URI for any security checks...  but JSScript just doesn't let us store both pieces of information.

As far as why the code in comment 0 and comment 1 works at all, the reason the 'name' getter is called is that js_ReportUncaughtException calls js_ValueToString on the exception, which lands us in exn_toString, which gets the "name" and "message" properties off the object in a way that lands us in the getter defined by the page JS above.  We could try to fix this, which would at least help pages that don't examine the exceptions themselves... but I think that would still be a band-aid; the real fix is to store both "filenames" in the script and use the right one for the right purpose.
Comment 7 Andreas Gal :gal 2011-01-13 14:04:11 PST
mrbkap, this is exactly what I was worried about when we allowed the error message of exceptions to cross different origin compartments. I still think we should just clamp those to a generic error.
Comment 8 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-01-13 14:29:25 PST
There is only one compartment here, no?
Comment 9 Andreas Gal :gal 2011-01-13 14:34:39 PST
Ah right. I didn't read the testcase right. We throw in our own compartment.
Comment 10 Lucas Adamski [:ladamski] 2011-01-20 18:21:29 PST
Being able to extract arbitrary data across domains seems like sg:high not moderate per https://wiki.mozilla.org/Security_Severity_Ratings?
Comment 11 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-01-20 18:38:00 PST
This is not extracting arbitrary data.  It's extracting the URI the script was redirected to.  Still a problem, but somewhat limited.
Comment 12 Eduardo Vela N 2011-01-20 18:53:42 PST
It is extracting arbitrary data across domains.

Comment 1:

<script>ReferenceError.prototype.__defineGetter__('name', function(){
   e=this;for(var j in e){if(j!='name')alert(j+':'+e[j])}
});</script>
<script src="http://jsbin.com/ujoho6/2"></script>

Extracts: "asdf"

from "name" in ReferenceError.
Comment 13 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-01-20 18:56:06 PST
That can extract the first bareword in the linked-to file, right?
Comment 14 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-01-20 18:56:40 PST
Again, I'm not saying this is not a serious problem.  I'm saying that this lets you extract _some_ information from other sites, but not arbitrary information...
Comment 15 Eduardo Vela N 2011-01-20 19:01:29 PST
yes that's accurate, and not only the first bareword, since in some cases not only that (eg. if it starts with a 0, or contains a - etc..) will fails with a syntax error.

This, however was enough to create a security vulnerability in Google.. (that here, Daniel reported to us).

So yes, sorry, it's not arbitrary data, it's.. some what limited data.
Comment 16 Eduardo Vela N 2011-01-20 19:01:57 PST
s/fails/fail/

spelling.fail()
Comment 17 David Mandelin [:dmandelin] 2011-01-24 11:57:57 PST
Marking non-blocking because it's sg:crit and the leakage seems fairly limited. Correct me if I'm wrong.
Comment 18 Eduardo Vela N 2011-01-31 20:08:55 PST
Hi all!

Sorry if this is the wrong place to have this discussion.

After reading the comments of this bug, I was hoping I could get the security team's point of view regarding if redirects shouldn't be used to pass security sensitive tokens anymore?

Because plugins and browsers have vulnerabilities that leak redirects more frequent that other vulnerabilities, such as uxss, or at least that's my impression, also it's worth noting that in Mozilla it seems like reading http redirects is considered medium risk, vs. uxss that is considered high risk, while in some web apps, reading redirects almost represents the same risk.

I can't give a lot of details, but several popular and sensitive websites (as well as some open authentication and identification protocols) are affected by this type of bugs in a way that it creates very high risk vulnerabilities, and we feel bad for not being able to fix them ourselves (though, I've been studying Spidermonkey/Gecko2 code, so hopefully one of this days I'll be able to contribute with something more than just bugs).

Anyways, just some thoughts :) since I am considering to start filing bugs internally in Google for features that set sensitive tokens in URL redirects because I just feel like they are vulnerable.
Comment 19 Johnny Stenback (:jst, jst@mozilla.com) 2011-06-23 13:58:57 PDT
Luke offered to help drive this and discuss with Boris what we want to do here. Over to him for now.
Comment 20 Luke Wagner [:luke] 2011-06-27 15:31:42 PDT
I talked to Boris and his non-bandaid solution in comment 6 makes sense to me.  From the JS side, we'd just add an extra "origin principals" field to scripts that DOM sets to mFinalURI.  This would let the DOM set script->filename to the pre-redirect URI (fixing the hole) and use the "origin principals" for making onerror security judgments.

We also discussed how this might work with zero JS engine additions (keeping an external hash table) but this is gross in two ways: dealing with script lifetimes and transitively setting the origin principals for nested scripts.
Comment 21 Luke Wagner [:luke] 2011-06-28 10:53:58 PDT
Created attachment 542517 [details] [diff] [review]
add JS engine API for originPrincipals

This adds an API to attach originPrincipals to a script.  See comment in jsapi.h.  Note: scripts created via Function/eval will get the calling script's originPrincipals (for both direct and indirect eval).  Does that sound right?
Comment 22 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-06-28 20:58:35 PDT
Comment on attachment 542517 [details] [diff] [review]
add JS engine API for originPrincipals

It's not clear to me what enforces the equality js_XDRScript asserts.

jsscript.h should document the "principals implies originprincipals" invariant.

Other than that, this looks good, I think.  I didn't review the nitty-gritty details, obviously.
Comment 23 Luke Wagner [:luke] 2011-06-29 12:03:06 PDT
(In reply to comment #22)
> It's not clear to me what enforces the equality js_XDRScript asserts.

The informal reasoning I passed by mrbkap was: we don't XDR content which should be the only place we have differing originPrincipals.  My worry was 2x calls to the principal transcoder would be bad for startup XDR time.
Comment 24 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-06-29 12:05:51 PDT
> we don't XDR content which should be the only place we have differing
> originPrincipals.

There's nothing preventing extensions from loading scripts over https://, in which case the origin principal will be https:// while the principal is system.

Can we instead make XDR smart about the (common) case where the two principals are pointer-equal and only serialize once in that case?  That seems like a better approach.
Comment 25 Luke Wagner [:luke] 2011-06-29 16:19:49 PDT
(In reply to comment #24)
> Can we instead make XDR smart about the (common) case where the two
> principals are pointer-equal and only serialize once in that case?

Yeah, that's what I was doing until I thought I had found a simplification...
Comment 26 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-09-02 14:02:46 PDT
Luke, ping?
Comment 27 Luke Wagner [:luke] 2011-09-02 16:42:56 PDT
Oops, this dropped off my radar.  Yes, I'll put up a new patch.  This just adds an API so that you can write a patch that actually fixes the bug so, in the meantime, perhaps you could write said patch and see if this actually works?
Comment 28 Luke Wagner [:luke] 2011-09-26 15:34:45 PDT
Created attachment 562555 [details] [diff] [review]
add JS engine API for originPrincipals (v2)

This patch supports script->principals != script->originPrincipals.
Comment 29 Luke Wagner [:luke] 2011-09-26 15:35:34 PDT
err, I meant to say "XDR'ing scripts where script->principals != script->originPrincipals".
Comment 30 Igor Bukanov 2011-09-27 00:46:14 PDT
(In reply to Luke Wagner [:luke] from comment #28)
> Created attachment 562555 [details] [diff] [review] [diff] [details] [review]
> add JS engine API for originPrincipals (v2)

Hm, why not just add a hook that allows to sanitize the information that goes into error instances that the compiler creates? Also why cannot we make JSScript::filename different from the URL stored in JSScript::principals so JSScript::filename would point to the name as used in the script tag?
Comment 31 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-10-18 13:59:54 PDT
> Hm, why not just add a hook that allows to sanitize the information that goes into error
> instances that the compiler creates?

Sanitize based on what?

> Also why cannot we make JSScript::filename different from the URL stored in
> JSScript::principals so JSScript::filename would point to the name as used in the script
> tag?

See comment 6, which discusses the reason at length, with links to the relevant bugs.
Comment 32 Luke Wagner [:luke] 2011-10-18 14:09:01 PDT
Created attachment 567876 [details] [diff] [review]
add JS engine API for originPrincipals (v2 rebased)

Rebased.

bz: IIRC, I try server'd this with a big other stack of patches and there was a hang on one mochitest, so there is apparently a bug I need to flush out.
Comment 33 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-10-18 19:57:10 PDT
Hmm.  So there's still a jsapi bit missing here from my point of view.  What I have to work with is a JSErrorReport (well, and JSContext and a const char* message), not a JSScript.

Can we propagate the origin principal to the JSErrorReport?
Comment 34 Luke Wagner [:luke] 2011-10-19 09:59:31 PDT
Yes, this can be done; PopulateReportBlame seems like the only place that initializes JSErrorReport::filename, so it can just fill in the origin principals as well.

The scary part, of course is ref-counting the principals correctly.  JSErrorReport has no destructor so I'll need to track down all the places we free it and make them call a destructor that can drop the ref.  Blech.
Comment 35 Luke Wagner [:luke] 2011-10-19 17:58:28 PDT
Created attachment 568267 [details] [diff] [review]
add JS engine API for originPrincipals (v3)

JSErrorReport: now with origin principals.
Comment 36 Luke Wagner [:luke] 2011-10-21 11:37:04 PDT
Created attachment 568718 [details] [diff] [review]
add JS engine API for originPrincipals (v4: not crash edition)

The last patch had a nice crash (arguments shadow members with the same dontcha know).  This patch is looking green on try.
Comment 37 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-12-07 17:11:20 PST
I suck.  I finally got to looking at this, and of course it doesn't apply on tip anymore...

Luke, do you recall what changeset you based this on or do you have a rebased version to hand?
Comment 38 Luke Wagner [:luke] 2011-12-07 17:21:06 PST
Created attachment 579908 [details] [diff] [review]
add JS engine API for originPrincipals (v4, rebased on 2087537890f0)
Comment 39 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-12-07 20:16:08 PST
>+#if JS_BITS_PER_WORD == 32
>+  private:
>+    void *padding_;
>+#endif

This makes everything after that point private in 32-bit builds, which doesn't so much compile.  Why is that even needed?
Comment 40 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-12-07 21:56:31 PST
Comment on attachment 579908 [details] [diff] [review]
add JS engine API for originPrincipals (v4, rebased on 2087537890f0)

Looks good to me.  With this, I can totally fix the bug.
Comment 41 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-12-07 21:56:58 PST
Created attachment 579963 [details] [diff] [review]
part 2.  Add an origin principal argument on nsIScriptContext::EvaluateString and pass that through to the JS engine.
Comment 42 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-12-07 21:57:26 PST
Created attachment 579964 [details] [diff] [review]
part 3.  Use the pre-redirect filename as the script filename and the channel principal as the origin principal, and base our cross-origin check on the origin principal.
Comment 43 Luke Wagner [:luke] 2011-12-08 11:55:01 PST
(In reply to Boris Zbarsky (:bz) from comment #39)
> This makes everything after that point private in 32-bit builds, which
> doesn't so much compile.  Why is that even needed?

Oops, I accidentally added the missing 'public:' in a down patch.  The padding is needed b/c JSScript is a GC-thing and its size must be a multiple of sizeof(Value).
Comment 44 Blake Kaplan (:mrbkap) (please use needinfo!) 2011-12-15 07:39:59 PST
Comment on attachment 579908 [details] [diff] [review]
add JS engine API for originPrincipals (v4, rebased on 2087537890f0)

Review of attachment 579908 [details] [diff] [review]:
-----------------------------------------------------------------

It is hard to imagine the amount of extra code we carry thanks to redirects.

::: js/src/frontend/TokenStream.h
@@ +842,5 @@
>      JSPackedBool        maybeStrSpecial[256];/* speeds up string scanning */
>      JSVersion           version;        /* (i.e. to identify keywords) */
>      bool                xml;            /* see JSOPTION_XML */
> +    JSContext           * const cx;
> +    JSPrincipals        * const originPrincipals;

What's with the crazy spaces after the *s?

::: js/src/jscntxt.cpp
@@ +761,5 @@
> +            /*
> +             * Depend on the caller to HOLD originPrincipals if the report
> +             * outlives the stack frame.
> +             */
> +            report->originPrincipals = iter.fp()->script()->originPrincipals;

When would a report outlive the stack frame? As far as I can tell, all callers of this function pass a stack-local address here.

::: js/src/jsscript.h
@@ +560,5 @@
>      js::types::TypeScript *types;
>  
> +#if JS_BITS_PER_WORD == 32
> +  private:
> +    void *padding_;

Still need a public: here, right?
Comment 45 Luke Wagner [:luke] 2011-12-15 09:15:32 PST
(In reply to Blake Kaplan (:mrbkap) from comment #44)
> > +             * Depend on the caller to HOLD originPrincipals if the report
> > +             * outlives the stack frame.
> > +             */
> > +            report->originPrincipals = iter.fp()->script()->originPrincipals;
> 
> When would a report outlive the stack frame? As far as I can tell, all
> callers of this function pass a stack-local address here.

True, but *report may be copied into a new malloc()'d JSErrorReport which is stored in an exception object.  But you're right that the comment is overly general; perhaps it should just say "Callers only pass stack-local JSErrorReport objects.".
Comment 46 Luke Wagner [:luke] 2011-12-15 09:52:23 PST
Any reason I should wait before landing the JS engine patch?
Comment 47 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-12-15 10:01:31 PST
Imo, no.  Just give it a better summary and all?  ;)

Blake, do you want to just review the content patches too?
Comment 48 Luke Wagner [:luke] 2011-12-15 13:56:55 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/ab1bb6660d5c
Note to sheriff: there are more patches to land.
Comment 49 Brandon Sterne (:bsterne) 2011-12-16 08:03:11 PST
This was merged into m-c today:
https://hg.mozilla.org/mozilla-central/rev/ab1bb6660d5c
Comment 50 Daniel Veditz [:dveditz] 2011-12-16 08:06:31 PST
I believe only part 1 was checked in (see comment 48), the other two are still waiting on reviews.
Comment 52 Jesse Ruderman 2011-12-19 10:59:48 PST
Backed out:

http://hg.mozilla.org/integration/mozilla-inbound/rev/19062b47ae9f
Back out c68ddc33f098 and 18332142caa5 (bug 624621) for jsreftest failures
Comment 53 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-12-19 17:44:27 PST
There was, in particular, one jsreftest failure:

REFTEST TEST-UNEXPECTED-FAIL | file:///home/cltbld/talos-slave/test/build/jsreftest/tests/jsreftest.html?test=js1_5/Regress/regress-328897.js | JS_ReportPendingException should Expected match to '/(Script error.|uncaught exception: Permission denied to get property UnnamedClass.classes)/', Actual value 'Permission denied for to get property XPCComponents.classes' item 1

Looks to me like the test assertion is just assuming this bug and needs fixing.  I'll do that and push this again.
Comment 55 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-12-19 19:48:37 PST
Oh, bah.  That fails too, because there's a missing space....
Comment 56 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-12-19 19:51:42 PST
Oh, its not a missing space.  It's actually the tbpl log parser thing just lying straight out, looks like.
Comment 57 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-12-19 19:55:41 PST
In particular, the actual value in the log is:

  Permission denied for <file://> to get property XPCComponents.classes

(compare to the output from the log parser in comment 53)
Comment 58 Phil Ringnalda (:philor) 2011-12-19 20:13:28 PST
Ah, in the popup summary *after* a failure has been starred? Yeah, I'll take care of that, I remember the bug where the *before* it was starred case was fixed.
Comment 59 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-12-19 20:16:42 PST
> Ah, in the popup summary *after* a failure has been starred? 

Yeah.  Didn't realize that's different from the before case!
Comment 60 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-12-20 15:25:14 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/2c7b395bce29
https://hg.mozilla.org/integration/mozilla-inbound/rev/3a24ad267c77

now after tryservering it to make sure J is green.
Comment 61 Phil Ringnalda (:philor) 2011-12-20 17:23:10 PST
Did you check Android? :)

Actual value 'Permission denied for <http://10.250.48.218:30168> to get property XPCComponents.classes'
Comment 62 Phil Ringnalda (:philor) 2011-12-20 22:00:53 PST
Marked as fails-if(browserIsRemote) in https://hg.mozilla.org/integration/mozilla-inbound/rev/6cff9824c2c1
Comment 63 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-12-21 08:35:10 PST
> Did you check Android? :)

Does try run that test on Android?  Apparently not!  See https://tbpl.mozilla.org/?tree=Try&rev=469f98207b4c while it lasts!
Comment 64 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-12-21 08:35:52 PST
bc, what's a reasonable test assertion here that would actually work on the remote browser setup?
Comment 65 Phil Ringnalda (:philor) 2011-12-21 08:55:54 PST
Bug 664857 invites you to actually enjoy and be entirely satisfied with the fact that "-u jsreftest" is broken, and that you must always say "-u jsreftest,jsreftest-1,jsreftest-2,jsreftest-3" until you know which hunk of Android you need, and the same with crashtests and reftests.

https://hg.mozilla.org/mozilla-central/rev/2c7b395bce29
https://hg.mozilla.org/mozilla-central/rev/3a24ad267c77
https://hg.mozilla.org/mozilla-central/rev/6cff9824c2c1
Comment 66 Bob Clary [:bc:] 2011-12-21 09:04:05 PST
The current version is: 

expect = /(Script error.|Permission denied for <file:\/\/> to get property XPCComponents.classes)/;

but bug 570807 comment 2 failed trying to match: 

Permission denied for <http://10.250.48.217:30139> to get property XPCComponents.classes

I think this would work in both the file based reftests and the web based android tests:

expect = /(Script error.|Permission denied for <[^>]+> to get property XPCComponents.classes)/;

jmaher: you agree?
Comment 67 Joel Maher (:jmaher) 2011-12-21 09:17:32 PST
changing this to be a more general regex sounds like a good idea.  For all android automation we do http instead of file.
Comment 68 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-12-21 09:46:56 PST
> and that you must always say "-u jsreftest,jsreftest-1,jsreftest-2,jsreftest-3"

Ah, I didn't check enough on the try chooser.  OK, then.
Comment 69 Daniel Veditz [:dveditz] 2011-12-21 14:00:10 PST
FIXED by the landings in comment 65

sirdarckcat: what is the state of other browsers? should this issue remain embargoed? On the current trajectory the fix won't be available until Firefox 12 (late April) though the fix would be safe enough to land in Aurora (Firefox 11 in mid-March).
Comment 70 Alex Keybl [:akeybl] 2012-02-13 11:21:29 PST
(In reply to Daniel Veditz from comment #69)
> FIXED by the landings in comment 65
> 
> sirdarckcat: what is the state of other browsers? should this issue remain
> embargoed? On the current trajectory the fix won't be available until
> Firefox 12 (late April) though the fix would be safe enough to land in
> Aurora (Firefox 11 in mid-March).

Does this still need to be considered for landing on Beta 11?
Comment 71 Lukas Blakk [:lsblakk] use ?needinfo 2012-03-20 10:34:24 PDT
[Triage Comment]
Please nominate for approval-mozilla-esr10 if this is ready to land on that branch.  See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for details.
Comment 72 Luke Wagner [:luke] 2012-04-06 02:01:18 PDT
*** Bug 387476 has been marked as a duplicate of this bug. ***
Comment 73 Luke Wagner [:luke] 2012-04-06 17:04:49 PDT
Just got an auto-bug email: have we decided to land this on ESR?  The patches aren't exactly small and I thought this bug goes back a long time, so I wasn't sure that the risk/reward ratio was in favor, but I could be wrong.
Comment 74 David Mandelin [:dmandelin] 2012-04-09 10:37:30 PDT
(In reply to Luke Wagner [:luke] from comment #73)
> Just got an auto-bug email: have we decided to land this on ESR?  The
> patches aren't exactly small and I thought this bug goes back a long time,
> so I wasn't sure that the risk/reward ratio was in favor, but I could be
> wrong.

In bug 714614 comment 24, akeybl said "We're committed to taking all sg:crit bugs that affect FF10 on the ESR. We should move forward with a new fix." So if you don't think this bug should be fixed on ESR, I think you need to make the case for an exception.
Comment 75 Luke Wagner [:luke] 2012-04-09 10:45:30 PDT
(In reply to David Mandelin from comment #74)
> So if you don't think this bug should be fixed on ESR, I think you need to make
> the case for an exception.

I did: poor risk/reward ratio.
Comment 76 Daniel Veditz [:dveditz] 2012-04-16 15:06:54 PDT
lowering severity to moderate for parity with chrome's SecSeverity-Medium
http://code.google.com/p/chromium/issues/detail?id=69187#c63
Comment 77 [On PTO until 6/29] 2012-04-17 11:23:08 PDT
Is this still embargoed for public disclosure? In other words, has everyone else fixed this yet?

I see that Chromium fixed this last year. Any word on Opera or IE?
Comment 78 Lukas Blakk [:lsblakk] use ?needinfo 2012-04-19 16:05:59 PDT
With the low ratio of reward to risk here, we'll wontfix and see this on esr17
Comment 79 [On PTO until 6/29] 2012-04-25 17:52:24 PDT
When I evaluate the following in the error console:

document.location="data:text/html,<script>window.SyntaxError.prototype.__defineGetter__('name',function(){var
e=this;for(var j in
e){if(j!='name')console.log(j,e[j])}});</script><script
src='http://google.com/'></script>"

I get "Source File: http://www.google.com" in the error for both 11 and nightly. The error is "Error: SyntaxError: syntax error" in nightly and just "Error: Syntax Error" in 11.

Did we really fix this? If so, how do I verify the fix correctly? I spoke to Dan Veditz about this and he didn't get any further than me.
Comment 80 Boris Zbarsky [:bz] (Out June 25-July 6) 2012-04-25 18:13:33 PDT
> When I evaluate the following in the error console:

The error console runs with chrome privileges.

The right way to test this bug is to put the relevant code into an untrusted webpage and see what gets logged.
Comment 81 Daniel Veditz [:dveditz] 2012-04-25 18:22:29 PDT
Created attachment 618499 [details]
data for test
Comment 82 [On PTO until 6/29] 2012-05-01 13:14:36 PDT
After much discussion with Boris and Dan Veditz, this was verified fixed by me on nightly trunk builds.

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