tc39 / ecma262

Status, process, and documents for ECMA-262
https://tc39.es/ecma262/
Other
15.05k stars 1.28k forks source link

`%TypedArray%.prototype.slice` does not completely account for the original TA's size changing #3248

Closed trflynn89 closed 10 months ago

trflynn89 commented 10 months ago

In %TypedArray%.prototype.slice, it is possible for the source TA's size to change, for example during the 13. Let A be ? TypedArraySpeciesCreate(O, « 𝔽(count) »). step. The spec partially handles this case by updating a few locals before later writing to the new TA:

14. If count > 0, then
    a. Set taRecord to MakeTypedArrayWithBufferWitnessRecord(O, seq-cst).
    b. If IsTypedArrayOutOfBounds(taRecord) is true, throw a TypeError exception.
    c. Set len to TypedArrayLength(taRecord).
    d. Set final to min(final, len).

However, these steps do not update the count value, which was previously initialized as:

12. Let count be max(final - k, 0).

This causes an issue later while reading values from the source TA:

viii. Let limit be targetByteIndex + min(count, len) × elementSize.
ix. Repeat, while targetByteIndex < limit,
    1. Let value be GetValueFromBuffer(srcBuffer, srcByteIndex, uint8, true, unordered).
    2. Perform SetValueInBuffer(targetBuffer, targetByteIndex, uint8, value, true, unordered).
    3. Set srcByteIndex to srcByteIndex + 1.
    4. Set targetByteIndex to targetByteIndex + 1.

In the call to GetValueFromBuffer, because count was not updated, it is possible to read from an index that is past-the-end of the srcBuffer.

This is observable with the following JS, which I reduced from the test/staging/ArrayBuffer/resizable/slice-species-create-resizes.js test262 test case:

const rab = new ArrayBuffer(4 * Uint8Array.BYTES_PER_ELEMENT, {
    maxByteLength: 8 * Uint8Array.BYTES_PER_ELEMENT,
});

let resizeWhenConstructorCalled = false;

class MyArray extends Uint8Array {
    constructor(...params) {
        super(...params);

        if (resizeWhenConstructorCalled) {
            rab.resize(2 * Uint8Array.BYTES_PER_ELEMENT);
        }
    }
}

const lengthTracking = new MyArray(rab);
resizeWhenConstructorCalled = true;

lengthTracking.slice(-3, -1);

In the call to slice, the size of the source TA is reduced from 4 to 2 during the 13. TypedArraySpeciesCreate step. Before that step, we have k=1, final=3, len=4, count=2. After step 14d, we have k=1, final=2, len=2. Because count is not updated, it is still count=2. This leads to reading one past-the-end of the source TA buffer.

michaelficarra commented 10 months ago

But as you say, len is updated, and limit (used for bounding the loop) is computed from the smaller of count and len, so I don't see how srcByteIndex would go past the end of the source TA.

trflynn89 commented 10 months ago

In the example, we read past the end of the source TA because the starting offset (k) is 1. Since len becomes 2, we only have 1 slot left to read from. So limit should be 1.

But because count wasn't updated, when we set limit, we have limit = min(count, len) = min(2, 2), thus we think we have 2 slots available to read from.

syg commented 10 months ago

Great catch, @trflynn89. Will have a PR up shortly.