Closed
Bug 1360852
Opened 7 years ago
Closed 7 years ago
Crash [@ js::jit::IonSetPropertyIC::update] with invalid write
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
VERIFIED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox-esr45 | --- | unaffected |
firefox-esr52 | --- | unaffected |
firefox53 | --- | wontfix |
firefox54 | + | verified |
firefox55 | + | verified |
People
(Reporter: decoder, Unassigned)
References
Details
(4 keywords, Whiteboard: [jsbugmon:update,bisect][adv-main54+])
Crash Data
Attachments
(2 files)
1.03 KB,
patch
|
evilpie
:
review+
abillings
:
approval-mozilla-beta+
abillings
:
sec-approval+
|
Details | Diff | Splinter Review |
544 bytes,
patch
|
evilpie
:
review+
|
Details | Diff | Splinter Review |
The following testcase crashes on mozilla-central revision 308cdb913d71 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-stdcxx-compat --disable-profiling --enable-debug --enable-optimize, run with --fuzzing-safe --no-threads --ion-eager): function f() { var _76 = {}; for (var i = 0; i < arguments.length; i++) { var typ = arguments[i]; _76[typ] = typ; } } g = f("number", "boolean", "object"); f(0, -0) f(0, 2147483649) Backtrace: received signal SIGSEGV, Segmentation fault. 0x000000000073711a in js::jit::IonSetPropertyIC::update (cx=0x7ffff694c000, outerScript=..., ic=0x7ffff5b48518, obj=..., idVal=..., rhs=...) at js/src/jit/IonIC.cpp:210 #0 0x000000000073711a in js::jit::IonSetPropertyIC::update (cx=0x7ffff694c000, outerScript=..., ic=0x7ffff5b48518, obj=..., idVal=..., rhs=...) at js/src/jit/IonIC.cpp:210 #1 0x000012e863a1d2e9 in ?? () #2 0x0000000000000000 in ?? () rax 0x7fffffffc2c8 140737488339656 rbx 0x7fffffffbf90 140737488338832 rcx 0x0 0 rdx 0x0 0 rsi 0x41e0000000200000 4746794007250599936 rdi 0x7ffff5b4855a 140737315636570 rbp 0x7fffffffc270 140737488339568 rsp 0x7fffffffbf20 140737488338720 r8 0x7fffffffc2d0 140737488339664 r9 0x7fffffffc2d8 140737488339672 r10 0x7fffffffc2b8 140737488339640 r11 0x7ffff5e7cc40 140737318997056 r12 0x7fffffffbf70 140737488338800 r13 0x7ffff5b48518 140737315636504 r14 0x7ffff694c000 140737330331648 r15 0x7ffff694c000 140737330331648 rip 0x73711a <js::jit::IonSetPropertyIC::update(JSContext*, JS::Handle<JSScript*>, js::jit::IonSetPropertyIC*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::Handle<JS::Value>)+618> => 0x73711a <js::jit::IonSetPropertyIC::update(JSContext*, JS::Handle<JSScript*>, js::jit::IonSetPropertyIC*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::Handle<JS::Value>)+618>: mov (%rsi),%rax 0x73711d <js::jit::IonSetPropertyIC::update(JSContext*, JS::Handle<JSScript*>, js::jit::IonSetPropertyIC*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::Handle<JS::Value>)+621>: mov (%rax),%rax Marking this sec-critical because this looks like an invalid write to a weird address (which might even be controllable given the values in the address).
Updated•7 years ago
|
Flags: needinfo?(jdemooij)
Updated•7 years ago
|
Flags: needinfo?(jdemooij)
Comment 1•7 years ago
|
||
GuardIsInt32Index needs to allocate the input register before calling addFailurePath. This is a regression from bug 1330793, although it's possible this wasn't a problem until bug 1341067 or bug 1357024. We should probably add some more assertions to the compiler/regalloc to check these things in debug builds.
Comment 2•7 years ago
|
||
Attachment #8863334 -
Flags: review?(evilpies)
Comment 3•7 years ago
|
||
[Tracking Requested - why for this release]:
status-firefox53:
--- → wontfix
status-firefox54:
--- → affected
status-firefox-esr45:
--- → unaffected
status-firefox-esr52:
--- → unaffected
tracking-firefox54:
--- → ?
tracking-firefox55:
--- → ?
Comment 5•7 years ago
|
||
Tracking 54/55+. I see Android crashes in this signature in the duped Bug in Comment 4.
Updated•7 years ago
|
Attachment #8863333 -
Flags: review?(evilpies) → review+
Comment 6•7 years ago
|
||
Comment on attachment 8863333 [details] [diff] [review] Part 1 - Fix [Security approval request comment] > How easily could an exploit be constructed based on the patch? Pretty difficult or impossible, at least on current release and beta. > Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? No. > Which older supported branches are affected by this flaw? The code was in 53 but it's likely only a problem on 54. > If not all supported branches, which bug introduced the flaw? Bug 1330793. > Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? Should be trivial. > How likely is this patch to cause regressions; how much testing does it need? Very unlikely to cause regressions. Approval Request Comment [Feature/Bug causing the regression]: Bug 1330793 but as mentioned above it's likely not a problem in 53. [User impact if declined]: Crashes, security bugs. [Is this code covered by automated tests?]: Yes. [Has the fix been verified in Nightly?]: Not yet. [Needs manual test from QE? If yes, steps to reproduce]: No. [List of other uplifts needed for the feature/fix]: None. [Is the change risky?]: No. [Why is the change risky/not risky?]: Very simple one-line fix. [String changes made/needed]: None.
Attachment #8863333 -
Flags: sec-approval?
Attachment #8863333 -
Flags: approval-mozilla-beta?
Comment 7•7 years ago
|
||
Comment on attachment 8863333 [details] [diff] [review] Part 1 - Fix Giving sec-approval+ and Beta approval.
Attachment #8863333 -
Flags: sec-approval?
Attachment #8863333 -
Flags: sec-approval+
Attachment #8863333 -
Flags: approval-mozilla-beta?
Attachment #8863333 -
Flags: approval-mozilla-beta+
Comment 8•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/70bcf05f0210a49d3e5aae0f204fd0249be3af9c Will get it uplifted to Beta later today in time for b4.
Comment 9•7 years ago
|
||
uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/4c630b9a7d2f
Flags: in-testsuite?
Updated•7 years ago
|
Attachment #8863334 -
Flags: review?(evilpies) → review+
Comment 10•7 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/70bcf05f0210
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Updated•7 years ago
|
Group: javascript-core-security → core-security-release
Updated•7 years ago
|
Status: RESOLVED → VERIFIED
Comment 11•7 years ago
|
||
JSBugMon: This bug has been automatically verified fixed.
Updated•7 years ago
|
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update,bisect][adv-main54+]
Updated•7 years ago
|
Comment 12•7 years ago
|
||
JSBugMon: This bug has been automatically verified fixed on Fx54
Updated•7 years ago
|
Group: core-security-release
You need to log in
before you can comment on or make changes to this bug.
Description
•