Closed Bug 1255949 Opened 8 years ago Closed 8 years ago

Crash [@ ??] with weird memory address


(Core :: JavaScript Engine, defect)

Not set



Tracking Status
firefox45 --- wontfix
firefox46 + fixed
firefox47 + fixed
firefox48 + fixed
firefox-esr38 --- unaffected
firefox-esr45 46+ fixed


(Reporter: decoder, Assigned: jandem)


(5 keywords, Whiteboard: [jsbugmon:update,bisect][fuzzblocker][adv-main46+][adv-esr45.1+])

Crash Data


(1 file)

The following testcase crashes on mozilla-central revision 3a11a57b43aa (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-debug, run with --fuzzing-safe --ion-offthread-compile=off --ion-eager):

a = [1, 2, 3, 4, 5];
function foo4(x, m, n) {
    v = 0;
    for (var i = m; i < n; i++)
      v += x[i] + x[i - 1] + x[i - 2];
    return v
for (i = 0; i < 5; i++) 
  foo4(a, 2, 5);
foo4('xxxxxxxxxxxxx', 0, 5);


Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7fc12ae in ?? ()
#0  0x00007ffff7fc12ae in ?? ()
#1  0x00007ffff7e00b68 in ?? ()
#2  0x0000000900000000 in ?? ()
#3  0x00007ffff7fe8bcd in ?? ()
#4  0x0000000000001044 in ?? ()
#5  0x00007ffff7e6d162 in ?? ()
#6  0x0000000000000000 in ?? ()
rax	0x7ffff7e00b68	140737352043368
rbx	0x5	5
rcx	0x7ffff7e8a070	140737352605808
rdx	0x7ffff7e82c90	140737352576144
rsi	0x7ffff7e82c88	140737352576136
rdi	0xc	12
rbp	0x0	0
rsp	0x7fffffffcca0	140737488342176
r8	0x0	0
r9	0xffffffff	4294967295
r10	0x7ffff3300050	140737273397328
r11	0x7ffff695d1e8	140737330401768
r12	0x0	0
r13	0x0	0
r14	0x1044	4164
r15	0x7fffffffcd50	140737488342352
rip	0x7ffff7fc12ae	140737353880238
=> 0x7ffff7fc12ae:	movzbl (%rdx,%r9,1),%edx
   0x7ffff7fc12b3:	cmp    $0x100,%edx

Marking s-s because this crashes with a weird memory address. Also marking as fuzzblocker because there is practically nothing a signature could reliably match on.
I'm going to assume this is crashing inside JIT code based on the lack of symbols. Jan, can you look at this or find somebody to look at it? Thanks.
Flags: needinfo?(jdemooij)
Bug 1131955 added MBoundsCheck::fallible_, if that's false we don't emit any LIR or code for it.

Here TryEliminateBoundsCheck is eliminating a fallible bounds check by updating an infallible one before it, but since it's infallible, we don't emit any code for it...

This is serious.
Keywords: sec-critical
Attached patch PatchSplinter Review
Assignee: nobody → jdemooij
Flags: needinfo?(jdemooij)
Attachment #8730863 - Flags: review?(hv1989)
Tracking since this is a sec-critical crash.
Comment on attachment 8730863 [details] [diff] [review]

Review of attachment 8730863 [details] [diff] [review]:

Good find!
Attachment #8730863 - Flags: review?(hv1989) → review+
Comment on attachment 8730863 [details] [diff] [review]

[Security approval request comment]
> How easily could an exploit be constructed based on the patch?
Writing a test for this is not completely obvious or trivial, but it's also not extremely difficult...

> Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?

> Which older supported branches are affected by this flaw?

> If not all supported branches, which bug introduced the flaw?
Bug 1131955.

> Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
Should be easy to backport.

> How likely is this patch to cause regressions; how much testing does it need?
Unlikely; I don't expect any perf/correctness regressions.
Attachment #8730863 - Flags: sec-approval?
Comment on attachment 8730863 [details] [diff] [review]

sec-approval+ for trunk. We'll want this on branches too.
Attachment #8730863 - Flags: sec-approval? → sec-approval+
Closed: 8 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Flags: needinfo?(jdemooij)
Comment on attachment 8730863 [details] [diff] [review]

Approval Request Comment
[Feature/regressing bug #]: Bug 1131955.
[User impact if declined]: Security bugs, crashes.
[Describe test coverage new/current, TreeHerder]: Fixes the reported test.
[Risks and why]: Low risk. The patch just adds some checks to disable an optimization in certain cases.
[String/UUID change made/needed]: None.
Flags: needinfo?(jdemooij)
Attachment #8730863 - Flags: approval-mozilla-esr45?
Attachment #8730863 - Flags: approval-mozilla-beta?
Attachment #8730863 - Flags: approval-mozilla-aurora?
Group: javascript-core-security → core-security-release
Comment on attachment 8730863 [details] [diff] [review]

Crash fix, sec-critical, taking this for aurora and for 46 beta 6.
Attachment #8730863 - Flags: approval-mozilla-beta?
Attachment #8730863 - Flags: approval-mozilla-beta+
Attachment #8730863 - Flags: approval-mozilla-aurora?
Attachment #8730863 - Flags: approval-mozilla-aurora+
Comment on attachment 8730863 [details] [diff] [review]

Sec-critical, taking it.
Should be in 45.1.0
Attachment #8730863 - Flags: approval-mozilla-esr45? → approval-mozilla-esr45+
Whiteboard: [jsbugmon:update,bisect][fuzzblocker] → [jsbugmon:update,bisect][fuzzblocker][adv-main46+][adv-esr45.1+]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.