ASSERTION fails in Array.prototype.copyWithin

[sunlili]

Hello, I run following code in ch 1.11.19(debug)，and it will crash by an assertion.

let b = [1.1, 2.2, 3.3];
b[4294967294] = 3;
Array.prototype.copyWithin.call(b, 0, 1);

Crash output:

ASSERTION 7690: (/.../ChakraCore-1.11.19/lib/Runtime/Library/JavascriptArray.cpp, line 9309) direction == -1 || (fromVal + count < MaxArrayLength && toVal + count < MaxArrayLength)
 Failure: (direction == -1 || (fromVal + count < MaxArrayLength && toVal + count < MaxArrayLength))
Illegal instruction

When reading the source code, I find that the if-condition and the asserts in else-branch are not mutually complemental. https://github.com/microsoft/ChakraCore/blob/c848d4d8d50c0dfb4a23540a9ee6cd023fa029c1/lib/Runtime/Library/JavascriptArray.cpp#L9286 The asserts in else-branch should be : Assert((fromVal + count) <= MaxArrayLength && (toVal + count) <= MaxArrayLength )

ISec Lab 2020.6.8

[ppenzin]

Thank you for the report, that is definitely a bug.

[sunlili]

@ppenzin Hello, I have submitted some bug issues, but they are not be confirmed after several months. Those bugs are very very important for me, since I am writing a paper which needs some experimental data. My deadline is at the end of January, before that time, I will submit some other bug issues too. So, could you help me to confirm those bugs as soon as possible, at least before the deadline? Thank you very much.

[ppenzin]

@sunlili we are a bit short-handed, but let me see what I can do. Briefly looking at the bugs, you have filed some as far as last year, do you need all of those as well?

In you paper, are you going to publish how you found the issues? In case you are working on automating bug discovery I think everybody here would be interested in learning more about it.

Feel free to ping us on discord (link in Readme).

[sunlili]

@ppenzin sorry for the late reply. I had seen the bug issues have been confirmed, thanks a lot. I developed a new fuzzer tool this year, which are running and finds some bugs. All details of the tool will be elaborated in the paper, I am writing it. If it is accepted, I will tell you. I commit some new bug issues today, could you looking at them? thank you again. My email is sunlili@ict.ac.cn, you can send me e-mails. I check emails more frequently:)

[ppenzin]

We are in looking to set up new CI for the project after it completes transition to community ownership, any fuzzing technology would be very helpful.

I noticed that @rhuanjl went though the bugs you filed, let us know if you need anything more in terms of verification.

[h9nirajpatil]

if ((direction == -1 && (fromVal >= MaxArrayLength || toVal >= MaxArrayLength)) || (((fromVal + count) > MaxArrayLength) || ((toVal + count) > MaxArrayLength))) { while (count > 0) { Var index = JavascriptNumber::ToVar(fromVal, scriptContext);

    JS_REENTRANT(jsReentLock, BOOL hasItem = JavascriptOperators::OP_HasItem(obj, index, scriptContext));
    if (hasItem)
    {
        Var val = nullptr;
        JS_REENTRANT(jsReentLock,
            val = JavascriptOperators::OP_GetElementI(obj, index, scriptContext),
            JavascriptOperators::OP_SetElementI(obj, JavascriptNumber::ToVar(toVal, scriptContext), val, scriptContext, PropertyOperation_ThrowIfNotExtensible));
    }
    else
    {
        JS_REENTRANT(jsReentLock, JavascriptOperators::OP_DeleteElementI(obj, JavascriptNumber::ToVar(toVal, scriptContext), scriptContext, PropertyOperation_ThrowOnDeleteIfNotConfig));
    }

    fromVal += direction;
    toVal += direction;
    count--;
}

} else { // Corrected assertion Assert((fromVal + count) <= MaxArrayLength && (toVal + count) <= MaxArrayLength);

// Normal optimized code path for indices within range
if (direction == -1)
{
    // Handle reverse copy within the valid range
    fromVal += count - 1;
    toVal += count - 1;
    while (count > 0)
    {
        Var index = JavascriptNumber::ToVar(fromVal, scriptContext);

        JS_REENTRANT(jsReentLock, BOOL hasItem = JavascriptOperators::OP_HasItem(obj, index, scriptContext));
        if (hasItem)
        {
            Var val = nullptr;
            JS_REENTRANT(jsReentLock,
                val = JavascriptOperators::OP_GetElementI(obj, index, scriptContext),
                JavascriptOperators::OP_SetElementI(obj, JavascriptNumber::ToVar(toVal, scriptContext), val, scriptContext, PropertyOperation_ThrowIfNotExtensible));
        }
        else
        {
            JS_REENTRANT(jsReentLock, JavascriptOperators::OP_DeleteElementI(obj, JavascriptNumber::ToVar(toVal, scriptContext), scriptContext, PropertyOperation_ThrowOnDeleteIfNotConfig));
        }

        fromVal--;
        toVal--;
        count--;
    }
}
else
{
    // Handle forward copy within the valid range
    while (count > 0)
    {
        Var index = JavascriptNumber::ToVar(fromVal, scriptContext);

        JS_REENTRANT(jsReentLock, BOOL hasItem = JavascriptOperators::OP_HasItem(obj, index, scriptContext));
        if (hasItem)
        {
            Var val = nullptr;
            JS_REENTRANT(jsReentLock,
                val = JavascriptOperators::OP_GetElementI(obj, index, scriptContext),
                JavascriptOperators::OP_SetElementI(obj, JavascriptNumber::ToVar(toVal, scriptContext), val, scriptContext, PropertyOperation_ThrowIfNotExtensible));
        }
        else
        {
            JS_REENTRANT(jsReentLock, JavascriptOperators::OP_DeleteElementI(obj, JavascriptNumber::ToVar(toVal, scriptContext), scriptContext, PropertyOperation_ThrowOnDeleteIfNotConfig));
        }

        fromVal++;
        toVal++;
        count--;
    }
}

}

[ppenzin]

@h9nirajpatil not sure what you meant, can you sent a PR if this is a change you are proposing?