From 4b736cb7ad0ae356f62f4b4f886c5ab25dbb4ea7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Morten=20S=C3=B8rvig?= Date: Fri, 11 Nov 2022 12:41:06 +0100 Subject: [PATCH] wasm: avoid LocalStorage past-the-end access on clear() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We were looping over all keys and removing the Qt keys using an integer index. However, removing one key shifts all other keys at higher indexes down one step such that the loop eventually indexes past the end. Fix this by getting the keys first in a separate step, and then remove keys without relying on a stable iteration order (which is also not guaranteed by the standard). Change-Id: I8bc577d1831d6931ebca2b2e04faf65c9affb429 Reviewed-by: Qt CI Bot Reviewed-by: Morten Johan Sørvig (cherry picked from commit 8a20a278fa5f1e2890f88a69d8b5f96873a67ac9) Reviewed-by: Qt Cherry-pick Bot --- src/corelib/io/qsettings_wasm.cpp | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/corelib/io/qsettings_wasm.cpp b/src/corelib/io/qsettings_wasm.cpp index 8404a526b65..15ab688abe5 100644 --- a/src/corelib/io/qsettings_wasm.cpp +++ b/src/corelib/io/qsettings_wasm.cpp @@ -122,13 +122,21 @@ QStringList QWasmLocalStorageSettingsPrivate::children(const QString &prefix, Ch void QWasmLocalStorageSettingsPrivate::clear() { - // Remove all Qt keys from window.localStorage + // Get all Qt keys from window.localStorage const int length = m_localStorage["length"].as(); + std::vector keys; + keys.reserve(length); for (int i = 0; i < length; ++i) { - std::string fullKey = (m_localStorage.call("key", i).as()); - QString key = QString::fromStdString(fullKey); - if (removeStoragePrefix(QStringView(key)).isEmpty() == false) - m_localStorage.call("removeItem", fullKey); + std::string key = (m_localStorage.call("key", i).as()); + keys.push_back(std::move(key)); + } + + // Remove all Qt keys. Note that localStorage does not guarantee a stable + // iteration order when the storage is mutated, which is why removal is done + // in a second step after getting all keys. + for (std::string key: keys) { + if (removeStoragePrefix(QString::fromStdString(key)).isEmpty() == false) + m_localStorage.call("removeItem", key); } }