Problem
analyzer.IsMultiStatement counts a ; as a statement separator when any
dialect reading leaves it outside a literal, because neither reading of $$
is safe alone (PR #61):
- Read as a dollar delimiter,
UPDATE t AS $$ SET id = 1; DROP TABLE t hides
its ; behind an unterminated body — a real bypass on MySQL, where $$ is
just identifier bytes.
- Read as ordinary bytes,
SELECT $$'$$; DROP TABLE t hides its ; behind an
unterminated ordinary literal — a real bypass on PostgreSQL.
Running both is correct for a package-level function with no dialect context.
The cost is over-rejection:
$ sqlguard explain --dialect postgres "SELECT $$a ; b$$"
explain: refusing to explain multi-statement input
That is a valid single PostgreSQL statement.
Why this is fixable without new API
explain.PlanAnalyzer already knows its dialect — it is a constructor
argument validated to "postgres" or "mysql":
type PlanAnalyzer struct {
db *sql.DB
dialect string // "postgres" or "mysql"
allowDML bool
}
but validate calls analyzer.IsMultiStatement(q) with no dialect context
(explain/explain.go). The OR exists because the function cannot know the
target; the caller can. On PostgreSQL only the dollar-quote reading applies;
on MySQL only the ordinary reading does. Either way the correct single reading
is known, so the over-rejection is avoidable.
Caution
This relaxes a security control based on a user-supplied flag, so it needs a
deliberate decision, not just a patch. Points in favour: --dialect already
drives EXPLAIN syntax and the transaction mode (ReadOnly: !dml), so it is
already trusted for safety-relevant behaviour. Points against: a mismatched
flag would weaken the check, and the MySQL --allow-dml path runs read-write
where DDL implicit-commits past the rollback.
A dialect-aware separator check would also share plumbing with the dialect
hint proposed in #62 for Redact, so the two are worth designing together.
Acceptance criteria
sqlguard explain --dialect postgres "SELECT $$a ; b$$" is accepted.
- Both bypass payloads above stay refused on the dialect where they are a
threat; TestIsMultiStatementNeedsBothReadings still passes for the
dialect-less path.
- The dialect-less
analyzer.IsMultiStatement keeps its current fail-closed
behaviour for out-of-tree callers.
website/docs/explain.md updated with a version marker.
References: #61, #62
Problem
analyzer.IsMultiStatementcounts a;as a statement separator when anydialect reading leaves it outside a literal, because neither reading of
$$is safe alone (PR #61):
UPDATE t AS $$ SET id = 1; DROP TABLE thidesits
;behind an unterminated body — a real bypass on MySQL, where$$isjust identifier bytes.
SELECT $$'$$; DROP TABLE thides its;behind anunterminated ordinary literal — a real bypass on PostgreSQL.
Running both is correct for a package-level function with no dialect context.
The cost is over-rejection:
That is a valid single PostgreSQL statement.
Why this is fixable without new API
explain.PlanAnalyzeralready knows its dialect — it is a constructorargument validated to
"postgres"or"mysql":but
validatecallsanalyzer.IsMultiStatement(q)with no dialect context(
explain/explain.go). The OR exists because the function cannot know thetarget; the caller can. On PostgreSQL only the dollar-quote reading applies;
on MySQL only the ordinary reading does. Either way the correct single reading
is known, so the over-rejection is avoidable.
Caution
This relaxes a security control based on a user-supplied flag, so it needs a
deliberate decision, not just a patch. Points in favour:
--dialectalreadydrives EXPLAIN syntax and the transaction mode (
ReadOnly: !dml), so it isalready trusted for safety-relevant behaviour. Points against: a mismatched
flag would weaken the check, and the MySQL
--allow-dmlpath runs read-writewhere DDL implicit-commits past the rollback.
A dialect-aware separator check would also share plumbing with the dialect
hint proposed in #62 for
Redact, so the two are worth designing together.Acceptance criteria
sqlguard explain --dialect postgres "SELECT $$a ; b$$"is accepted.threat;
TestIsMultiStatementNeedsBothReadingsstill passes for thedialect-less path.
analyzer.IsMultiStatementkeeps its current fail-closedbehaviour for out-of-tree callers.
website/docs/explain.mdupdated with a version marker.References: #61, #62