Since #1046, QETSql::execReadOnly() runs a query with PRAGMA query_only
set and switches it off before returning. Switching it off aborts a
statement SQLite is still stepping through ("abort due to ROLLBACK"), and
QSQLITE has already stepped to the first row by then. A query that
produces its rows as it goes -- a UNION ALL without ORDER BY -- therefore
came back with its first row only and no error. A sorted query was not
affected, because SQLite has read every row before returning the first.
Every query from the SQL box of a table, a saved <graphics_table> query
and the scripting qet.query() goes through here.
The checked run is now finished before query_only is switched off, and a
query that passed is run again for the caller. SQLite refuses a write at
its first step, so passing that step is what proves a statement reads
only; the second run is of a statement already shown to be read-only.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G2d2Zi8BfrYRPX88zhaoFG
Since #983, projectDataBase::newQuery() checked a query with
sqlite3_prepare_v2() and sqlite3_stmt_readonly() on the handle of the
QSQLITE driver. Those calls go to the libsqlite3 QElectroTech links. The
QSQLITE plugin of the Qt online installer does not use that library: it
carries its own copy of SQLite, so the handle belongs to another library
and the call crashes. #1021 then put newQuery() on every element
selection, which is where #1045 hits it.
The check now runs the query with PRAGMA query_only set, through the
driver. SQLite refuses a write itself, before touching a row, so the CTE
prefix #983 closed ("WITH x AS (SELECT 1) DELETE FROM element") stays
closed. A refused or failed query comes back empty, because several
callers call exec() again on what newQuery() returns, after query_only
is off.
QElectroTech no longer calls the SQLite C API anywhere.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
projectDataBase::isReadOnlySelect() decides whether a query only reads
by looking at its first keyword and rejecting internal semicolons.
SQLite has allowed a CTE prefix in front of a data-modifying statement
since 3.8.3, so
WITH x AS (SELECT 1) DELETE FROM element
begins with WITH, contains no semicolon, passes the check, and deletes
every row. UPDATE and INSERT go through the same way.
This is not only reachable from the custom-query box. ProjectDBModel::
fromXml() reads a <graphics_table>'s saved <query> straight out of the
.qet and fillValue() executes it, so a project file can carry the
statement. Reproduced against a build of this branch's parent, with no
scripting and no CLI flag beyond the export itself: a project whose
stored table query was replaced with the DELETE above exported a bill
of materials of 0 rows instead of 14, exit code 0, nothing logged. A
silently empty or -- with UPDATE -- silently altered BOM is the kind of
output someone orders parts from.
Fixed by asking SQLite about the statement it actually compiled.
sqlite3_prepare_v2() compiles without running, sqlite3_stmt_readonly()
reports on the compiled statement rather than on how it was spelled,
and the prepare tail catches a second statement structurally. The same
project now exports its 14 rows again and logs a reason for the
refusal, while an ordinary WITH ... SELECT in a project file still runs
untouched -- the fix is not "ban CTEs".
isReadOnlySelect() stays in front of it rather than being replaced:
SQLite considers ATTACH, BEGIN and several PRAGMAs read-only too, since
none of them change the contents of the database, so dropping the
statement-type allowlist would have widened what is accepted while
fixing what is executed.
The check lives in its own translation unit depending on nothing but
QString and SQLite, so tests/qttest/tst_sqlreadonly.cpp can link it
alone and exercise the security property without standing up a
QETProject: 18 assertions covering the three CTE-prefixed writes named
in the review, bare writes, trailing statements, comment-only input
(which compiles to a null statement sqlite3_stmt_readonly() must not be
handed) and a null connection (refused, not waved through). Confirmed
the suite discriminates by deliberately disabling the new check and
watching exactly the nine write-refusal assertions go red while the
accept cases stayed green.
ctest 13/13, qet-coherence-check and qet-pdflink-check clean on the
example corpus.
Reported in PR #980's review thread by @elevatormind and confirmed
against this code by @scorpio810; fixed here on its own because the
flaw is in already-released code and needs none of that branch to
reach.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ispyisail