Closed
Bug 1268740
Opened 8 years ago
Closed 8 years ago
Crash [@ js::TypedArrayMethods]
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
VERIFIED
FIXED
mozilla49
Tracking | Status | |
---|---|---|
firefox47 | --- | unaffected |
firefox48 | + | verified |
firefox49 | + | verified |
firefox-esr45 | --- | unaffected |
People
(Reporter: gkw, Assigned: arai)
References
Details
(4 keywords, Whiteboard: [jsbugmon:update])
Crash Data
Attachments
(2 files)
30.44 KB,
text/plain
|
Details | |
7.50 KB,
patch
|
lth
:
review+
lizzard
:
approval-mozilla-aurora+
abillings
:
sec-approval+
|
Details | Diff | Splinter Review |
The following testcase crashes on mozilla-central revision 4292da9df16b (build with --enable-debug --enable-more-deterministic, run with --fuzzing-safe --no-threads --ion-eager): new Int32Array(new Int8Array(~2877108499)); Backtrace: 0 js-dbg-64-dm-clang-darwin-4292da9df16b 0x0000000100939fe3 js::TypedArrayMethods<js::TypedArrayObject>::setFromTypedArray(JSContext*, JS::Handle<js::TypedArrayObject*>, JS::Handle<JSObject*>, unsigned int) + 90979 (TypedArrayCommon.h:232) 1 js-dbg-64-dm-clang-darwin-4292da9df16b 0x0000000100905113 (anonymous namespace)::TypedArrayObjectTemplate<int>::fromTypedArray(JSContext*, JS::Handle<JSObject*>, bool, JS::Handle<JSObject*>) + 1491 (TypedArrayObject.cpp:972) 2 js-dbg-64-dm-clang-darwin-4292da9df16b 0x00000001008edc62 (anonymous namespace)::TypedArrayObjectTemplate<int>::class_constructor(JSContext*, unsigned int, JS::Value*) + 690 (TypedArrayObject.cpp:482) 3 js-dbg-64-dm-clang-darwin-4292da9df16b 0x00000001007e568e js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) + 222 (jscntxtinlines.h:236) 4 js-dbg-64-dm-clang-darwin-4292da9df16b 0x00000001007ef876 js::CallJSNativeConstructor(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) + 118 (jscntxtinlines.h:268) 5 js-dbg-64-dm-clang-darwin-4292da9df16b 0x00000001007d6640 InternalConstruct(JSContext*, js::AnyConstructArgs const&) + 496 (Interpreter.cpp:569) 6 js-dbg-64-dm-clang-darwin-4292da9df16b 0x00000001007d6257 js::ConstructFromStack(JSContext*, JS::CallArgs const&) + 87 (Interpreter.cpp:608) 7 js-dbg-64-dm-clang-darwin-4292da9df16b 0x00000001001f20da js::jit::DoCallFallback(JSContext*, js::jit::BaselineFrame*, js::jit::ICCall_Fallback*, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>) + 1482 (BaselineIC.cpp:5948) 8 ??? 0x0000000102707dab 0 + 4335893931 9 ??? 0x0000000102844b70 0 + 4337191792 For detailed crash information, see attachment. TypedArrays are involved -> s-s by default.
Reporter | ||
Comment 1•8 years ago
|
||
Reporter | ||
Comment 2•8 years ago
|
||
=== Treeherder Build Bisection Results by autoBisect === The "good" changeset has the timestamp "20160415094550" and the hash "2f5f51545d0e466479ec6fdf8b89af72685dfc6d". The "bad" changeset has the timestamp "20160415095344" and the hash "a338aa9ac43ff12190109267d2210d230db9286e". Likely regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=2f5f51545d0e466479ec6fdf8b89af72685dfc6d&tochange=a338aa9ac43ff12190109267d2210d230db9286e Arai-san, is bug 1263803 a likely regressor?
Blocks: 1263803
status-firefox47:
--- → unaffected
status-firefox48:
--- → affected
status-firefox-esr45:
--- → unaffected
Flags: needinfo?(arai.unmht)
Reporter | ||
Comment 3•8 years ago
|
||
m-i rev 83b0db55bdc9 still seems to reproduce this issue.
Assignee | ||
Comment 4•8 years ago
|
||
Thanks, yes, it's from bug 1263803. I should've passed byteLength separately with `count * unit` format, both to AllocateArrayBuffer and maybeCreateArrayBuffer. It allows us to merge the code throwing JSMSG_NEED_DIET into single place in maybeCreateArrayBuffer. in most case, `count=elementLength` and `unit=BYTES_PER_ELEMENT`, but in `CloneArrayBufferNoCopy`, `count=byteLength` and `unit=1`, as we're using `srcArray->byteLength()` value for the allocation.
Assignee: nobody → arai.unmht
Flags: needinfo?(arai.unmht)
Attachment #8746938 -
Flags: review?(lhansen)
Updated•8 years ago
|
Attachment #8746938 -
Flags: review?(lhansen) → review+
Assignee | ||
Comment 5•8 years ago
|
||
Comment on attachment 8746938 [details] [diff] [review] Change AllocateArrayBuffer to receive byteLength with |count * unit| format. [Security approval request comment] > How easily could an exploit be constructed based on the patch? This is buffer over flow, and it could be exploited based on the testcase. (I'm not sure how tho) > Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? The testcase describes how to hit the buffer over flow. > Which older supported branches are affected by this flaw? mozilla-aurora (mozilla48) > If not all supported branches, which bug introduced the flaw? bug 1263803 > Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? Same patch is applicable to mozilla-aurora. > How likely is this patch to cause regressions; how much testing does it need? Less likely. it adds range check for one missing case.
Attachment #8746938 -
Flags: sec-approval?
Updated•8 years ago
|
Keywords: csectype-intoverflow,
sec-critical
Comment 6•8 years ago
|
||
sec-approval+ for trunk. We should get a patch nominated for aurora after it is on trunk as well.
tracking-firefox48:
--- → +
tracking-firefox49:
--- → +
Updated•8 years ago
|
Attachment #8746938 -
Flags: sec-approval? → sec-approval+
Assignee | ||
Comment 7•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/3dfd039e8de2b2caaa6eb6e3be302a51f5313995 Bug 1268740 - Change AllocateArrayBuffer to receive byteLength with |count * unit| format. r=lth
Assignee | ||
Comment 8•8 years ago
|
||
Comment on attachment 8746938 [details] [diff] [review] Change AllocateArrayBuffer to receive byteLength with |count * unit| format. Asking approval in advance to land it to aurora quickly. same patch is applicable to mozilla-aurora. Approval Request Comment > [Feature/regressing bug #] bug 1263803 > [User impact if declined] Possible exploit with buffer overflow. > [Describe test coverage new/current, TreeHerder] Tested in mozilla-inbound automated test. (passed on 14 pushes at this point) > [Risks and why] Low. It adds range check for one missing case. > [String/UUID change made/needed] None
Attachment #8746938 -
Flags: approval-mozilla-aurora?
Comment 9•8 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/3dfd039e8de2
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Updated•8 years ago
|
Status: RESOLVED → VERIFIED
Comment 10•8 years ago
|
||
JSBugMon: This bug has been automatically verified fixed.
Updated•8 years ago
|
Group: javascript-core-security → core-security-release
Comment 11•8 years ago
|
||
Comment on attachment 8746938 [details] [diff] [review] Change AllocateArrayBuffer to receive byteLength with |count * unit| format. Sec-critical, has approval, let's land this on aurora
Attachment #8746938 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Updated•8 years ago
|
Comment 13•8 years ago
|
||
JSBugMon: This bug has been automatically verified fixed on Fx48
Updated•8 years ago
|
Group: core-security-release
You need to log in
before you can comment on or make changes to this bug.
Description
•