mirror of
https://github.com/qelectrotech/qelectrotech-source-mirror.git
synced 2026-09-26 11:54:14 +02:00
ispyisail
f777be05b4
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>
44 lines
1.4 KiB
C++
44 lines
1.4 KiB
C++
/*
|
|
Copyright 2006-2026 The QElectroTech Team
|
|
This file is part of QElectroTech.
|
|
|
|
QElectroTech is free software: you can redistribute it and/or modify
|
|
it under the terms of the GNU General Public License as published by
|
|
the Free Software Foundation, either version 2 of the License, or
|
|
(at your option) any later version.
|
|
|
|
QElectroTech is distributed in the hope that it will be useful,
|
|
but WITHOUT ANY WARRANTY; without even the implied warranty of
|
|
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
|
GNU General Public License for more details.
|
|
|
|
You should have received a copy of the GNU General Public License
|
|
along with QElectroTech. If not, see <http://www.gnu.org/licenses/>.
|
|
*/
|
|
#ifndef SQLREADONLY_H
|
|
#define SQLREADONLY_H
|
|
|
|
#include <QString>
|
|
|
|
struct sqlite3;
|
|
|
|
/**
|
|
Deciding whether a piece of SQL only reads.
|
|
|
|
Deliberately its own translation unit, depending on nothing but QString
|
|
and SQLite: it is the enforcement point for every query QElectroTech
|
|
runs against a project database, including queries that arrive from
|
|
outside the application (a .qet file's saved report/table query), so it
|
|
is worth being able to test it in isolation -- see
|
|
tests/qttest/tst_sqlreadonly.cpp, which links this file and nothing
|
|
else of QElectroTech.
|
|
*/
|
|
namespace QETSql {
|
|
|
|
bool isSingleReadOnlyStatement(sqlite3 *handle,
|
|
const QString &query,
|
|
QString *error = nullptr);
|
|
}
|
|
|
|
#endif // SQLREADONLY_H
|