amadan

carlos / platform Public

carlos db query: make the pragma guard an invariant over every token

The prefix check kept losing to disguises SQLite accepts: a comment then
a byte-order mark, a space then a BOM, repeated BOMs, a BOM after
EXPLAIN. It is replaced by a rule over the whole statement. A small
tokenizer drops comments and whitespace (space, tab, CR, LF, form feed,
vertical tab, and U+FEFF anywhere) and keeps string literals and quoted
identifiers as single tokens. Wherever a PRAGMA keyword token appears,
the pragma it names (bare, quoted or schema-qualified) may be given a
value only if it is a read-only one, in the (argument) form. The
one-statement check uses the same tokenizer for what follows the first
semicolon.

A generated test builds 8,624 disguises of query_only, temp_store,
temp_store_directory and writable_schema changes from BOMs, comments,
whitespace mixes and EXPLAIN [QUERY PLAN] leads, and asserts every one
is refused, plus the reads that must still run. Swapping the old prefix
check back in fails 4,250 of its assertions.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude cd153826f14e804fd930d6de9c783c2df65b3f3a parent d7a0c91
2 files changed, +154 −74
  • db.go +102 −74
  • db_test.go +52 −0
diff --git a/db.go b/db.go
index 86d3df454..1a5a7f39a 100644
--- a/db.go
+++ b/db.go
@@ -443,55 +443,118 @@ var dbReadPragmas = map[string]bool{
// changes the connection (query_only, temp_store, temp_store_directory,
// writable_schema and the rest), since a pragma is the one statement that
// can loosen what follows it.
+//
+// The pragma rule is an invariant over the whole statement's tokens, not a
+// prefix check: SQLite skips byte-order marks, comments and whitespace
+// anywhere, and applies a pragma at prepare even under EXPLAIN. So wherever
+// a PRAGMA keyword appears, the pragma it names may be given a value only
+// if it is a read-only one, and only in the (argument) form.
func checkQuerySQL(q string) error {
- first, rest := splitFirstStatement(q)
- if strings.TrimSpace(stripSQLComments(rest)) != "" {
+ _, rest := splitFirstStatement(q)
+ if len(sqlTokens(rest)) > 0 {
return errOneStatement
}
- // SQLite skips a leading byte-order mark and treats space, tab, CR, LF
- // and form feed as whitespace, and it applies a pragma at prepare even
- // under EXPLAIN [QUERY PLAN]. Normalize all three away before looking.
- stmt := strings.ToLower(stripSQLComments(strings.TrimPrefix(first, "\uFEFF")))
- stmt = strings.TrimSpace(strings.TrimSuffix(strings.TrimSpace(stmt), ";"))
- stmt = cutSQLKeyword(stmt, "explain")
- if rest := cutSQLKeyword(stmt, "query"); rest != stmt {
- stmt = cutSQLKeyword(rest, "plan")
- }
- body := cutSQLKeyword(stmt, "pragma")
- if body == stmt {
- return nil
- }
- // PRAGMA [schema.]name [= value | (value)]
- n := 0
- for n < len(body) && (isSQLIdentByte(body[n]) || body[n] == '.') {
- n++
- }
- name := body[:n]
- if i := strings.LastIndexByte(name, '.'); i >= 0 {
- name = name[i+1:]
- }
- after := strings.TrimSpace(body[n:])
- if after == "" {
- return nil // a bare read of the pragma's value
- }
- if dbReadPragmas[name] && strings.HasPrefix(after, "(") {
- return nil
+ toks := sqlTokens(q)
+ for i, tk := range toks {
+ if tk.kind != sqlWord || tk.text != "pragma" {
+ continue
+ }
+ // PRAGMA [schema .] name [= value | (value)]
+ j := i + 1
+ name := ""
+ if j < len(toks) && toks[j].kind != sqlPunct {
+ name = toks[j].text
+ j++
+ }
+ if j+1 < len(toks) && toks[j].kind == sqlPunct && toks[j].text == "." && toks[j+1].kind != sqlPunct {
+ name = toks[j+1].text
+ j += 2
+ }
+ if j >= len(toks) || toks[j].kind != sqlPunct || (toks[j].text != "=" && toks[j].text != "(") {
+ continue // a bare read of the pragma's value
+ }
+ if toks[j].text == "(" && dbReadPragmas[name] {
+ continue
+ }
+ return errPragmaChange
}
- return errPragmaChange
+ return nil
+}
+
+type sqlTokenKind int
+
+const (
+ sqlWord sqlTokenKind = iota // keyword or bare identifier, lower case
+ sqlQuoted // string literal or quoted identifier, unquoted, lower case
+ sqlPunct // any other single byte
+)
+
+type sqlToken struct {
+ kind sqlTokenKind
+ text string
}
-// cutSQLKeyword removes a leading keyword (already lower case) and the
-// whitespace after it, or returns s unchanged when it does not start with
-// that whole word.
-func cutSQLKeyword(s, kw string) string {
- if !strings.HasPrefix(s, kw) || (len(s) > len(kw) && isSQLIdentByte(s[len(kw)])) {
- return s
+const utf8BOM = "\uFEFF"
+
+// sqlTokens splits q the way SQLite's tokenizer does, for the guard's
+// purposes: comments and whitespace (space, tab, newline, carriage return,
+// form feed, vertical tab, and a byte-order mark anywhere) separate tokens
+// and vanish; string literals and quoted identifiers ('...', "...", `...`,
+// [...]) are one token each; runs of identifier bytes are words; anything
+// else is one punctuation byte.
+func sqlTokens(q string) []sqlToken {
+ var out []sqlToken
+ for i := 0; i < len(q); {
+ c := q[i]
+ switch {
+ case strings.HasPrefix(q[i:], utf8BOM):
+ i += len(utf8BOM)
+ case c == ' ' || c == '\t' || c == '\n' || c == '\r' || c == '\f' || c == '\v':
+ i++
+ case c == '-' && i+1 < len(q) && q[i+1] == '-':
+ if j := strings.IndexByte(q[i:], '\n'); j >= 0 {
+ i += j + 1
+ } else {
+ i = len(q)
+ }
+ case c == '/' && i+1 < len(q) && q[i+1] == '*':
+ if j := strings.Index(q[i+2:], "*/"); j >= 0 {
+ i += j + 4
+ } else {
+ i = len(q)
+ }
+ case c == '\'' || c == '"' || c == '`':
+ end := min(skipQuoted(q, i, c), len(q))
+ body := strings.ReplaceAll(q[i+1:end], string([]byte{c, c}), string(c))
+ out = append(out, sqlToken{sqlQuoted, strings.ToLower(body)})
+ i = end + 1
+ case c == '[':
+ j := strings.IndexByte(q[i:], ']')
+ if j < 0 {
+ j = len(q) - i
+ }
+ out = append(out, sqlToken{sqlQuoted, strings.ToLower(q[i+1 : i+j])})
+ i += j + 1
+ case isSQLIdentByte(c):
+ j := i
+ for j < len(q) && isSQLIdentByte(q[j]) && !strings.HasPrefix(q[j:], utf8BOM) {
+ j++
+ }
+ out = append(out, sqlToken{sqlWord, strings.ToLower(q[i:j])})
+ i = j
+ default:
+ out = append(out, sqlToken{sqlPunct, string(c)})
+ i++
+ }
}
- return strings.TrimSpace(s[len(kw):])
+ return out
}
+// isSQLIdentByte reports a byte that can continue an unquoted identifier:
+// ASCII letters, digits, _ and $, and any byte of a multi-byte UTF-8
+// character (the byte-order mark is checked for before this).
func isSQLIdentByte(c byte) bool {
- return c == '_' || c >= 'a' && c <= 'z' || c >= 'A' && c <= 'Z' || c >= '0' && c <= '9'
+ return c == '_' || c == '$' || c >= 'a' && c <= 'z' || c >= 'A' && c <= 'Z' || c >= '0' && c <= '9' || c >= 0x80
}
// splitFirstStatement returns the text up to and including the first
@@ -545,38 +608,3 @@ func skipQuoted(q string, i int, quote byte) int {
}
return len(q)
}
-
-// stripSQLComments removes -- and /* */ comments outside quotes, leaving a
-// space in their place.
-func stripSQLComments(q string) string {
- var b strings.Builder
- for i := 0; i < len(q); i++ {
- c := q[i]
- switch {
- case c == '\'' || c == '"' || c == '`':
- j := skipQuoted(q, i, c)
- if j >= len(q) {
- j = len(q) - 1
- }
- b.WriteString(q[i : j+1])
- i = j
- case c == '-' && i+1 < len(q) && q[i+1] == '-':
- j := strings.IndexByte(q[i:], '\n')
- if j < 0 {
- return b.String()
- }
- b.WriteByte(' ')
- i += j
- case c == '/' && i+1 < len(q) && q[i+1] == '*':
- j := strings.Index(q[i+2:], "*/")
- if j < 0 {
- return b.String()
- }
- b.WriteByte(' ')
- i += j + 3
- default:
- b.WriteByte(c)
- }
- }
- return b.String()
-}
diff --git a/db_test.go b/db_test.go
index b7a65f520..94d7c688c 100644
--- a/db_test.go
+++ b/db_test.go
@@ -508,3 +508,55 @@ func TestPullPublishesNothingOnceCancelled(t *testing.T) {
t.Fatalf("a cancelled pull left %s (%v)", out, err)
}
}
+
+// The pragma guard is an invariant over the whole statement, not a prefix
+// check: SQLite skips byte-order marks, comments and every kind of
+// whitespace anywhere, and applies a pragma at prepare even under EXPLAIN.
+// This generates the disguises from parts and asserts every one is refused.
+func TestQueryRefusesEveryDisguiseOfAPragmaChange(t *testing.T) {
+ bom := "\uFEFF"
+ gaps := []string{"", " ", "\t", "\n", "\f", "\r\n", bom, " " + bom, bom + bom, "/*x*/", "/*x*/" + bom, bom + "/**/ ", "-- c\n", " /* a */ /* b */ "}
+ leads := []string{"", "EXPLAIN", "explain query plan", "EXPLAIN QUERY PLAN"}
+ changes := []struct{ name, set string }{
+ {"query_only", "=0"}, {"query_only", "(0)"}, {"temp_store", "=1"}, {"temp_store", "=FILE"},
+ {"temp_store_directory", "='/tmp/x'"}, {"writable_schema", "=1"}, {"main.writable_schema", "=ON"},
+ {`"query_only"`, "=0"}, {"[query_only]", "=0"}, {"`temp_store`", "=1"}, {"main . query_only", "=0"},
+ }
+ path := fixtureDB(t)
+ n := 0
+ for _, lead := range leads {
+ for _, g1 := range gaps {
+ for _, g2 := range gaps {
+ for _, c := range changes {
+ sep := g2
+ if sep == "" {
+ sep = " " // two words need something between them
+ }
+ stmt := g1 + lead + sep + "PRAGMA" + sep + c.name + g1 + c.set
+ if err := checkQuerySQL(stmt); err == nil || !strings.Contains(err.Error(), "pragma") {
+ t.Errorf("%q: error = %v, want the pragma refusal", stmt, err)
+ }
+ n++
+ }
+ }
+ }
+ }
+ if n < 1000 {
+ t.Fatalf("generated only %d cases", n)
+ }
+ // And end to end, a handful through the real query path.
+ for _, stmt := range []string{"/*x*/" + bom + "PRAGMA query_only=0", " " + bom + "PRAGMA query_only=0", bom + bom + "PRAGMA temp_store=1", "EXPLAIN " + bom + "PRAGMA query_only=0"} {
+ if err := runQuery(&bytes.Buffer{}, path, stmt); err == nil || !strings.Contains(err.Error(), "pragma") {
+ t.Errorf("%q through runQuery: error = %v, want the pragma refusal", stmt, err)
+ }
+ }
+ // What must still run.
+ for _, stmt := range []string{
+ "PRAGMA table_info(users)", bom + " PRAGMA table_info ( 'users' )", "PRAGMA query_only", "EXPLAIN PRAGMA query_only",
+ "SELECT 'PRAGMA query_only=0' AS s", `SELECT 1 AS "pragma"`, "SELECT 1 -- PRAGMA query_only=0", "SELECT 1 /* PRAGMA temp_store=1 */",
+ } {
+ if err := runQuery(&bytes.Buffer{}, path, stmt); err != nil {
+ t.Errorf("%q: error = %v, want it to run", stmt, err)
+ }
+ }
+}