583404 - TM: Unsafe loop in js_ClearNative

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[David Mandelin [:dmandelin]]

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.

Comment 1

[Robert Sayre]

igor, could you take a look at 

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

we think this is what hit bug 579957

Comment 2

[David Mandelin [:dmandelin]]

(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

Comment 3

[Igor Bukanov]

(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.

Comment 4

[Igor Bukanov]

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.

Comment 6

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/8d4eaefc5894