Details
P1: Critical Description
Calling delete on a JS Set object can cause asan builds of Qt to trip with:
==89802==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60c000a092c0 at pc 0x000102e76788 bp 0x00016f627a80 sp 0x00016f627230 READ of size 128 at 0x60c000a092c0 thread T0 ==89802==WARNING: invalid path to external symbolizer! ==89802==WARNING: Failed to use and restart external symbolizer! #0 0x102e76784 in __asan_memmove+0xe40 (/<path_to_llvm_build_sources>/llvm-build/Release+Asserts/lib/clang/18/lib/darwin/libclang_rt.asan_osx_dynamic.dylib:arm64+0x4e784) #1 0x1044a77b0 in QV4::ESTable::remove(QV4::Value const&)+0x90 (/<path_to_qt_sources>/QtQml.framework/Versions/A/QtQml:arm64+0x10b7b0) #2 0x10453f4b0 in QV4::SetPrototype::method_delete(QV4::FunctionObject const*, QV4::Value const*, QV4::Value const*, int)+0x114 (/<path_to_qt_sources>/QtQml.framework/Versions/A/QtQml:arm64+0x1a34b0) #3 0x10456c94c in QV4::Moth::VME::interpret(QV4::JSTypesStackFrame*, QV4::ExecutionEngine*, char const*)+0x141c (/<path_to_qt_sources>/QtQml.framework/Versions/A/QtQml:arm64+0x1d094c) #4 0x10456b508 in QV4::Moth::VME::exec(QV4::JSTypesStackFrame*, QV4::ExecutionEngine*)+0x12c (/<path_to_qt_sources>/QtQml.framework/Versions/A/QtQml:arm64+0x1cf508) #5 0x1044b3a00 in QV4::doCall(QV4::Function*, QV4::Value const*, QV4::Value const*, int, QV4::ExecutionContext*)+0xc4 (/<path_to_qt_sources>/QtQml.framework/Versions/A/QtQml:arm64+0x117a00) #6 0x1044b319c in QV4::Function::call(QObject*, void**, QMetaType const*, int, QV4::ExecutionContext*)+0x208 (/<path_to_qt_sources>/QtQml.framework/Versions/A/QtQml:arm64+0x11719c) #7 0x104628cec in QQmlJavaScriptExpression::evaluate(void**, QMetaType const*, int)+0x80 (/<path_to_qt_sources>/QtQml.framework/Versions/A/QtQml:arm64+0x28ccec) #8 0x1045ce43c in QQmlBoundSignalExpression::evaluate(void**)+0x21c (/<path_to_qt_sources>/QtQml.framework/Versions/A/QtQml:arm64+0x23243c) #9 0x1045ce950 in QQmlBoundSignal_callback(QQmlNotifierEndpoint*, void**)+0x48 (/<path_to_qt_sources>/QtQml.framework/Versions/A/QtQml:arm64+0x232950) #10 0x104652b9c in QQmlNotifier::emitNotify(QQmlNotifierEndpoint*, void**)+0x200 (/<path_to_qt_sources>/QtQml.framework/Versions/A/QtQml:arm64+0x2b6b9c) #11 0x104cbe644 in void doActivate<false>(QObject*, int, void**)+0x9c (/<path_to_qt_sources>/QtCore.framework/Versions/A/QtCore:arm64+0x10e644) #12 0x10465c894 in QQmlObjectCreator::finalize(QQmlInstantiationInterrupt&)+0x654 (/<path_to_qt_sources>/QtQml.framework/Versions/A/QtQml:arm64+0x2c0894) #13 0x1045df0b4 in QQmlComponentPrivate::complete(QQmlEnginePrivate*, QQmlComponentPrivate::ConstructionState*)+0x40 (/<path_to_qt_sources>/QtQml.framework/Versions/A/QtQml:arm64+0x2430b4) #14 0x1045dcb30 in QQmlComponentPrivate::completeCreate()+0x134 (/<path_to_qt_sources>/QtQml.framework/Versions/A/QtQml:arm64+0x240b30) #15 0x1045dee08 in QQmlComponentPrivate::createWithProperties(QObject*, QMap<QString, QVariant> const&, QQmlContext*, QQmlComponentPrivate::CreateBehavior)+0x104 (/<path_to_qt_sources>/QtQml.framework/Versions/A/QtQml:arm64+0x242e08)
This is due to some off by one errors in ESTable::remove when memmove is called on the key and value arrays.
memmove(m_keys + idx, m_keys + idx + 1, (m_size - idx)*sizeof(Value)); memmove(m_values + idx, m_values + idx + 1, (m_size - idx)*sizeof(Value));
The above is calling memmove to shift all elements in m_keys and m_values to the right of idx to the left by one. However the count parameter one too large. Imagine we want to remove the element in a set with an idx of 0, that means we are moving `(m_size - 0) * sizeof(Value)` bytes, but the src paramater is not at the start of other array. So memmove will overrun the src buffer by an extra sizeof(Value).
This could cause a crash if these m_keys and m_values arrays are exactly aligned on the boundary of a memory page.
In practice you would rarely run into this issue unless the arrays were exactly the right size as m_capacity starts at 8 and is then doubled when needed so only when removing elements from a full ESTable would you run into problems.
Attachments
| For Gerrit Dashboard: QTBUG-123999 | ||||||
|---|---|---|---|---|---|---|
| # | Subject | Branch | Project | Status | CR | V |
| 552708,1 | Fix heap-buffer-overflow in ESTable::remove | dev | qt/qtdeclarative | Status: ABANDONED | 0 | 0 |
| 552709,5 | Fix heap-buffer-overflow in ESTable::remove | dev | qt/qtdeclarative | Status: MERGED | +2 | 0 |
| 552718,1 | Fix heap-buffer-overflow in ESTable::remove | dev | qt/qtdeclarative | Status: ABANDONED | 0 | 0 |
| 553096,2 | Fix heap-buffer-overflow in ESTable::remove | 6.7 | qt/qtdeclarative | Status: MERGED | +2 | 0 |
| 553153,1 | Fix heap-buffer-overflow in ESTable::remove | 6.6 | qt/qtdeclarative | Status: ABANDONED | +2 | 0 |
| 553678,2 | Fix heap-buffer-overflow in ESTable::remove | tqtc/lts-6.5 | qt/tqtc-qtdeclarative | Status: MERGED | +2 | +1 |
| 554191,3 | Fix heap-buffer-overflow in ESTable::remove | tqtc/lts-5.15 | qt/tqtc-qtdeclarative | Status: MERGED | +2 | 0 |
| 554192,4 | Fix heap-buffer-overflow in ESTable::remove | tqtc/lts-6.2 | qt/tqtc-qtdeclarative | Status: MERGED | +2 | 0 |
| 561785,2 | tst_qv4estable: fix -Wsign-compare | dev | qt/qtdeclarative | Status: MERGED | +2 | 0 |
| 562085,2 | tst_qv4estable: fix -Wsign-compare | 6.7 | qt/qtdeclarative | Status: MERGED | +2 | 0 |
| 562111,2 | tst_qv4estable: fix -Wsign-compare | tqtc/lts-6.5 | qt/tqtc-qtdeclarative | Status: MERGED | +2 | 0 |
| 562112,1 | tst_qv4estable: fix -Wsign-compare | tqtc/lts-6.2 | qt/tqtc-qtdeclarative | Status: ABANDONED | 0 | 0 |