summaryrefslogtreecommitdiffstats
path: root/src/plugins
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 00:10:42 +0000
commit47a1e10d63d9c11fe600c703240031b2d02eb3a6 (patch)
tree3598298bebd0f34859c9a8e16e0efe1f86e03c2c /src/plugins
parent122455d37e94e6703d958fb8eac665c4f89c717a (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>
Diffstat (limited to 'src/plugins')
-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);