Closed Bug 610223 Opened 9 years ago Closed 9 years ago

Regexp object allows arbitrary memory reading

Categories

(Core :: JavaScript Engine, defect, P2, critical)

x86
All
defect

Tracking

()

VERIFIED FIXED
mozilla2.0b8
Tracking Status
blocking2.0 --- beta8+
status1.9.2 --- unaffected
status1.9.1 --- unaffected
fennec 2.0+ ---

People

(Reporter: decoder, Assigned: cdleary)

References

(Blocks 1 open bug)

Details

(Keywords: regression, Whiteboard: [sg:high] fixed-in-tracemonkey)

Attachments

(5 files, 7 obsolete files)

12.30 KB, patch
gal
: review+
Details | Diff | Splinter Review
5.56 KB, patch
gal
: review+
Details | Diff | Splinter Review
13.46 KB, patch
gal
: review+
Details | Diff | Splinter Review
6.50 KB, patch
gal
: review+
Details | Diff | Splinter Review
856 bytes, text/html
Details
User-Agent:       Mozilla/5.0 (X11; Linux i686; rv:2.0b6) Gecko/20100101 Firefox/4.0b6
Build Identifier: Mozilla/5.0 (X11; Linux i686; rv:2.0b6) Gecko/20100101 Firefox/4.0b6

Firefox 4.0b6 JavaScript Engine Arbitrary Memory Reading

Original release date: <undisclosed>
Last revised: 2010-11-07
Source: Christian Holler <http://users.own-hero.net/~decoder/>

Credits: Christian Holler <decoder@own-hero.net>
         Maximilian Grothusmann <maxi@own-hero.net>


Systems Affected:

 Mozilla Firefox 4.0b6 (http://www.mozilla.com/en/firefox/beta/) - Web Browser Beta

Severity: High


Overview:

I. Description

 The Mozilla Firefox 4 JavaScript engine supports the Regexp.input property to set
 the input string used for a regular expression match. When assigning Regexp.input,
 the engine fails to reset the submatch properties Regexp.$1..$9. When accessing
 these properties, they are provided as dependent strings to Regexp.input at the
 time of access to the submatch property. When matching some large string and then
 setting Regexp.input to another object or an empty string, the whole memory starting
 at the provided object with the length of the submatch is returned.

II. Impact

 The described weakness can be used to read arbitrary memory starting at the location
 of any object in memory that we can refer to. This includes reading beyond our own 
 document,  leaking information of other tabs or add-ons that are open, including any 
 sensitive information that might be contained in there.

III. Proof of concept

 === Explanation ===

 In our proof of concept exploit, we first create a large string "haystack" that has
 approximately 16 MB size (which should be enough for demonstration purposes) with
 a repeating pattern. We then create a regular expression "needle" containing a single 
 match which encloses roughly 512 KB of memory (the limit for a single pattern is 1 MB,
 but we can use the "global" flag and simply match multiple times to read further).

 We then execute the regular expression and then set Regexp.input to some different
 object. In our case, we choose the "document" object to demonstrate reading beyond
 our own document to disclose other tabs and add-ons.

 === Demonstration ===

 Exploit is attached to this bug report. Open the poc.html file and you'll see a dump
 of 512 KB memory, starting at the object "document". You can alter both the starting
 point within "document" by increasing the "repeat" variable on top (if you get a crash
 you've gone too far), or choose a different object to start with (any object that can
 be referenced in the script context, including all global objects).

 The proof-of-concept exploit was tested in Firefox 4b6 and verified to also work
 against nightly builds.

 In our tests, we were able to read browser history, data from other tabs and even 
 sensitive information such as the password of the "Firefox Sync" feature.

IV. Solution

The following patch will fix the described problem, we do however not claim 
completeness for this fix:

diff -r 633e895d5e84 js/src/jscntxt.h
--- a/js/src/jscntxt.h	Thu Sep 09 12:19:50 2010 -0700
+++ b/js/src/jscntxt.h	Sun Nov 07 16:18:09 2010 +0100
@@ -1797,6 +1797,7 @@
 
     void setInput(JSString *newInput) {
         input = newInput;
+        matchPairs.clear();
     }
 
     /* Accessors. */

Timeline:

 2010-11-07: Reported to Mozilla
 2010-11-05: Vulnerability discovered

Reproducible: Always

Steps to Reproduce:
1. Open the attached proof-of-concept exploit (poc.html).
2. Inspect memory dump.
Actual Results:  
Memory is revealed to the script's context.

Expected Results:  
Submatch properties should have been reset when the input property is changed, hence empty string or undefined should be returned.
Attached file HTML file to run JS exploit in browser (obsolete) —
Just as an additional hint for debugging, you can set RegExp.input in the poc exploit to an empty string to trigger an assertion in the debug build of the js engine which should help to quickly get an overview of what's going on :)

Doing so will bring up

Assertion failure: end <= input->length(), at jsregexpinlines.h:274

because the engine tries to create a dependent string from the empty string which has shorter length than the submatch.
Status: UNCONFIRMED → NEW
blocking2.0: --- → ?
Ever confirmed: true
OS: Linux → All
Whiteboard: [sg:critical]
Thanks for the detailed report.
Priority: -- → P2
Target Milestone: --- → mozilla2.0b8
I don't think we have to hold b7 for this, but we might want to accelerate b8 since b7 will have quite a few users. This also affects FF4 mobile beta 2.
tracking-fennec: --- → ?
Assignee: general → decoder-mozilla
Flags: in-testsuite?
Version: unspecified → Trunk
Attachment #488750 - Flags: review?(gal)
Comment on attachment 488750 [details] [diff] [review]
Possible fix (patch) for the problem

That looks good. Thanks. We should get a + from cdleary too. He wrote the code I think.
Attachment #488750 - Flags: review?(gal)
Attachment #488750 - Flags: review?(cdleary)
Attachment #488750 - Flags: review+
Sorry, I fear I have to retract my patch. It fixes the bug but it introduces an incompatibility as well. Older Mozilla releases as well as JavaScriptCore and v8 let you access the old RegExp.$1-$9 after setting RegExp.input to a new value. The patch disables this by clearing the matchPairs.
I haven't checked what IE does, yet.
I'd hope no one really (ab)uses this API but it's a change in behaviour none the less.
Yeah, we better verify that. Unfortunately a lot of webpages seem to be using this horrible API. I tried to partially eliminate preserving regexp statics and it broke facebook and a few other high profile sites.
Assignee: decoder-mozilla → cdleary
Attachment #488750 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #488750 - Flags: review?(cdleary)
Note that static_input_setter appropriately coerced the input value to a string.
Attachment #488908 - Attachment is obsolete: true
(In reply to comment #11)

> Note that static_input_setter appropriately coerced the input value to a
> string.

Yes, we also noticed that all objects are first coerced into string (except if the object is already a string of course). This didn't stop us though from reading on in heap memory (and revealing sensitive information). But you are right, the description in the initial report is somewhat misleading here :)
Attached patch Account for RegExp.input reset. (obsolete) — Splinter Review
I'm going to turn this into a patch set that:

- Decouples "pending" input from matchPairs input -- they are now syntactically and conceptually different.
- Fixes and adds a test for (what I believe is) a bug with undefined capture groups with the sticky flag on (code auditing bonus).
- Tightens the encapsulation of RegExpStatics -- RegExp is no longer a friend (replaced by |updateFromMatch|) but PreserveRegExpStatics should be, and several methods have been made private as a result.
- Beefs up invariant checking to ensure that capturing parens remain within |matchPairsInput| bounds during debug runs. The number of capturing parens to check is usually low, so this should generally be low overhead in debug mode.
Attachment #488915 - Attachment is obsolete: true
Attached patch 3. Avoid redundant inputOffset. (obsolete) — Splinter Review
And avoid it entirely for non-sticky regexps.
Attachment #488993 - Flags: review?(gal)
Whoops, that wasn't the latest version of the last patch.
Attachment #488993 - Attachment is obsolete: true
Attachment #489008 - Flags: review?(gal)
Attachment #488993 - Flags: review?(gal)
Does not seem to affect old branches (just get a long string of "133713371337..." as expected).

Would be nice to pin down the change that introduced this in case we need to back-port that fix to the branches for some reason, although it's probably tied in a with a bunch of FF4 js-engine rewrite that will never be a possibility for backporting.
Keywords: regression
Whiteboard: [sg:critical] → [sg:high]
This combines the original two PoC attachments into one that can be opened directly from bugzilla to simplify testing. The script part is unchanged from attachment 488748 [details].
Attachment #488748 - Attachment is obsolete: true
Attachment #488749 - Attachment is obsolete: true
(In reply to comment #20)

> This combines the original two PoC attachments into one that can be opened
> directly from bugzilla to simplify testing. The script part is unchanged from
> attachment 488748 [details].

What we forgot in the exploit: If you use something like 

document.write(RegExp.$1.replace(/([^\x20-\x7f])+/g, "<br>"));

(without the escape call of course), then it's much easier to search for strings (including passwords) :)
(In reply to comment #19)
> it's probably tied
> in a with a bunch of FF4 js-engine rewrite that will never be a possibility for
> backporting.

That's correct.
'hg bisect' says:
The first bad revision is:
changeset:   50491:597254d97174
user:        Chris Leary <cdleary@mozilla.com>
date:        Wed Aug 11 13:30:07 2010 -0700
summary:     Bug 564953: Port YARR! Lands macroassembler. (r=gal)
blocking2.0: ? → ---
tracking-fennec: ? → 2.0+
Why was the blocking2.0 nomination removed? Is only Fennec (and not Firefox 4) affected by this?
Comment #0 says this affects Firefox 4 on the desktop, so renominating for blocking.
blocking2.0: --- → ?
status2.0: --- → ?
blocking2.0: ? → beta8+
status2.0: ? → ---
Attachment #488990 - Flags: review?(gal) → review+
Attachment #488991 - Flags: review?(gal) → review+
Attachment #488992 - Flags: review?(gal) → review+
Attachment #489008 - Flags: review?(gal) → review+
http://hg.mozilla.org/tracemonkey/rev/cbd2053aa825
Whiteboard: [sg:high] → [sg:high] fixed-in-tracemonkey
http://hg.mozilla.org/mozilla-central/rev/cbd2053aa825
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Depends on: 613400
Blocks: 676763
Not affecting 1.9.x branches and fixed for a long time on 2.0 branch, opening this up.
Group: core-security
Status: RESOLVED → VERIFIED
rforbes-bugspam-for-setting-that-bounty-flag-20130719
Flags: sec-bounty+
You need to log in before you can comment on or make changes to this bug.