In QV4, we can update the length of an ArrayObject without actually changing the amount of memory allocated for the items in the array. This is reasonable, because otherwise array.length = 2000000 eats up a lot of memory for lots of undefined items.
Unfortunately, Array.concat does not appear to verify that all the items that it copies are actually allocated — meaning that setting the length of an array in this manner and then using Array.concat allows an out-of-bounds read:
(I found this bug while fuzzing with a modified version of Fuzzilli, in case anyone wants to go looking deeper into these types of bugs, or test against other versions; I limited myself to 5.13.2 because that was the current release on most systems when I went looking for this)
I think that this happens because we trust getLength in qv4arrayobject.cpp:417, and do not check for this scenario in ArrayData::append (qv4arraydata.cpp:589). The length is forwarded through Object::arrayPut (qv4object.cpp:257) to SimpleArrayData::putArray (qv4arraydata.cpp:321), where it simply copies n items from the values pointer that is passed in. Of course, at this point we no longer know how much space was allocated. Maybe the best way to fix this would be to check that no more than os->values.alloc items are passed into arrayPut?
This appears to be related to
QTBUG-51581 (which addressed similar symptoms but does not appear to fix this issue).
|For Gerrit Dashboard: QTBUG-81037|
|285507,4||Avoid oob access on Array.concat||5.14||qt/qtdeclarative||Status: MERGED||+2||0|
|361439,4||JS: Ensure that array keeps valid after length changes and fix concat||dev||qt/qtdeclarative||Status: MERGED||+2||0|
|362056,2||JS: Ensure that array keeps valid after length changes and fix concat||6.2||qt/qtdeclarative||Status: MERGED||+2||0|