summaryrefslogtreecommitdiffstats
path: root/src/plugins/sqldrivers
diff options
context:
space:
mode:
authorGiuseppe D'Angelo <giuseppe.dangelo@kdab.com>2021-05-28 17:16:03 +0200
committerGiuseppe D'Angelo <giuseppe.dangelo@kdab.com>2021-05-31 23:12:46 +0200
commit0f38259cb3aee5cce5a2af99af3f69712c9f1123 (patch)
treeb88b4ba1d8e04f934957a9a837d036db18b81806 /src/plugins/sqldrivers
parent459529ace98636c1ab534d889ab178da03b4c309 (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 Pick-to: 6.1 5.15 5.12 Fixes: QTBUG-94070 Reviewed-by: Andy Shaw <andy.shaw@qt.io>
Diffstat (limited to 'src/plugins/sqldrivers')
-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 dc9728977a..91a1abaf98 100644
--- a/src/plugins/sqldrivers/sqlite/qsql_sqlite.cpp
+++ b/src/plugins/sqldrivers/sqlite/qsql_sqlite.cpp
@@ -499,7 +499,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);