Closed Bug 636487 Opened 9 years ago Closed 5 years ago

Crash [@ JSString::isRope] after replacing Error.prototype.toString

Categories

(Core :: JavaScript Engine, defect, critical)

defect
Not set
critical

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- .x+

People

(Reporter: jandem, Unassigned)

References

Details

(Keywords: crash, regression, testcase, Whiteboard: [sg:dos][js-triage-done])

Crash Data

Attachments

(1 file)

--
Error.prototype.__defineGetter__("toString", function(){}); 
throw Error.prototype;
--
This crashes debug/release shell/browser.

I think this is a NULL-pointer dereference and not exploitable, but I haven't looked closely so marking s-s for now.
Attached file Stack trace
Group: core-security
Nominating for .x

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   57437:95e9c2e8708d
user:        David Mandelin
date:        Fri Nov 12 18:17:21 2010 -0800
summary:     Bug 605752: don't crash on OOM inside ExecutablePool, r=dvander, a=beta8+
Blocks: 605752
blocking2.0: --- → ?
Keywords: regression
Version: unspecified → Trunk
blocking2.0: ? → .x+
Whiteboard: js-triage-needed
Severity: normal → critical
In js_ReportUncaughtException, we call |ToString| on |exn| but don't return false if it returns |NULL|.  Admittedly these are exception-reporting guts, so perhaps that's desirable for some obscure reason.  But based on the wealth of other return-falses I see here, it seems to me that that's what should happen here too.

    str = ToString(cx, exn);
    JSAutoByteString bytesStorage;
    if (!str) {
        bytes = "unknown (can't convert to string)";
    } else {
        roots[1] = StringValue(str);
        if (!bytesStorage.encode(cx, str))
            return false;
        bytes = bytesStorage.ptr();
    }

    JSAutoByteString filename;
    if (!reportp && exnObject && exnObject->isError()) {
        if (!JS_GetProperty(cx, exnObject, js_message_str, &roots[2]))
            return false;
        if (JSVAL_IS_STRING(roots[2])) {
            bytesStorage.clear();
            if (!bytesStorage.encode(cx, str))
                return false;
Whiteboard: js-triage-needed → [js-triage-done][good first bug][mentor=Waldo][lang=c++]
Crash Signature: [@ JSString::isRope]
Summary: Crash after replacing Error.prototype.toString → Crash [@ JSString::isRope] after replacing Error.prototype.toString
Whiteboard: [js-triage-done][good first bug][mentor=Waldo][lang=c++] → [sg:dos][js-triage-done][good first bug][mentor=Waldo][lang=c++]
Couldn't reproduce this against the tip of the default branch @ 95628:9274e6b53af4.

Can somebody confirm this is fixed?
The first good revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/725a2ff9d08c
user:        Masatoshi Kimura
date:        Tue Apr 03 20:08:28 2012 -0400
summary:     Bug 739659 - Try duck typing in js_ReportUncaughtException. r=luke
Whiteboard: [sg:dos][js-triage-done][good first bug][mentor=Waldo][lang=c++] → [sg:dos][js-triage-done]
Assignee: general → nobody
WFM as of m-c rev e537a1ba501b, assuming fixed by bug 739659 as per comment 5.
Status: NEW → RESOLVED
Closed: 5 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.