Closed Bug 876458 Opened 7 years ago Closed 7 years ago

IonMonkey: Assertion failure: str, at ./dist/include/js/Value.h:640 or Assertion failure: hasBase(), at vm/String.h:948 or Crash on Heap or Crash [@ StringMatch]

Categories

(Core :: JavaScript Engine, defect, critical)

x86_64
Linux
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla24
Tracking Status
firefox21 --- wontfix
firefox22 + fixed
firefox23 + fixed
firefox24 + verified
firefox-esr17 --- unaffected
b2g18 --- wontfix

People

(Reporter: decoder, Assigned: bhackett1024)

References

(Blocks 1 open bug)

Details

(4 keywords, Whiteboard: [jsbugmon:update,ignore][adv-main22+])

Crash Data

Attachments

(2 files, 2 obsolete files)

The following testcase asserts on mozilla-central revision a39263b0c896 (run with --ion-eager):


var y = "";
test();
function test() {
  y;
  // Using the next line instead of the unescape will assert
  //y.toString();
  unescape('x' + y);
}
y = y * 2;
test();
Keywords: crash
Whiteboard: [jsbugmon:update,bisect]
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   132678:63b006573c65
user:        Brian Hackett
date:        Wed May 22 11:36:29 2013 -0600
summary:     Bug 870052 - Various tweaks to reduce recompilation on asm.js style apps, r=jandem.

This iteration took 322.241 seconds to run.
Needinfo from Brian based on comment 2 :)
Flags: needinfo?(bhackett1024)
This is s-s. I have a variation that causes the following crash:


Program received signal SIGSEGV, Segmentation fault.
0x08244c10 in StringMatch (patlen=1, pat=0xf76000b8, textlen=258916923, text=0xf76000ba) at js/src/jsstr.cpp:1002
1002                if (*c == p0)
(gdb) bt
#0  0x08244c10 in StringMatch (patlen=1, pat=0xf76000b8, textlen=258916923, text=0xf76000ba) at js/src/jsstr.cpp:1002
#1  operator() (res=<synthetic pointer>, index=425869, str=, this=<synthetic pointer>, cx=<optimized out>) at js/src/jsstr.cpp:2978
#2  SplitHelper<SplitStringMatcher> (splitMatch=<synthetic pointer>, limit=4294967295, str=<optimized out>, cx=0x902dfa8, type=...) at js/src/jsstr.cpp:2830
#3  js::str_split (cx=0x902dfa8, argc=1, vp=0xffffbcd4) at js/src/jsstr.cpp:3058
#4  0xf770dda1 in ?? ()
#5  0x08310139 in EnterIon (jitcode=0xf770dce8, fp=0xf77110b8, cx=0x902dfa8) at js/src/ion/Ion.cpp:1844
#6  js::ion::Cannon (cx=0x902dfa8, fp=0xf77110b8) at js/src/ion/Ion.cpp:1875
#7  0x081c4164 in js::RunScript (cx=0x902dfa8, fp=0xf77110b8) at js/src/jsinterp.cpp:336
#8  0x081c4f08 in js::Invoke (cx=0x902dfa8, args=..., construct=js::NO_CONSTRUCT) at js/src/jsinterp.cpp:408
#9  0x081c55fd in js::Invoke (cx=0x902dfa8, thisv=..., fval=..., argc=0, argv=0xffffc24c, rval=0xffffc214) at js/src/jsinterp.cpp:441
#10 0x082d4869 in js::ion::DoCallFallback (cx=0x902dfa8, frame=0xffffc26c, stub=0x903fef8, argc=0, vp=0xffffc23c, res=$jsval(-nan(0xfff8200000000))) at js/src/ion/BaselineIC.cpp:6855
(gdb) info reg
eax            0xf7700000       -143654912
(gdb) x /i $pc
=> 0x8244c10 <js::str_split(JSContext*, unsigned int, JS::Value*)+5760>:        cmp    (%eax),%di


In addition, this assertion popped up:

Assertion failure: hasBase(), at vm/String.h:948
Crash Signature: [@ StringMatch]
Summary: IonMonkey: Assertion failure: str, at ./dist/include/js/Value.h:640 or Crash on Heap → IonMonkey: Assertion failure: str, at ./dist/include/js/Value.h:640 or Assertion failure: hasBase(), at vm/String.h:948 or Crash on Heap or Crash [@ StringMatch]
Group: core-security
Attachment #754490 - Attachment is obsolete: true
Attachment #754762 - Attachment is obsolete: true
Attached patch patchSplinter Review
Old Ion bug, GVN could flatten fallible and infallible unboxes of the same value together and effectively eliminate a type barrier.  Don't know why this wasn't cropping up before...
Assignee: general → bhackett1024
Attachment #754867 - Flags: review?(jdemooij)
Flags: needinfo?(bhackett1024)
Attachment #754867 - Flags: review?(jdemooij) → review+
Comment on attachment 754867 [details] [diff] [review]
patch

[Security approval request comment]
How easily could an exploit be constructed based on the patch?

Should be difficult, requires knowledge of compiler internals and it's not clear if this even can be triggered on older branches (fuzzers didn't find a testcase until a recent unrelated patch landed).

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?

Possibly all Ion branches

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

Trivial

How likely is this patch to cause regressions; how much testing does it need?

None
Attachment #754867 - Flags: sec-approval?
Comment on attachment 754867 [details] [diff] [review]
patch

sec-approval+ for m-c. Please nominate for branch after it is in.
Attachment #754867 - Flags: sec-approval? → sec-approval+
Whiteboard: [jsbugmon:update] → [jsbugmon:update,ignore]
JSBugMon: The testcase found in this bug no longer reproduces (tried revision 75407626ba46).
https://hg.mozilla.org/mozilla-central/rev/6a891dc6fee1
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
Status: RESOLVED → VERIFIED
JSBugMon: This bug has been automatically verified fixed.
Duplicate of this bug: 877207
Comment on attachment 754867 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Old Ion bug
User impact if declined: Potentially crashy/exploitable codegen, though it is unclear if this can actually occur on older branches
Testing completed (on m-c, etc.): on m-c
Risk to taking this patch (and alternatives if risky): none
Attachment #754867 - Flags: approval-mozilla-beta?
Attachment #754867 - Flags: approval-mozilla-aurora?
Comment on attachment 754867 [details] [diff] [review]
patch

no risk sec-high; approving
Attachment #754867 - Flags: approval-mozilla-beta?
Attachment #754867 - Flags: approval-mozilla-beta+
Attachment #754867 - Flags: approval-mozilla-aurora?
Attachment #754867 - Flags: approval-mozilla-aurora+
Whiteboard: [jsbugmon:update,ignore] → [jsbugmon:update,ignore][adv-main22+]
Marking status-firefox24:verified based on comment 13.
Group: core-security
You need to log in before you can comment on or make changes to this bug.