Closed
Bug 736609
Opened 12 years ago
Closed 12 years ago
Malloc error with ArrayBuffer, Uint32Array and Uint8Array
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
People
(Reporter: gkw, Assigned: dmandelin)
References
Details
(Keywords: crash, testcase, Whiteboard: [sg:critical][qa?])
Attachments
(2 files)
10.32 KB,
text/plain
|
Details | |
1004 bytes,
patch
|
sfink
:
review+
akeybl
:
approval-mozilla-aurora+
akeybl
:
approval-mozilla-beta+
akeybl
:
approval-mozilla-esr10+
|
Details | Diff | Splinter Review |
try { a = ArrayBuffer(76); b = Uint32Array(a); uneval() c = Uint8Array(a); c.set(b) } catch (e) {} crashes js debug and opt shell on m-c changeset c7ef262e3024 with a malloc error: Tested on 32-bit. malloc: *** error for object 0xb74ae4: incorrect checksum for freed object - object was probably modified after being freed. *** set a breakpoint in malloc_error_break to debug s-s because malloc errors seem scary. A Valgrind log is attached.
![]() |
Reporter | |
Comment 1•12 years ago
|
||
a = Int32Array(6) Uint8Array(a.buffer).set(a) is a testcase which causes a similar error - the testcase manifests as the following on Linux: *** glibc detected *** ./js-opt-32-im-linux: free(): invalid next size (fast): 0x095e3a18 *** ======= Backtrace: ========= /lib32/libc.so.6(+0x6ff02)[0xf751cf02] /lib32/libc.so.6(+0x70ba2)[0xf751dba2] /lib32/libc.so.6(cfree+0x6d)[0xf7520c5d] ./js-opt-32-im-linux[0x81844d0] /snip
Comment 2•12 years ago
|
||
autoBisect.py -p -a64 --valgrind t.js Due to skipped revisions, the first bad revision could be any of: changeset: 96f52c040d20 user: Nikhil Marathe date: Mon Aug 01 15:26:54 2011 -0700 summary: Bug 667047 - Ensure proper __proto__ behaviour as a normal property after setting it to null once. r=mrbkap changeset: 794fe3408d3a user: Nikhil Marathe date: Mon Aug 01 15:49:51 2011 -0700 summary: Bug 664249 - Inline TypedArray properties into slots. r=mrbkap
Blocks: 664249
Updated•12 years ago
|
Whiteboard: js-triage-needed → [sg:critical] js-triage-needed
Updated•12 years ago
|
status-firefox-esr10:
--- → affected
Comment 3•12 years ago
|
||
Assigning to dmandelin to find a real assignee. This flaw has been in the code a while but was recently found when we changed our fuzzers to look for more things like the flaw that was used in the Pwn2Own contest. Others may be out there looking for things like this too (at the very least, Willem and Vincenzo might be).
Assignee: general → dmandelin
status-firefox12:
--- → affected
status-firefox13:
--- → affected
status-firefox14:
--- → affected
tracking-firefox-esr10:
--- → ?
tracking-firefox12:
--- → ?
tracking-firefox13:
--- → +
tracking-firefox14:
--- → +
Assignee | ||
Updated•12 years ago
|
Whiteboard: [sg:critical] js-triage-needed → [sg:critical]
Assignee | ||
Comment 4•12 years ago
|
||
Great test case: it's a bad bug but very easy to spot with this test case + the memory corruption thing Visual Studio has.
Attachment #609492 -
Flags: review?(sphink)
Assignee | ||
Comment 5•12 years ago
|
||
Alex, can I get your advice on landing this? If you look at the patch you'll see that it's *very* easy to spot what's going on here. We have some typed array feature work scheduled to land in the next few weeks that might work as cover bugs if that would be useful. I don't think we could land those bugs on the ESR though.
Comment 6•12 years ago
|
||
Comment on attachment 609492 [details] [diff] [review] Patch Review of attachment 609492 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jstypedarray.cpp @@ +1955,5 @@ > } > > // We have to make a copy of the source array here, since > // there's overlap, and we have to convert types. > + void *srcbuf = cx->malloc_(getByteLength(tarray)); Um, yeah. Your version is better.
Attachment #609492 -
Flags: review?(sphink) → review+
Assignee | ||
Comment 7•12 years ago
|
||
Thanks for the quick review. I guess now we need a cover bug to land it in. Steve, do you think we can land it in one of your typed array bugs? That would cover it pretty well, but we probably don't want to land those bugs on ESR. But I'm not sure how we do land things on ESR. Do we use cover bugs, or do we just land the security fixes directly because it's the last place they go?
Comment 8•12 years ago
|
||
Cover bugs are useful sometimes, if we have something plausible at hand. For ESR and non-trunk branches (e.g. Firefox 12) you don't want to land array feature changes and I can't imagine any bug you could write that would obscure what's going on here if what's actually checked in is a one-line patch. Landing this safe patch right at the release freeze date for Firefox 12 and ESR will have to be good enough. It's a pretty straightforward heap buffer overrun and likely to be easy to incorporate into an exploit.
Comment 9•12 years ago
|
||
[Triage Comment] Please nominate the patch for ESR landing as per https://wiki.mozilla.org/Release_Management/ESR_Landing_Process
Assignee | ||
Comment 10•12 years ago
|
||
Comment on attachment 609492 [details] [diff] [review] Patch [Approval Request Comment] Regression caused by (bug #): probably 667047 or 664249 User impact if declined: leaves in a buffer overflow vulnerability Testing completed (on m-c, etc.): standard js regression test suite Risk to taking this patch (and alternatives if risky): none, AFAICT String changes made by this patch: none
Attachment #609492 -
Flags: approval-mozilla-esr10?
Comment 11•12 years ago
|
||
Comment on attachment 609492 [details] [diff] [review] Patch (In reply to David Mandelin from comment #5) > Alex, can I get your advice on landing this? If you look at the patch you'll > see that it's *very* easy to spot what's going on here. We have some typed > array feature work scheduled to land in the next few weeks that might work > as cover bugs if that would be useful. I don't think we could land those > bugs on the ESR though. Let's move forward with landing this fix on Aurora/Beta/ESR before our code freeze date (4/13). Pre-approving for Aurora 13 and Beta 12, assuming this patch applies.
Attachment #609492 -
Flags: approval-mozilla-esr10?
Attachment #609492 -
Flags: approval-mozilla-esr10+
Attachment #609492 -
Flags: approval-mozilla-beta+
Attachment #609492 -
Flags: approval-mozilla-aurora+
Assignee | ||
Comment 12•12 years ago
|
||
http://hg.mozilla.org/integration/mozilla-inbound/rev/8f783fc30dd3 http://hg.mozilla.org/releases/mozilla-esr10/rev/14c0e5f4ad2a http://hg.mozilla.org/releases/mozilla-beta/rev/07381182a1e8 http://hg.mozilla.org/releases/mozilla-aurora/rev/863691ed9f15
Target Milestone: --- → mozilla14
Assignee | ||
Updated•12 years ago
|
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment 13•12 years ago
|
||
JSBugMon: This bug has been automatically verified fixed.
Updated•12 years ago
|
Status: RESOLVED → VERIFIED
Updated•12 years ago
|
Group: core-security
Updated•12 years ago
|
Comment 14•12 years ago
|
||
Could someone please point me to directions for how to create a 32-bit opt debug shell for verification?
Whiteboard: [sg:critical] → [sg:critical][qa?]
![]() |
Reporter | |
Comment 15•12 years ago
|
||
(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #14) > Could someone please point me to directions for how to create a 32-bit opt > debug shell for verification? Which platform (Win/Lin/Mac 10.6/10.7)?
Comment 16•12 years ago
|
||
Mac 10.7, I guess.
![]() |
Reporter | |
Comment 17•12 years ago
|
||
(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #16) > Mac 10.7, I guess. I run something like: LD=ld CROSS_COMPILE=1 CC="clang -Qunused-arguments -fcolor-diagnostics -arch i386" RANLIB=ranlib CXX="clang++ -Qunused-arguments -fcolor-diagnostics -arch i386" AS=$CC AR=ar STRIP="strip -x -S" HOST_CC="clang -Qunused-arguments -fcolor-diagnostics" HOST_CXX="clang++ -Qunused-arguments -fcolor-diagnostics" sh ./configure --target=i386-apple-darwin8.0.0 --enable-optimize --disable-debug in my js/src directory. (I use Clang) (I also removed parameters that you aren't likely to need, e.g. --enable-profiling --enable-methodjit --enable-type-inference --enable-more-deterministic --disable-tests --enable-valgrind )
Comment 18•11 years ago
|
||
Automatically extracted testcase for this bug was committed: https://hg.mozilla.org/mozilla-central/rev/efaf8960a929
Flags: in-testsuite+
You need to log in
before you can comment on or make changes to this bug.
Description
•