Closed Bug 786130 Opened 8 years ago Closed 8 years ago

IonMonkey: Crash [@ js::SkipSpace] with use-after-free

Categories

(Core :: JavaScript Engine, defect)

Other Branch
x86
Linux
defect
Not set
major

Tracking

()

VERIFIED FIXED
Tracking Status
firefox17 --- unaffected
firefox18 --- affected
firefox-esr10 --- unaffected

People

(Reporter: decoder, Assigned: dvander)

References

Details

(Keywords: crash, csectype-uaf, testcase, Whiteboard: [ion:p1:fx18] [jsbugmon:origRev=92b9b2840a79,bisect])

Crash Data

Attachments

(1 file)

The following testcase crashes on ionmonkey revision 92b9b2840a79 (run with --ion -n -m --ion-eager -a):


var lfcode = new Array();
lfcode.push("\
gczeal(2);\
try {\
  xxxyyyzzz();\
} catch (e) {\
  var match = e.toString().search(/ReferenceError/);\
}\
");
lfcode.push("\
StrictEquality(true, new Boolean(true), false, 0);\
function StrictEquality(x, y, expect, i) {\
    result = ( /\\W+/   ^ expect+\".getUTCMinutes()\");\
}\
");
while (true) {
  var file = lfcode.shift(); if (file == undefined) { break; }
  loadFile(file);
}
function loadFile(lfVarx) {
  if (lfVarx.substr(-3) == ".js") {
  } else {
	evaluate(lfVarx);
  }
}
Crash trace:


==10866== Invalid read of size 2
==10866==    at 0x818ECB2: js::SkipSpace(unsigned short const*, unsigned short const*) (jsstrinlines.h:100)
==10866==    by 0x819419B: bool js::StringToNumberType<double>(JSContext*, JSString*, double*) (jsnuminlines.h:54)
==10866==    by 0x8192A76: ToNumberSlow (jsnum.cpp:1375)
==10866==    by 0x8192D8E: ToInt32Slow (jsnum.cpp:1448)
==10866==    by 0x806BE6C: ToInt32 (jsapi.h:2866)
==10866==    by 0x858BF6B: js::BitXor(JSContext*, JS::Value const&, JS::Value const&, int*) (jsinterpinlines.h:895)
==10866==    by 0x78CD6AA: ???
==10866==  Address 0xdadadada is not stack'd, malloc'd or (recently) free'd
Whiteboard: [jsbugmon:update] → [jsbugmon:update][ion:p1:fx18]
Can't reproduce locally on x86 debug or x86_64 debug.
Whiteboard: [jsbugmon:update][ion:p1:fx18] → [ion:p1:fx18] [jsbugmon:update,ignore]
JSBugMon: The testcase found in this bug no longer reproduces (tried revision d16c4404e8c4).
Whiteboard: [ion:p1:fx18] [jsbugmon:update,ignore] → [ion:p1:fx18] [jsbugmon:bisectfix]
Whiteboard: [ion:p1:fx18] [jsbugmon:bisectfix] → [ion:p1:fx18] [jsbugmon:]
JSBugMon: Fix Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first good revision is:
changeset:   105607:6cd206b37176
parent:      104959:b63bb39ed1c0
parent:      105606:a0240c1043ee
user:        David Anderson
date:        Wed Aug 29 17:51:24 2012 -0700
summary:     Merge from mozilla-central.

Oops! We didn't test rev b63bb39ed1c0, a parent of the blamed revision! Let's do that now.
Rev b63bb39ed1c0: Updating... Compiling... Testing... gdb -n -batch -x /home/decoder/LangFuzz/fuzzing/util/gdb-quick.txt /home/decoder/Desktop/autobisect-cache/js-dbg-32-b63bb39ed1c0-linux core.5401
Exit status: CRASHED signal 11 (SIGSEGV) (0.032 seconds)
bad (interesting) 
As expected, the parent's label is the opposite of the blamed rev's label.

Oops! We didn't test rev a0240c1043ee, a parent of the blamed revision! Let's do that now.
We did not test rev a0240c1043ee because it is not a descendant of either 92b9b2840a79 or d16c4404e8c4.
Rev a0240c1043ee: Updating... Compiling... Testing... Exit status: ABNORMAL return code 1 (0.012 seconds)
good (not interesting) 
Bisect lied to us! Parent rev a0240c1043ee was also good!

Perhaps we should expand the search to include the common ancestor of the blamed changeset's parents.
The common ancestor of b63bb39ed1c0 and a0240c1043ee is 88e47f6905e9.
Rev 88e47f6905e9: Updating... Compiling... Testing... Exit status: ABNORMAL return code 1 (0.012 seconds)
good (not interesting) 
Try setting -s to 88e47f6905e9, and -e to b63bb39ed1c0, and re-run autoBisect.
Gary: what autoBisect is telling here in the last line seems to be misleading. When I use "-s 88e47f6905e9 -e b63bb39ed1c0" I'm getting the first *bad* changeset, although we were bisecting for the fix before that.
(In reply to Christian Holler (:decoder) from comment #5)
> Gary: what autoBisect is telling here in the last line seems to be
> misleading. When I use "-s 88e47f6905e9 -e b63bb39ed1c0" I'm getting the
> first *bad* changeset, although we were bisecting for the fix before that.

Yes, you're right. It had never worked properly - I've disabled this print statement. Will have to think of some other way to do what I wanted it to do. :-/
Assignee: general → dvander
Status: NEW → ASSIGNED
I can reproduce this against the given cset, but not tip. It looks like a rooting bug. It's time to use HandleValue everywhere, since IonMonkey does not use the conservative scanner for its stack range.
Attached patch fixSplinter Review
Applying the BitXor part of this patch to the problematic cset, does in fact fix this bug.
Attachment #659838 - Flags: review?(sstangl)
Comment on attachment 659838 [details] [diff] [review]
fix

Review of attachment 659838 [details] [diff] [review]:
-----------------------------------------------------------------

Oof, good catch.
Attachment #659838 - Flags: review?(sstangl) → review+
https://hg.mozilla.org/projects/ionmonkey/rev/01f6ddbb6542
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
JSBugMon: This bug has been automatically verified fixed.
We made more improvements to autoBisectJs. Hope this shows up a better bisection result.
Whiteboard: [ion:p1:fx18] [jsbugmon:] → [ion:p1:fx18] [jsbugmon:origRev=92b9b2840a79,bisect]
(In reply to Gary Kwong [:gkw, :nth10sd] from comment #12)
> We made more improvements to autoBisectJs. Hope this shows up a better
> bisection result.

Worth noting: with bugs like this there's not going to be a useful regressing cset, so even if auto-bisect finds something, it won't really be related.
Group: core-security
You need to log in before you can comment on or make changes to this bug.