Bug (CVE-2016-4728) – [JSC] Error prototypes are called on remote scripts

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[eduardo]

VULNERABILITY DETAILS
ReferenceError is called on remote scripts. this can leak sensitive information from the remote site.

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

this will leak in the console "asdf" which is the content of the remote file

[eduardo]

Acknowledgement should go to Daniel Divricean.

[eduardo]

huh, let me know if this is not the right place to report bugs in safari.

since it seems like everyone cc'd was from the chrome team

[Adam Barth]

I've CCed some JavaScriptCore experts.  I suspect this issue is in JavaScriptCore, not in WebCore, but I could be mistaken.

[eduardo]

thanks adam!

[Lucas Forschler]

<rdar://problem/8847294>

[Geoffrey Garen]

Eduardo, can you explain this exploit more?

What you've shown is that, if you include two <script> elements in the same document, they can both access the same global object. Isn't that how the <script> element works by design?

[eduardo]

Hi Geoffrey!

So yes, they access the same global object, and that's by design.

However, site 1 is able to read content from site 2, which bypasses SOP.

The amount of data that can be read is limited, but it's enough to
create security problems in some websites.

Greetings

[Geoffrey Garen]

I see the same behavior in Firefox.

Unless I'm missing something, this is correct behavior.

[Adam Barth]

The issue is somewhat subtle.  Here's the explanation I wrote for the V8 folks:

Think about it from the perspective of a malicious web site.  The web site is running in your browser and wants to read you email.  Suppose you use a web mail provide who's inbox is located at http://webmail.com/inbox.  Now, the malicious web site includes the following script tag:

<script src="http://webmail.com/inbox"></script>

They're allowed to do this according to the browser security policy, even though http://webmail.com/inbox isn't a JavaScript file at all.  Now, the JavaScript engine will try to parse the inbox and execute it, but it will likely fail.  In this case, imagine in fails with a ReferenceError where the identifier happens to be a word in the subject of your email, like AndroidTabletToBeReleasedFeburary31st.  Using the technique in comment #0, the malicious web site can learn information about this ReferenceError and therefore learn things about your email that it's not supposed to know.

The issue is similar in spirit to JSON hijacking <http://haacked.com/archive/2009/06/25/json-hijacking.aspx>, but the target URL doesn't need to be JSON because we're learning information about it even though it has parse errors.

[Adam Barth]

The approach we're recommending to the V8 folks is to make ReferenceError (and it's prototype) immutable so the enclosing page can't be called back when the other script has a parse error.

[eduardo]

> I see the same behavior in Firefox.
> Unless I'm missing something, this is correct behavior.
https://bugzilla.mozilla.org/show_bug.cgi?id=624621

[eduardo]

(In reply to comment #11)
> > I see the same behavior in Firefox.
> > Unless I'm missing something, this is correct behavior.
> https://bugzilla.mozilla.org/show_bug.cgi?id=624621

Oh and, Firefox is planing to fix this as well.

Copy/pasting Boris Zbarsky reponse:

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.

[Geoffrey Garen]

(In reply to comment #10)
> The approach we're recommending to the V8 folks is to make ReferenceError (and it's prototype) immutable so the enclosing page can't be called back when the other script has a parse error.

Is that enough?

The target script will do many observable things before ReferenceError'ing, since all global access is observable through getters, setters, and other means. Those will all leak information.

A determined attacker can even seed an initial set of globals in order to get farther along before ReferenceError'ing.

[Adam Barth]

> The target script will do many observable things before ReferenceError'ing, since all global access is observable through getters, setters, and other means. Those will all leak information.

Yes.  You're allowed to learn things about a script by executing it, but that's it.  It's important that the parsing rules for JavaScript are sufficiently strict so as to prevent the above from letting the attacker extract meaningful information from other data types.  That's why E4X is such a bad idea.  It relaxes the JavaScript parser too much.

Yes, this security model is insane and fragile.  I take no responsibility for it.  :)

[Geoffrey Garen]

> > The target script will do many observable things before ReferenceError'ing, since all global access is observable through getters, setters, and other means. Those will all leak information.
> 
> Yes.  You're allowed to learn things about a script by executing it, but that's it.  It's important that the parsing rules for JavaScript are sufficiently strict so as to prevent the above from letting the attacker extract meaningful information from other data types.

I don't understand the distinction you're drawing between parsing and execution.

A ReferenceError happens at execution time, not parse time. So, if your goal is to be strict in some way about parse errors, changing the behavior of ReferenceError doesn't meet your goal.

The parser uses the global object as a symbol table. So, even parsing can run getters and setters.

Parsing only happens immediately prior to execution. So, what's the point in being strict about parsing but not execution?

[Adam Barth]

It's slightly hard to communicate via these small text boxes.  We can talk by phone if that would be easier.

This issue is caused by two sadnesses in the web platform:

1) <script src="..."> can load script from third-party web sites.
2) The script element ignores the mime type of the script.

That means every URL on every web site is fair game for the attacker to shove into a script tag.  Of course, many of those URLs contains confidential information (e.g., because the user is authenticated via cookies, basic auth, or TLS certificates or because both the user and the URL are behind firewalls).

Now, as the browser, it's our job to prevent the attacker from learning those pieces of confidential information.  The approach we use is to (more or less) silently reject URLs that do not actually appear to be valid JavaScript.  In particular, we try not to leak information to the enclosing page if the script has a parse error (because it's likely to be some non-JavaScript URL, such as HTML).

E4X really screws up this part of the security model because it wildly expands the set of byte sequences that parat parse as JavaScript, which means the attacker can try to extract information from many more URLs.  That's why E4X is so useful in constructing attacks.

Now, back to this issue, these ReferenceErrors are somewhat on the boundary between our desire to silently reject invalid junk and to execute JavaScript.  It just so happens that there is reason to believe that letting JavaScript introspect on these reference errors leaks too much information in the case when the attacker points a <script> tag at a non-JavaScript URL.  I'm not sure whether we've got live examples to show you, but given our experience with similar attack vectors, I'm pretty confident they exist.

[Geoffrey Garen]

Thanks, Adam. I think I understand the underlying assumptions here better now. We may need to discuss this further when it comes time to fix it.

That said, if someone wants to take a crack at making the error objects and prototypes read-only in JSC, I'm happy to advise -- it shouldn't be too difficult.

[David Levin]

*** Bug 52871 has been marked as a duplicate of this bug. ***

[Eric Seidel (no email)]

From reading the description above, it sounds like this is a JSC-only issue?  Perhaps someone with v8 knowledge could explain if/when/how it was fixed for V8?

[Mark Lam]

Comparing what Chrome Canary, Firefox Nightly, and WebKit ToT are doing, here's what I see:

With a 'name' getter set on SyntaxError.prototype:
                                                                                    Chrome  Firefox  WebKit
1. Syntax Error created and thrown in script from self domain:
   a. Getter is called when converting error to string implicitly:                    Y       Y        Y
   b. Getter is called when accessing error.name:                                     Y       Y        Y
   c. Getter is called when accessing error.message:                                  N       N        N
   d. Getter is called when calling error.toString():                                 Y       Y        Y

   e. Getter is called when preparing to call Window.onerror:                         N       N        Y      <==== difference
   f. Getter is called when converting error to string implicitly in Window.onerror:  N       N        N
   g. Getter is called when accessing error.name in Window.onerror:                   N       N        N
   h. Getter is called when accessing error.message in Window.onerror:                N       N        N
   i. Getter is called when calling error.toString() in Window.onerror:               N       N        N

2. Syntax Error created and thrown in script from another domain:
   a. Getter is called in other domain when converting error to string implicitly:    Y       Y        Y
   b. Getter is called in other domain when accessing error.name:                     Y       Y        Y
   c. Getter is called in other domain when accessing error.message:                  N       N        N
   d. Getter is called in other domain when calling error.toString()                  Y       Y        Y

   e. Getter is called in self domain when servicing Window.onerror:                  N       N        Y      <==== difference
   f. Getter is called when converting error to string implicitly in Window.onerror:  N       N        N
   g. Getter is called when accessing error.name in Window.onerror:                   N       N        N
   h. Getter is called when accessing error.message in Window.onerror:                N       N        N
   i. Getter is called when calling error.toString() in Window.onerror:               N       N        N

3. Syntax Error thrown while parsing script from another domain:
   a. Getter is called in other domain when converting error to string implicitly:    N/A     N/A      N/A
   b. Getter is called in other domain when accessing error.name:                     N/A     N/A      N/A
   c. Getter is called in other domain when accessing error.message:                  N/A     N/A      N/A
   d. Getter is called in other domain when calling error.toString()                  N/A     N/A      N/A

   e. Getter is called in self domain when servicing Window.onerror:                  N       N        Y      <==== difference
   f. Getter is called when converting error to string implicitly in Window.onerror:  N       N        N
   g. Getter is called when accessing error.name in Window.onerror:                   N       N        N
   h. Getter is called when accessing error.message in Window.onerror:                N       N        N
   i. Getter is called when calling error.toString() in Window.onerror:               N       N        N

The behavior is the same if I repeat the above with Error.prototype and ReferenceError.prototype instead.



With a 'toString' getter set on SyntaxError.prototype:
                                                                                    Chrome  Firefox  WebKit
1. Syntax Error created and thrown in script from self domain:
   a. Getter is called when converting error to string implicitly:                    Y       Y        Y
   b. Getter is called when accessing error.name:                                     N       N        N
   c. Getter is called when accessing error.message:                                  N       N        N
   d. Getter is called when calling error.toString():                                 Y       Y        Y

   e. Getter is called when preparing to call Window.onerror:                         N       N        Y      <==== difference
   f. Getter is called when converting error to string implicitly in Window.onerror:  N       N        N
   g. Getter is called when accessing error.name in Window.onerror:                   N       N        N
   h. Getter is called when accessing error.message in Window.onerror:                N       N        N
   i. Getter is called when calling error.toString() in Window.onerror:               N       N        N

2. Syntax Error created and thrown in script from another domain:
   a. Getter is called in other domain when converting error to string implicitly:    Y       Y        Y
   b. Getter is called in other domain when accessing error.name:                     N       N        N
   c. Getter is called in other domain when accessing error.message:                  N       N        N
   d. Getter is called in other domain when calling error.toString()                  Y       Y        Y

   e. Getter is called in self domain when servicing Window.onerror:                  N       N        Y      <==== difference
   f. Getter is called when converting error to string implicitly in Window.onerror:  N       N        N
   g. Getter is called when accessing error.name in Window.onerror:                   N       N        N
   h. Getter is called when accessing error.message in Window.onerror:                N       N        N
   i. Getter is called when calling error.toString() in Window.onerror:               N       N        N

3. Syntax Error thrown while parsing script from another domain:
   a. Getter is called in other domain when converting error to string implicitly:    N/A     N/A      N/A
   b. Getter is called in other domain when accessing error.name:                     N/A     N/A      N/A
   c. Getter is called in other domain when accessing error.message:                  N/A     N/A      N/A
   d. Getter is called in other domain when calling error.toString()                  N/A     N/A      N/A

   e. Getter is called in self domain when servicing Window.onerror:                  N       N        Y      <==== difference
   f. Getter is called when converting error to string implicitly in Window.onerror:  N       N        N
   g. Getter is called when accessing error.name in Window.onerror:                   N       N        N
   h. Getter is called when accessing error.message in Window.onerror:                N       N        N
   i. Getter is called when calling error.toString() in Window.onerror:               N       N        N

The behavior is the same if I repeat the above with Error.prototype and ReferenceError.prototype instead.

Based on the above, what we need to do is not to make 'name', and 'toString' unwritable or unconfigurable.  Instead, we need to fix Window.onerror to not access the default toString() which will access an overridden toString getter, or name getter.  Instead, we'll have Window.onerror use a sanitized version of Error.prototype.toString that will ignore the getters.

[Mark Lam]

I forgot to mention that ...

(In reply to comment #20)
                                                                                    Chrome  Firefox  WebKit
1. Syntax Error created and thrown in script from self domain:
   e. Getter is called when preparing to call Window.onerror:                         N       N        Y      <==== difference

... this difference is harmless because the getter is reading the message from an error thrown from the same domain.

2. Syntax Error created and thrown in script from another domain:
   e. Getter is called in self domain when servicing Window.onerror:                  N       N        Y      <==== difference

... this difference is potentially harmful because the message from an error thrown from another domain may leak sensitive info.

3. Syntax Error thrown while parsing script from another domain:
   e. Getter is called in self domain when servicing Window.onerror:                  N       N        Y      <==== difference

... this difference is potentially harmful because the message from an error thrown from another domain may leak sensitive info.

Both Chrome and Firefox does not distinguish between errors thrown from the self domain or from another domain.  When servicing Window.onerror, they simply don't use Error.prototype.name / toString, thereby bypassing the user defined getters.

[Mark Lam]

Created attachment 282012 [details]
proposed patch.

[Mark Lam]

Created attachment 282013 [details]
proposed patch w/ rebased test results (for line number changes).

[Keith Miller]

Comment on attachment 282013 [details]
proposed patch w/ rebased test results (for line number changes).

r=me.

[Mark Lam]

Comment on attachment 282013 [details]
proposed patch w/ rebased test results (for line number changes).

View in context: https://bugs.webkit.org/attachment.cgi?id=282013&action=review

> Source/WebCore/ChangeLog:17
> +        Parsing Error are reported to the main script's window.onerror function.  AFAIK,

"Parsing Error are" should be "Parsing errors are".  Will fix before landing.

[David Kilzer (:ddkilzer)]

Comment on attachment 282013 [details]
proposed patch w/ rebased test results (for line number changes).

View in context: https://bugs.webkit.org/attachment.cgi?id=282013&action=review

Otherwise the comments seem fine.

> Source/WebCore/ChangeLog:15
> +        The issue is that script from a site, A, should not be allowed to view the source
> +        of scripts from another site, B.  However, if the script from site B results in
> +        a Error when parsed on load, then site A may glean information from that Error's
> +        message if site A does the following:
> +        1. set a getter for the Error's 'name'
> +        2. set a getter for the Error's 'toString'

Leave this part out of the commit.  (It's fine to put it in the bugs.webkit.org bug, though.

(The layout tests will tell the story anyway, but let's just leave this out as the text below provides enough context for the fix.)

> Source/WebCore/ChangeLog:28
> +        Credit for reporting this issue goes to Daniel Divricean (http://divricean.ro).

Usually we don't put credit in the commit log (just the bug, and in the security advisory), but I suppose it's fine.

[Mark Lam]

Thanks for the feedback.

(In reply to comment #26)
> Comment on attachment 282013 [details]
> proposed patch w/ rebased test results (for line number changes).
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=282013&action=review
> 
> Otherwise the comments seem fine.
> 
> > Source/WebCore/ChangeLog:15
> > +        The issue is that script from a site, A, should not be allowed to view the source
> > +        of scripts from another site, B.  However, if the script from site B results in
> > +        a Error when parsed on load, then site A may glean information from that Error's
> > +        message if site A does the following:
> > +        1. set a getter for the Error's 'name'
> > +        2. set a getter for the Error's 'toString'
> 
> Leave this part out of the commit.  (It's fine to put it in the
> bugs.webkit.org bug, though.

I've removed this.

> > Source/WebCore/ChangeLog:28
> > +        Credit for reporting this issue goes to Daniel Divricean (http://divricean.ro).
> 
> Usually we don't put credit in the commit log (just the bug, and in the
> security advisory), but I suppose it's fine.

I'll leave this in.

[Mark Lam]

Thanks for the reviews.  Landed in r202460: <http://trac.webkit.org/r202460>.