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

VERIFIED FIXED

Status

()

--
major
VERIFIED FIXED
6 years ago
2 years ago

People

(Reporter: decoder, Assigned: dvander)

Tracking

(Blocks: 2 bugs, {crash, csectype-uaf, testcase})

Other Branch
x86
Linux
crash, csectype-uaf, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox17 unaffected, firefox18 affected, firefox-esr10 unaffected)

Details

(Whiteboard: [ion:p1:fx18] [jsbugmon:origRev=92b9b2840a79,bisect], crash signature)

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
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);
  }
}
(Reporter)

Comment 1

6 years ago
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.
(Reporter)

Updated

6 years ago
Whiteboard: [jsbugmon:update][ion:p1:fx18] → [ion:p1:fx18] [jsbugmon:update,ignore]
(Reporter)

Comment 3

6 years ago
JSBugMon: The testcase found in this bug no longer reproduces (tried revision d16c4404e8c4).
(Reporter)

Updated

6 years ago
Whiteboard: [ion:p1:fx18] [jsbugmon:update,ignore] → [ion:p1:fx18] [jsbugmon:bisectfix]
(Reporter)

Updated

6 years ago
Whiteboard: [ion:p1:fx18] [jsbugmon:bisectfix] → [ion:p1:fx18] [jsbugmon:]
(Reporter)

Comment 4

6 years ago
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.
(Reporter)

Comment 5

6 years ago
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.
Created attachment 659838 [details] [diff] [review]
fix

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
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Reporter)

Updated

6 years ago
Status: RESOLVED → VERIFIED
(Reporter)

Comment 11

6 years ago
JSBugMon: This bug has been automatically verified fixed.
status-firefox-esr10: --- → unaffected
status-firefox17: --- → unaffected
status-firefox18: --- → affected
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
Keywords: csectype-uaf
You need to log in before you can comment on or make changes to this bug.