summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorGiuseppe D'Angelo <giuseppe.dangelo@kdab.com>2021-05-28 17:16:03 +0200
committerQt Cherry-pick Bot <cherrypick_bot@qt-project.org>2021-06-01 10:57:32 +0000
commit07b690bcdb6b4607bb691939a27481b88d554033 (patch)
tree2a7dd563ed8eb70d7b8b5199e6b8b9415936368c
parentc35d18677420d8a2b31fcd976fb8084f44932129 (diff)
SQLite driver: fix crash when binding a QByteArray/QString
Passing SQLITE_STATIC to sqlite3_bind_*() means that ownership of the data stays in the caller, i.e. SQLite itself doesn't make a copy; such data must be therefore be kept valid until sqlite3_step() is called. The code in the SQLite driver uses that option to avoid copying byte array or string data. But, unlike what the comments in the code say, we do NOT keep the QByteArray/QString alive long enough: they're contained by a temporary QVariant object which gets destroyed at the end of the loop that binds each argument. Luckily the fix is simple: since that QVariant is just a copy of the QVariants used as bound parameters, and these are held in a container (which lives long enough), simply create a reference to the container's elements rather than a copy. This ensures that the data is alive by the time sqlite3_step() is called. This problem doesn't normally appear because of implicit sharing of QByteArray/QString. When the QVariant is copied, the inner element is just a shallow copy. Getting the pointer to the data, and destroying the QVariant, does not destroy the data (it's kept alive by the QByteArray/QString inside the *copied-from* QVariant). Of course there's a catch: if the *copied-from* QVariant contains a QString created via fromRawData, then everything blows up. In this case, 1. the copied QVariant is created (which bumps the QString refcount)¹ 2. the QString inside of it is accessed directly (via QVariant::constData) 3. utf16() is called on that string, which detaches it (!) 4. the result of utf16() is passed to SQLite, with SQLITE_STATIC 5. the copied QVariant is destroyed; this destroys the inner QString, which, being detached, deallocates the data too early. 6. sqlite3_step() is called, kaboom. (The copied-from QVariant still has the string created by fromRawData.) ¹ Note that QString uses the Small QVariant Optimization, so the QString object itself into the QVariant is copied, it's not just a *QVariant* refcount increase. Change-Id: Idcdb192809f1f8f79b4a901e1247f933eb06e854 Fixes: QTBUG-94070 Reviewed-by: Andy Shaw <andy.shaw@qt.io> (cherry picked from commit 0f38259cb3aee5cce5a2af99af3f69712c9f1123) Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
-rw-r--r--src/plugins/sqldrivers/sqlite/qsql_sqlite.cpp2
1 files changed, 1 insertions, 1 deletions
diff --git a/src/plugins/sqldrivers/sqlite/qsql_sqlite.cpp b/src/plugins/sqldrivers/sqlite/qsql_sqlite.cpp
index f1a003ddcd..a9b4bd4bca 100644
--- a/src/plugins/sqldrivers/sqlite/qsql_sqlite.cpp
+++ b/src/plugins/sqldrivers/sqlite/qsql_sqlite.cpp
@@ -500,7 +500,7 @@ bool QSQLiteResult::exec()
if (paramCountIsValid) {
for (int i = 0; i < paramCount; ++i) {
res = SQLITE_OK;
- const QVariant value = values.at(i);
+ const QVariant &value = values.at(i);
if (value.isNull()) {
res = sqlite3_bind_null(d->stmt, i + 1);