crash in js::ExecuteRegExp(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSString*>, js::MatchConduit&, js::RegExpStaticsUpdate)

RESOLVED FIXED in mozilla33

Status

()

Core
JavaScript Engine
P5
critical
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: kbrosnan, Unassigned)

Tracking

({crash, sec-low})

unspecified
mozilla33
All
Android
crash, sec-low
Points:
---

Firefox Tracking Flags

(firefox25 affected, firefox26 affected, firefox27 affected, firefox28- affected, fennec+)

Details

(Whiteboard: [native-crash], crash signature)

(Reporter)

Description

5 years ago
This bug was filed from the Socorro interface and is 
report bp-c33f092d-94c6-4fc3-b503-c01c32131025.
=============================================================

Device list does not seem interesting nor URLs.

Filing as security sensitive as a small number of the crashes are marked Low Exploitability, Medium Exploitability and High Exploitability.
(Reporter)

Updated

5 years ago
Whiteboard: [native-crash]
The code where it crashed has blame on sstangl, but of course the bug could have happened earlier and just got detected at that point.
tracking-fennec: ? → +
Flags: needinfo?(nihsanullah)
Keywords: sec-want
(Reporter)

Comment 2

5 years ago
Sec-want is for new features. This needs a rating.
Keywords: sec-want
You're right, I was looking for sec-review-needed, but apparently we're using sec-review? for that now??
Flags: sec-review?
the resports are mostly null derefs which wouldn't be exploitable. Which ones are you seeing with a high exploitability rating?

Without a testcase i don't foresee a brilliant future for this bug. Given the architecture prevalence of the reports, however, it does look like we could use some focused RegExp-on-ARM fuzzing. I'm assuming that the raw address at the top of the stacks means we're JIT-ing regexps on ARM as we do on x86 and that code may not be as robust/tested. (If we're not JIT-ing on ARM then these stacks show we've gone off the rails and it's a more serious bug than the null deref would indicate.)
Flags: sec-review? → sec-review?(choller)
I've been fuzzing specifically RegExp-only on armv7 since yesterday, but haven't hit any crashes so far.
No luck here. Maybe Gary can find something with jsfunfuzz, but as far as I know, he is already running it on the right hardware and hasn't seen this signature.

Maybe Sean can also comment here and provide us some useful hints, esp. regarding comment 4?
Flags: sec-review?(gary)
Flags: sec-review?(choller)
Flags: needinfo?(sstangl)
(In reply to Daniel Veditz [:dveditz] from comment #4)
> I'm assuming that the raw
> address at the top of the stacks means we're JIT-ing regexps on ARM as we do
> on x86 and that code may not be as robust/tested. (If we're not JIT-ing on
> ARM then these stacks show we've gone off the rails and it's a more serious
> bug than the null deref would indicate.)

The YARR JIT does work on ARM, and the stacks indicate that the crashes are within the regexp jitcode. We've seen NULL derefs occasionally on x86/x86_64.
Flags: needinfo?(sstangl)
Christian, what kind of fuzzing were you doing in comments 6-7? Generating random regexps from the grammar and running them against random strings?

ISTM fuzzing YARR well is actually a little difficult, as against random strings you're unlikely to have complicated matches, and the non-matching and trivially-matching cases are not very interesting.
I used Jesse's old RegExp fuzzer (now part of jsfunfuzz) and I think it's more sophisticated than just generating both things randomly. I also have a mutation module in LangFuzz for RegExp that mutates existing pairs of RegExp+Strings but that whole setup is hard to use on ARM.
There have been some fixes to YARR on Apple's side in the meantime, in particular, https://bugs.webkit.org/show_bug.cgi?id=123421. It doesn't look like any of these fixes are relevant to us, but importing them can't hurt, if at least to stay in sync with upstream.
I'm back fuzzing on ARM and no, while I haven't yet specifically found anything bad on ARM for regex, I also don't think I'm a good source of information for giving this a sec-review+, so clearing it for now.
Flags: sec-review?(gary)
Keywords: sec-low
Sec-low and not a topcrasher, not a release blocking bug.
tracking-firefox28: ? → -
Group: javascript-core-security
Our long term solution for this class of bugs is to move FF to use the Irregexp, the v8 regex engine. That should land soon: https://bugzilla.mozilla.org/show_bug.cgi?id=976446.
Flags: needinfo?(nihsanullah)
Group: javascript-core-security
filter on [mass-p5]
Priority: -- → P5
Kevin, could you check if these crashes still exist?  We changed regexp engines, so probably they have gone away and this can be closed.  Thanks.
Flags: needinfo?(kbrosnan)
(Reporter)

Comment 17

4 years ago
Yes I don't see this on any 33 builds which is where bug 976446 was fixed.
Group: core-security
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Flags: needinfo?(kbrosnan)
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
(Reporter)

Updated

4 years ago
Duplicate of this bug: 1001607
You need to log in before you can comment on or make changes to this bug.