The default bug view has changed. See this FAQ.
Bug 615657 (CVE-2011-0054)

Buffer overflow/Memory corruption: cg->upvarList.count <= cg->upvarMap.length

RESOLVED FIXED

Status

()

Core
JavaScript Engine
--
critical
RESOLVED FIXED
6 years ago
3 years ago

People

(Reporter: decoder, Assigned: brendan)

Tracking

(Blocks: 1 bug, {verified1.9.1, verified1.9.2})

unspecified
x86
Linux
verified1.9.1, verified1.9.2
Points:
---
Bug Flags:
sec-bounty +

Firefox Tracking Flags

(blocking2.0 betaN+, blocking1.9.2 .14+, status1.9.2 .14-fixed, blocking1.9.1 .17+, status1.9.1 .17-fixed)

Details

(Whiteboard: [sg:critical] fixed-in-tracemonkey)

Attachments

(9 attachments)

(Reporter)

Description

6 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.12) Gecko/20101027 Ubuntu/10.10 (maverick) Firefox/3.6.12
Build Identifier: 

The following problem affects all Spidermonkey versions I tested (1.9.2 and 2.0) and hence Firefox 3.5.x and Firefox 4. Under test were mozilla-2.0-297086a0fb61 and mozilla-1-9-2-053f07027a38:

There is a serious problem with the code that copies upvarMap.vector in jsscript.cpp. Using a combination of inner/outer functions with eval, we were able to trigger the following assertion (mozilla-2.0-297086a0fb61):

Assertion failure: cg->upvarList.count <= cg->upvarMap.length, at jsscript.cpp:1224

The respective code looks as follows:

if (cg->upvarList.count != 0) {
    JS_ASSERT(cg->upvarList.count <= cg->upvarMap.length);
    memcpy(script->upvars()->vector, cg->upvarMap.vector,
           cg->upvarList.count * sizeof(uint32));
    cg->upvarList.clear();
    cx->free(cg->upvarMap.vector);
    cg->upvarMap.vector = NULL;
}

In an optimized non-debug build, the memcpy simply either overruns the available space in script->upvars()->vector, or reads beyond cg->upvarMap.vector or both.

The attacker can control cg->upvarList.count arbitrarily here. We suspect the issue to be exploitable although it shouldn't be too easy (the attacker can hardly control what is actually written, only the amount).

I'll attach test.js to reproduce the problem and some valgrind logs showing different behavior depending on how large the upvar list gets. Using 24 additional upvars in the script triggers the assertion, using 32 crashes straight away without even asserting (I suspect two different problems here).

Reproducible: Always

Steps to Reproduce:
1. Run test.js
2. Observe crash/assertion
Actual Results:  
Crash/Assertion

Expected Results:  
Normal run.
(Reporter)

Comment 1

6 years ago
Created attachment 494087 [details]
Script to reproduce described crash

This script demonstrates the crash.
(Reporter)

Comment 2

6 years ago
Created attachment 494089 [details]
Valgrind log for 32bit with 24 upVars on 1.9.2

Valgrind log for 32bit with 24 upVars on 1.9.2
(Reporter)

Comment 3

6 years ago
Created attachment 494090 [details]
Valgrind log for 32bit with 32 upVars on 1.9.2

Valgrind log for 32bit with 32 upVars on 1.9.2
(Reporter)

Comment 4

6 years ago
Created attachment 494092 [details]
Valgrind log for 32bit with 32 upVars on 2.0

Valgrind log for 32bit with 32 upVars on 2.0
(Reporter)

Comment 5

6 years ago
Created attachment 494093 [details]
Valgrind log for 64bit with 32 upVars on 2.0

Valgrind log for 64bit with 32 upVars on 2.0
(Reporter)

Updated

6 years ago
Component: General → JavaScript Engine
(Reporter)

Updated

6 years ago
blocking1.9.2: --- → ?
blocking2.0: --- → ?
Brendan, this looks like upvar code, so handing to you to start with. Sounds like a critical bug to me, but please let me know if you disagree.
Assignee: nobody → brendan
status1.9.1: --- → ?
status1.9.2: --- → wanted

Updated

6 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
Whiteboard: [sg:critical]
blocking1.9.2: ? → needed
Created attachment 496209 [details]
testcase (click to crash)

Incorporated the PoC script into a loadable page, and increased the upvars to ensure a crash.  For some reason I hit the unresponsive script dialog twice just executing the loop 1000 times, which is odd because it doesn't do much except grow some strings. I've seen PoC's using tons more memory and loops that don't hit that warning.

I think the following crash happened when I closed the page after running the testcase, not directly when I clicked it.

bp-104c40b5-7653-4d6e-ac52-ff31e2101208

Updated

6 years ago
Attachment #494093 - Attachment mime type: text/x-log → text/plain

Updated

6 years ago
Attachment #494092 - Attachment mime type: text/x-log → text/plain

Updated

6 years ago
Attachment #494090 - Attachment mime type: text/x-log → text/plain

Updated

6 years ago
Attachment #494089 - Attachment mime type: text/x-log → text/plain
(Assignee)

Comment 11

6 years ago
Created attachment 496287 [details] [diff] [review]
fix

I hate MakeUpvarForEval... (which means I hate SunSpider, which wants it -- but you knew that).

/be
Attachment #496287 - Flags: review?(dmandelin)
Comment on attachment 496287 [details] [diff] [review]
fix

It's cool how the assert basically documents what the other part of the patch is for. Made it super-easy to review.
Attachment #496287 - Flags: review?(dmandelin) → review+

Updated

6 years ago
blocking2.0: ? → beta9+
(Assignee)

Comment 13

6 years ago
http://hg.mozilla.org/tracemonkey/rev/fb3b0fd656bf

/be
blocking2.0: beta9+ → ?
Whiteboard: [sg:critical] → [sg:critical] fixed-in-tracemonkey

Updated

6 years ago
blocking2.0: ? → betaN+

Comment 14

6 years ago
http://hg.mozilla.org/mozilla-central/rev/fb3b0fd656bf
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Does this bug affect FF3.5 or did upvars come in with 1.9.2?
blocking1.9.2: needed → .14+
(Assignee)

Comment 16

6 years ago
This affects 3.5 too.

/be
blocking1.9.1: --- → .17+
status1.9.1: ? → wanted
(Assignee)

Comment 17

6 years ago
I don't know how to prove this can't be exploited. With enough work, it probably could be, although it is not as easy to exploit as sg:criticals involving free memory reads during virtual call instruction sequences.

/be
Created attachment 505586 [details] [diff] [review]
191 patch

Pedestrian port to 191, nothing difficult to need re-review.
Attachment #505586 - Flags: approval1.9.1.17?
Created attachment 505588 [details] [diff] [review]
192 patch

...also a trivial port.
Attachment #505588 - Flags: approval1.9.2.14?
Comment on attachment 505586 [details] [diff] [review]
191 patch

Approved for 1.9.2.14 and 1.9.1.17, a=dveditz for release-drivers
Attachment #505586 - Flags: approval1.9.1.17? → approval1.9.1.17+
Attachment #505588 - Flags: approval1.9.2.14? → approval1.9.2.14+
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/83f326171713
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/28ce03bbd37c
status1.9.1: wanted → .17-fixed
status1.9.2: wanted → .14-fixed
QA Contact: general → general
(In reply to comment #9)
> Created attachment 496209 [details]
> testcase (click to crash)
> 
> Incorporated the PoC script into a loadable page, and increased the upvars to
> ensure a crash.  For some reason I hit the unresponsive script dialog twice
> just executing the loop 1000 times, which is odd because it doesn't do much
> except grow some strings. I've seen PoC's using tons more memory and loops that
> don't hit that warning.


Verified for 1.9.2 using this testcase. Crashes in 1.9.2.13 and verified fixed in 1.9.14 (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.14) Gecko/20110121 Firefox/3.6.14 ( .NET CLR 3.5.30729)).

Same with 1.9.1.16 and verified fixed in 1.9.17 (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.17) Gecko/20110121 Firefox/3.5.17 ( .NET CLR 3.5.30729)).
Keywords: verified1.9.1, verified1.9.2
Alias: CVE-2011-0054
Group: core-security
(Reporter)

Updated

6 years ago
Blocks: 676763
Flags: sec-bounty+
You need to log in before you can comment on or make changes to this bug.