Last Comment Bug 502832 - (CVE-2009-2662) TM: Crash [@ memcpy]
(CVE-2009-2662)
: TM: Crash [@ memcpy]
Status: RESOLVED FIXED
: [sg:critical] [needs someone to conti...
: crash, qawanted, testcase-wanted, verified1.9.1
Product: Core
Classification: Components
Component: JavaScript Engine
: Trunk
: x86 All
: -- normal (vote)
: ---
Assigned To: Jason Orendorff [:jorendorff]
: general
: http://bonds.finam.ru/issue/info/
: 521880
:
  Show dependency treegraph
 
Reported: 2009-07-07 08:16 PDT by Bob Clary [:bc:]
Modified: 2010-03-12 21:37 PST (History)
14 users (show)
mbeltzner: blocking1.9.1.1-
mbeltzner: wanted1.9.1.x+
dveditz: wanted1.9.0.x-
samuel.sidler+old: in‑testsuite?
See Also:
Crash Signature:
[@ memcpy]
  ---
  ---
  ---
  ---
  ---
  ---
  ---
  ---
  ---
  ---
  ---
  ---
  ---
  .2+
  .2-fixed


Attachments
v1 (876 bytes, patch)
2009-07-14 10:10 PDT, Jason Orendorff [:jorendorff]
no flags Details | Diff | Splinter Review
v2 (3.22 KB, patch)
2009-07-14 13:09 PDT, Jason Orendorff [:jorendorff]
graydon: review+
samuel.sidler+old: approval1.9.1.2+
Details | Diff | Splinter Review
unreduced testcase.tar.bz2 (98.96 KB, application/x-bzip2)
2009-07-22 21:40 PDT, Bob Clary [:bc:]
no flags Details
Linux stack (7.74 KB, text/plain)
2009-07-23 00:03 PDT, Gary Kwong [:nth10sd]
no flags Details
half-localized testcase off bc's (361.69 KB, text/html)
2009-07-23 02:53 PDT, Gary Kwong [:nth10sd]
no flags Details
slightly more localized, in no way reduced (440.98 KB, text/html)
2009-07-23 05:08 PDT, Gary Kwong [:nth10sd]
no flags Details
slightly more localized and a wee bit reduced (419.11 KB, text/html)
2009-07-23 10:21 PDT, Gary Kwong [:nth10sd]
no flags Details

Summon comment box

Description Bob Clary [:bc:] 2009-07-07 08:16:26 PDT
see bug 502449 comment 3 and bug 502672. Filing new bug and making sensitive due to stacks etc.

start browser, load http://bonds.finam.ru/issue/info/ , close browser.
on windows only afaict.

on win2k3/1.9.1 build from this weekend with vc 8 express I got.

Exploitability Classification: EXPLOITABLE
Recommended Bug Title: Exploitable - User Mode Write AV starting at
MSVCR80D!memcpy+0x000000000000005a (Hash=0x1f2a4941.0x39393718)

User mode write access violations that are not near NULL are exploitable.
ChildEBP RetAddr  
0012c558 004e80c2 js3250!js_Invoke+0x95c
0012c5a4 0050aeea js3250!js_fun_apply+0x2c2
0012ccd4 004f70cc js3250!js_Interpret+0x1179a
0012cdb0 004e80c2 js3250!js_Invoke+0x95c
0012cdfc 0050aeea js3250!js_fun_apply+0x2c2
0012d52c 004f70cc js3250!js_Interpret+0x1179a
0012d608 004e80c2 js3250!js_Invoke+0x95c
0012d654 0050aeea js3250!js_fun_apply+0x2c2
0012dd84 004f70cc js3250!js_Interpret+0x1179a
0012de60 012f6c57 js3250!js_Invoke+0x95c

with fresh builds on windows xp and vc 8 pro and 1.9.1/1.9.2 and tracemonkey

>	msvcr80d.dll!memcpy(unsigned char * dst=0x05dc3bdc, unsigned char * src=0x0012b380, unsigned long count=4201)  Line 188	Asm
 	js3250.dll!TraceRecorder::snapshot(ExitType exitType=TIMEOUT_EXIT)  Line 2948 + 0x1a bytes	C++
 	js3250.dll!TraceRecorder::TraceRecorder(JSContext * cx=0x04f99268, VMSideExit * _anchor=0x00000000, nanojit::Fragment * _fragment=0x06a7f958, TreeInfo * ti=0x06a8b4e8, unsigned int stackSlots=4201, unsigned int ngslots=0, JSTraceType_ * typeMap=0x06886068, VMSideExit * innermostNestedGuard=0x00000000, unsigned char * outer=0x00000000, unsigned long outerArgc=0)  Line 1637 + 0xa bytes	C++
 	js3250.dll!js_StartRecorder(JSContext * cx=0x04f99268, VMSideExit * anchor=0x00000000, nanojit::Fragment * f=0x06a7f958, TreeInfo * ti=0x06a8b4e8, unsigned int stackSlots=4201, unsigned int ngslots=0, JSTraceType_ * typeMap=0x06886068, VMSideExit * expectedInnerExit=0x00000000, unsigned char * outer=0x00000000, unsigned long outerArgc=0)  Line 4140 + 0x4b bytes	C++
 	js3250.dll!js_RecordTree(JSContext * cx=0x04f99268, JSTraceMonitor * tm=0x00d264c8, nanojit::Fragment * f=0x06a7f958, unsigned char * outer=0x00000000, unsigned long outerArgc=0, JSObject * globalObj=0x061f3be0, unsigned long globalShape=61629, Queue<unsigned short> * globalSlots=0x00d30758, unsigned long argc=4196)  Line 4440 + 0x3c bytes	C++
 	js3250.dll!js_MonitorLoopEdge(JSContext * cx=0x04f99268, unsigned int & inlineCallCount=0)  Line 5517 + 0x28 bytes	C++
 	js3250.dll!js_Interpret(JSContext * cx=0x04f99268)  Line 3940 + 0x668 bytes	C++
 	js3250.dll!js_Invoke(JSContext * cx=0x04f99268, unsigned int argc=4196, int * vp=0x0699c044, unsigned int flags=0)  Line 1397 + 0x9 bytes	C++
 	js3250.dll!js_fun_apply(JSContext * cx=0x04f99268, unsigned int argc=4196, int * vp=0x07e71514)  Line 2080 + 0x13 bytes	C++
 	js3250.dll!js_Interpret(JSContext * cx=0x04f99268)  Line 5210 + 0x17 bytes	C++
 	js3250.dll!js_Invoke(JSContext * cx=0x04f99268, unsigned int argc=1, int * vp=0x07e71118, unsigned int flags=0)  Line 1397 + 0x9 bytes	C++
 	js3250.dll!js_fun_apply(JSContext * cx=0x04f99268, unsigned int argc=1, int * vp=0x07e710e0)  Line 2080 + 0x13 bytes	C++
 	js3250.dll!js_Interpret(JSContext * cx=0x04f99268)  Line 5210 + 0x17 bytes	C++
 	js3250.dll!js_Invoke(JSContext * cx=0x04f99268, unsigned int argc=1, int * vp=0x07e710d4, unsigned int flags=0)  Line 1397 + 0x9 bytes	C++
 	js3250.dll!js_fun_apply(JSContext * cx=0x04f99268, unsigned int argc=1, int * vp=0x07e7109c)  Line 2080 + 0x13 bytes	C++
 	js3250.dll!js_Interpret(JSContext * cx=0x04f99268)  Line 5210 + 0x17 bytes	C++
 	js3250.dll!js_Invoke(JSContext * cx=0x04f99268, unsigned int argc=1, int * vp=0x07e7106c, unsigned int flags=0)  Line 1397 + 0x9 bytes	C++
 	js3250.dll!js_fun_apply(JSContext * cx=0x04f99268, unsigned int argc=1, int * vp=0x07e71034)  Line 2080 + 0x13 bytes	C++
 	js3250.dll!js_Interpret(JSContext * cx=0x04f99268)  Line 5210 + 0x17 bytes	C++
 	js3250.dll!js_Invoke(JSContext * cx=0x04f99268, unsigned int argc=1, int * vp=0x07e71028, unsigned int flags=0)  Line 1397 + 0x9 bytes	C++
Comment 1 Bob Clary [:bc:] 2009-07-07 08:21:20 PDT
!exploitable says this second stack is exploitable as well.
Comment 2 Aaron Train [:aaronmt] 2009-07-07 14:07:35 PDT
Bob,

For what it's worth, I was able to reproduce this on Linux (Mozilla/5.0 X11; U; Linux; i686; en-US; rv:1.9.1) Gecko/20090624 Firefox 3.5) but not latest 1.9.2 trunk. So far not able to reproduce on Mac.
Comment 3 Bob Clary [:bc:] 2009-07-08 07:18:58 PDT
http://bonds.finam.ru/trades/today/ also.
Comment 4 Gary Kwong [:nth10sd] 2009-07-10 23:07:36 PDT
I was able to repro this once on http://bonds.finam.ru/trades/today/ on Ubuntu Linux with Fx 3.5 but I couldn't repro it again.
Comment 5 Samuel Sidler (old account; do not CC) 2009-07-13 13:40:47 PDT
sayrer: Can we get an owner for this bug?
Comment 6 Jason Orendorff [:jorendorff] 2009-07-14 10:10:18 PDT
Created attachment 388497 [details] [review]
v1

This one-line patch avoids crashing in memcpy. But the alloca call preceding this is just asking for crashes, too.

Graydon, do you already have a patch somewhere that eliminates alloca calls in jstracer? If so, I think we should land it. If not, I can patch just this one call.
Comment 7 Jason Orendorff [:jorendorff] 2009-07-14 13:09:57 PDT
Created attachment 388526 [details] [review]
v2

This version allocates that memory from cx->tempPool rather than alloca.
Comment 8 Daniel Veditz 2009-07-14 13:11:30 PDT
Any way to capture the crashing page and attach, or better, reduce first?
Comment 9 Graydon Hoare :graydon 2009-07-14 13:25:39 PDT
Comment on attachment 388526 [details] [review]
v2

Fine by me. That's the general thing I was doing with alloca()-squashing anyways.
Comment 10 Andreas Gal :gal 2009-07-14 13:46:57 PDT
This is going to be an unpopular comment given yesterday but did you measure the performance impact?
Comment 11 Jason Orendorff [:jorendorff] 2009-07-14 14:04:04 PDT
(In reply to comment #10)
> This is going to be an unpopular comment given yesterday but did you measure
> the performance impact?

bench.sh shows nothing significant, but it's rather noisy. I'll run SunSpider and v8 in a sec.
Comment 12 Jason Orendorff [:jorendorff] 2009-07-14 14:31:52 PDT
SunSpider shows no change. On v8, I get "not conclusive: might be *1.007x as slow*", but every test is slower by a few tenths of a percent.
Comment 13 Jason Orendorff [:jorendorff] 2009-07-14 16:52:55 PDT
http://hg.mozilla.org/tracemonkey/rev/e51fb83dbd64

I don't see how this crash could actually be exploited. Still, I'm not terribly comfortable having likely stack overruns in the product.

Graydon had the fix for this bug three months ago and we did not land it. The sentiment at the time was that the actual crash (bug 484693) had been fixed, and Graydon's patch removing the other alloca calls was an unnecessary perf loss.

Do we still feel that way?

Do we have some reason to believe the remaining alloca calls are safe? I took a look, and I'm not prepared to vouch for them.

I think the performance benefit of the remaining alloca calls is likely not worth the time it would take to check that they are safe. In other words, if we were to remove them (and I think we should), it would not be worth our time to write--and review with appropriate care--the patches to re-add them.
Comment 15 Aaron Train [:aaronmt] 2009-07-20 14:48:37 PDT
(In reply to comment #13)
> http://hg.mozilla.org/tracemonkey/rev/e51fb83dbd64
> 
> I don't see how this crash could actually be exploited. Still, I'm not terribly
> comfortable having likely stack overruns in the product.
> 
> Graydon had the fix for this bug three months ago and we did not land it. The
> sentiment at the time was that the actual crash (bug 484693) had been fixed,
> and Graydon's patch removing the other alloca calls was an unnecessary perf
> loss.
> 
> Do we still feel that way?
> 
> Do we have some reason to believe the remaining alloca calls are safe? I took a
> look, and I'm not prepared to vouch for them.
> 
> I think the performance benefit of the remaining alloca calls is likely not
> worth the time it would take to check that they are safe. In other words, if we
> were to remove them (and I think we should), it would not be worth our time to
> write--and review with appropriate care--the patches to re-add them.

FWIW, I stumbled upon another test URL on 3.5.1

EXPLOITABLE: Exploitable - User Mode Write AV starting at MSVCR80D!memcpy+0x000000000000005a (Hash=0x1f2a4941.0x11133665): http://www.ylighting.com/kitchendining-allitems-price.html: EXIT STATUS: NORMAL (119.310000 seconds)

http://www.ylighting.com/kitchendining-allitems-price.html
Comment 16 Brendan Eich [:brendan] 2009-07-20 15:04:50 PDT
(In reply to comment #13)
> Do we still feel that way?

Not I -- we shipped, we have a bit more time to be safer and fix any perf regressions.

/be
Comment 17 Gary Kwong [:nth10sd] 2009-07-21 06:21:03 PDT
(In reply to comment #16)
> (In reply to comment #13)
> > Do we still feel that way?
> 
> Not I -- we shipped, we have a bit more time to be safer and fix any perf
> regressions.
> 
> /be

I'm thus guessing that some version of the patch here can be ported in time for 1.9.1.2 then, since it's now baking on m-c (so far for a day or so)
Comment 18 Samuel Sidler (old account; do not CC) 2009-07-22 20:26:18 PDT
Please, please, please get a reduced testcase in this bug before we lose the live site.
Comment 19 Bob Clary [:bc:] 2009-07-22 21:40:09 PDT
Created attachment 390151 [details]
unreduced testcase.tar.bz2

this reproduced the stack.
Comment 20 Gary Kwong [:nth10sd] 2009-07-22 23:10:31 PDT
(In reply to comment #19)
> Created an attachment (id=390151) [details]
> unreduced testcase.tar.bz2
> 
> this reproduced the stack.

I'm getting a crash with 3.5.1 debug build on Linux - way to go, Bob! I'm on it with Lithium now.
Comment 21 Gary Kwong [:nth10sd] 2009-07-22 23:11:02 PDT
(In reply to comment #20)
> (In reply to comment #19)
> > Created an attachment (id=390151) [details] [details]
> > unreduced testcase.tar.bz2
> > 
> > this reproduced the stack.
> 
> I'm getting a crash with 3.5.1 debug build on Linux - way to go, Bob! I'm on it
> with Lithium now.

CC'ing Jesse too.
Comment 22 Bob Clary [:bc:] 2009-07-22 23:49:17 PDT
Gary, Thanks. I've been trying to reduce it on WinXp and have been having a hard time. Did you get the same stack? We need to make sure the reduced test reproduces the exact stack.
Comment 23 Gary Kwong [:nth10sd] 2009-07-23 00:03:02 PDT
Created attachment 390164 [details]
Linux stack

I must admit too, even with your testcase, it's highly unreliable - I need to execute a random set of actions on the webpage to get it to crash. Still working on it.
Comment 24 Gary Kwong [:nth10sd] 2009-07-23 02:53:47 PDT
Created attachment 390190 [details]
half-localized testcase off bc's

This half-localized ~10k-liner crashes Linux reliably without random actions - I wrote in a setTimeout auto-reload to get it to crash. (Thanks gal)
Comment 25 Gary Kwong [:nth10sd] 2009-07-23 03:00:37 PDT
(In reply to comment #24)
> Created an attachment (id=390190) [details]
> localized testcase off bc's

Remember to have a local copy and change the path at the top "window.location" variable to reflect the path on your local copy. (I'm on Ubuntu 9.04 btw) I am unable to reproduce this crash on Mac 10.5 so far.
Comment 26 Gary Kwong [:nth10sd] 2009-07-23 05:08:27 PDT
Created attachment 390205 [details]
slightly more localized, in no way reduced

I'm now tearing my hair out at localizing/reducing this - it's still not fully localized yet.

Remember to have a local copy then change the window.location path at the top of the testcase to your location.

(Any suggestions to help is greatly appreciated!)
Comment 27 Gary Kwong [:nth10sd] 2009-07-23 10:21:20 PDT
Created attachment 390259 [details]
slightly more localized and a wee bit reduced

Someone else please feel free to take over - that's all for my day. :-(
Comment 28 Bob Clary [:bc:] 2009-07-27 11:45:37 PDT
*** Bug 506701 has been marked as a duplicate of this bug. ***
Comment 29 Daniel Veditz 2009-07-27 11:59:55 PDT
Can we get this fix into 3.5.2?
Comment 30 Jason Orendorff [:jorendorff] 2009-07-27 12:55:05 PDT
Comment on attachment 388526 [details] [review]
v2

Sure. I have this sitting in a patch queue ready to push to mozilla-1.9.1 actually.
Comment 31 Samuel Sidler (old account; do not CC) 2009-07-27 13:18:22 PDT
Comment on attachment 388526 [details] [review]
v2

Approved for 1.9.1.2. a=ss for release-drivers.

Please land and use the ".2-fixed" option of the status1.9.1 flag to indicate fixed status.
Comment 32 Jason Orendorff [:jorendorff] 2009-07-27 14:03:44 PDT
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/57c2310ac1d5
Comment 33 Bob Clary [:bc:] 2009-07-30 11:39:48 PDT
v 1.9.1 mac/win/linux
Comment 34 Jesse Ruderman 2010-03-12 21:37:10 PST
*** Bug 502672 has been marked as a duplicate of this bug. ***
Comment 35 Jesse Ruderman 2010-03-12 21:37:38 PST
*** Bug 503310 has been marked as a duplicate of this bug. ***

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