Closed Bug 583404 Opened 11 years ago Closed 11 years ago

TM: Unsafe loop in js_ClearNative

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: dmandelin, Assigned: igor)

References

Details

(Whiteboard: fixed-in-tracemonkey, [sg:critical], [critsmash:patch])

js_ClearNative has this code:

    JSScope *scope;
    uint32 i, n;

    /*
     * Clear our scope and the property cache of all obj's properties only if
     * obj owns the scope (i.e., not if obj is sharing another object's scope).
     * NB: we do not clear any reserved slots lying below JSSLOT_FREE(clasp).
     */
    JS_LOCK_OBJ(cx, obj);
    scope = obj->scope();
    if (!scope->isSharedEmpty()) {
        /* Now that we're done using scope->lastProp/table, clear scope. */
        scope->clear(cx);

        /* Clear slot values and reset freeslot so we're consistent. */
        i = obj->numSlots();
        n = JSSLOT_FREE(obj->getClass());
        while (--i >= n)
            obj->setSlot(i, UndefinedValue());
        scope->freeslot = n;

I believe that |n| can be 0 now, so using unsigned ints here can underflow. I fixed this in JM as bug 583402.
igor, could you take a look at 

http://hg.mozilla.org/projects/jaegermonkey/rev/fbbb0c6655c9

we think this is what hit bug 579957
Assignee: general → igor
(In reply to comment #1)
> igor, could you take a look at 
> 
> http://hg.mozilla.org/projects/jaegermonkey/rev/fbbb0c6655c9
> 
> we think this is what hit bug 579957

Please note there is a bug in my original fix. I used the wrong value for the new value of scope->freeslot. I pushed a followup here:

http://hg.mozilla.org/projects/jaegermonkey/rev/8a3800153866
(In reply to comment #1)
> igor, could you take a look at 
> 
> http://hg.mozilla.org/projects/jaegermonkey/rev/fbbb0c6655c9
> 
> we think this is what hit bug 579957

Right, this indeed the case.
http://hg.mozilla.org/tracemonkey/rev/8d4eaefc5894 - landed to TM.

This is the JM fix with a nit fixed to use the original uint32, not int, for indexes as both JSCLASS_FREE(clasp) and obj->numSlots() returns uint32.
Blocks: 579957
Whiteboard: fixed-in-tracemonkey, [sg:critical]
Whiteboard: fixed-in-tracemonkey, [sg:critical] → fixed-in-tracemonkey, [sg:critical], [critsmash:patch]
Duplicate of this bug: 583421
http://hg.mozilla.org/mozilla-central/rev/8d4eaefc5894
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Group: core-security
You need to log in before you can comment on or make changes to this bug.