Crash [@ JSC::Yarr::execute] or [@ js::RegExpShared::execute]

VERIFIED FIXED in Firefox 15

Status

()

defect
--
critical
VERIFIED FIXED
7 years ago
7 years ago

People

(Reporter: gkw, Assigned: luke)

Tracking

(Blocks 1 bug, 4 keywords)

Trunk
mozilla17
x86
Linux
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox14 wontfix, firefox15 fixed, firefox16 fixed, firefox17 fixed, firefox-esr10 unaffected)

Details

(Whiteboard: [advisory-tracking+][qa?], crash signature)

Attachments

(2 attachments)

Reporter

Description

7 years ago
Posted file stacks
r = evalcx("/x/", undefined);
s = "";
gc()
Function("\
    s.match(r);\
    schedulegc(__proto__);\
    ({c:schedulegc(2)});\
    s.match(r);\
")()

crashes js debug shell on m-c changeset 6c60e99d9739 without any CLI arguments at JSC::Yarr::execute with js::RegExpShared::execute on the stack.
Reporter

Comment 1

7 years ago
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   91130:15a23c3923ff
user:        Bill McCloskey
date:        Tue Apr 03 14:07:38 2012 -0700
summary:     Bug 742570 - Improve GC testing functions (r=igor)

Not sure if this only affects gc testing functions, so setting Firefox 14 and later as affected.
Assignee: general → wmccloskey
Here's what seems to be happening. We have a cross-compartment wrapper to a regexp. We call some regexp method on it, and that calls RegExpToShared on the CCW. That calls through some proxy code, which basically unwraps the regexp and calls RegExpToShared on the unwrapped regexp. We don't have a RegExpShared for this regexp yet, so we call RegExpObject::createShared, which looks like this:

bool
RegExpObject::createShared(JSContext *cx, RegExpGuard *g)
{
    Rooted<RegExpObject*> self(cx, this);

    JS_ASSERT(!maybeShared());
    if (!cx->compartment->regExps.get(cx, getSource(), getFlags(), g))
        return false;

    self->setShared(cx, **g);
    return true;
}

The crucial thing here is that while self is the unwrapped regexp, cx->compartment is the compartment that the CCW was in. So they don't match. Consequently, we're putting the RegExpShared in the wrong compartment's table. Later on, we collect that compartment and kill the RegExpShared, but the |self| object from above still has a pointer to it because it was in a different compartment and not traced.

An easy fix would be to change cx->compartment to self->compartment(). However, it seems like this is a more general problem. I see a number of IndirectProxyHandler methods that unwrap a CCW and do some work on the unwrapped object without ever switching compartments. This seems pretty dangerous to me. Is it by design? Can we change it so that we enter the right compartment? We'd have to wrap the result, but that doesn't seem to be an issue in these cases as far as I can tell.
Assignee

Comment 3

7 years ago
Sorry you had to look into this.  It seems to me like the fix would be for CrossCompartmentWrapper to override regexp_toShared, wrapping the call to IndirectProxyHandler::regexp_toShared with an AutoCompartment, as with most of the rest of the ProxyHandler hooks.
Assignee

Comment 4

7 years ago
Posted patch fix and testSplinter Review
Attachment #649678 - Flags: review?(wmccloskey)
Comment on attachment 649678 [details] [diff] [review]
fix and test

Thanks. I hadn't realized that CrossCompartmentWrapper overrode all these.
Attachment #649678 - Flags: review?(wmccloskey) → review+
Assignee

Comment 7

7 years ago
Comment on attachment 649678 [details] [diff] [review]
fix and test

[Approval Request Comment]
User impact if declined: memory corruption
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): low
Attachment #649678 - Flags: approval-mozilla-beta?
Attachment #649678 - Flags: approval-mozilla-aurora?
Reporter

Comment 8

7 years ago
This will probably be WONTFIX for Firefox 14.
Assignee: wmccloskey → luke
https://hg.mozilla.org/mozilla-central/rev/a2398a7be210
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
Crash Signature: [@ JSC::Yarr::execute] [@ js::RegExpShared::execute] → [@ JSC::Yarr::execute] [@ js::RegExpShared::execute]
JSBugMon: This bug has been automatically verified fixed.
Attachment #649678 - Flags: approval-mozilla-beta?
Attachment #649678 - Flags: approval-mozilla-beta+
Attachment #649678 - Flags: approval-mozilla-aurora?
Attachment #649678 - Flags: approval-mozilla-aurora+
Assignee

Comment 11

7 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/6372bb9fb95d
https://hg.mozilla.org/releases/mozilla-beta/rev/ac9af7f6b0d1
Crash Signature: [@ JSC::Yarr::execute] [@ js::RegExpShared::execute] → [@ JSC::Yarr::execute] [@ js::RegExpShared::execute]
Whiteboard: [advisory-tracking+]
Does this need verification given comment 10?
Keywords: verifyme
Whiteboard: [advisory-tracking+] → [advisory-tracking+][qa?]
Group: core-security
A testcase for this bug was automatically identified at js/src/jit-test/tests/basic/testBug780712.js.
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.