Closed Bug 1268740 Opened 4 years ago Closed 4 years ago
Crash [@ js::Typed
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.
=== 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?
m-i rev 83b0db55bdc9 still seems to reproduce this issue.
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
Attachment #8746938 - Flags: review?(lhansen)
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?
sec-approval+ for trunk. We should get a patch nominated for aurora after it is on trunk as well.
Attachment #8746938 - Flags: sec-approval? → sec-approval+
https://hg.mozilla.org/integration/mozilla-inbound/rev/3dfd039e8de2b2caaa6eb6e3be302a51f5313995 Bug 1268740 - Change AllocateArrayBuffer to receive byteLength with |count * unit| format. r=lth
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?
JSBugMon: This bug has been automatically verified fixed.
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+
JSBugMon: This bug has been automatically verified fixed on Fx48
You need to log in before you can comment on or make changes to this bug.