security: require parameterized safe SQL
Jaime Fournier
7be7d56044d04b81af739effdfbe4c87240421c1
--- a/docs/kimi3-security-recommmendations.md +++ b/docs/kimi3-security-recommmendations.md @@ -808,6 +808,12 @@ making string-built SQL unrepresentable in safe code. `sql-interpolation` lint to error in safe-prelude files. - **Accept:** safe-prelude script cannot call the string-SQL API; lint errors on interpolation; `safety-guide.md` §4 updated. +- **Status:** complete for the safe prelude. `(jerboa prelude safe)` now + exposes `sqlite-query`, `sqlite-execute`, and `sqlite-exec` as syntax forms + that require a literal SQL string and at least one parameter; low-level + `sqlite-prepare`/`sqlite-bind`/`sqlite-step` are no longer exported there. + The `sql-interpolation` lint rule now reports errors, and + `docs/safety-guide.md` §4 documents the canonical surface. ### K3-P1-12 — Eliminate the safe-prelude import conflict **Serves:** G1, G4. **Effort:** 2 days. (Folded into P0-06 if done there.) --- a/docs/safety-guide.md +++ b/docs/safety-guide.md @@ -252,9 +252,11 @@ To check for leaked resources at shutdown: ## 4. Contract-Checked Standard Library -When you import `(jerboa prelude safe)`, the standard names (`sqlite-exec`, -`tcp-connect`, etc.) are transparently replaced with contract-checked -versions from `(std safe)`. +When you import `(jerboa prelude safe)`, the standard names (`sqlite-query`, +`sqlite-execute`, `tcp-connect`, etc.) are transparently replaced with safer +interfaces. SQLite query/execute calls must use a literal SQL string and at +least one parameter; file, network, JSON, FASL, and TCP APIs route through +contract-checked wrappers from `(std safe)`. ### Taint-Checked Sinks @@ -280,19 +282,27 @@ that imports raw Chez or unsafe modules bypasses these wrappers; keep ### What the Contracts Check -- **Type validation** before FFI calls — e.g., `sqlite-exec` verifies the +- **Type validation** before FFI calls — e.g., SQLite wrappers verify the database handle and SQL string before calling into C - **Post-condition checks** on results - **SQL injection heuristics** — multi-statement detection, comment injection, and other patterns are rejected at runtime +- **Safe-prelude SQL shape** — `(jerboa prelude safe)` exposes + `sqlite-query`, `sqlite-execute`, and `sqlite-exec` only as literal-SQL, + parameterized macros. `sqlite-prepare`/`sqlite-bind`/`sqlite-step` are not + exported by the safe prelude. ```scheme (import (jerboa prelude safe)) -;; This works — single statement, no injection patterns: -(sqlite-exec db "SELECT * FROM users WHERE id = ?") +;; This works — literal SQL plus a parameter: +(sqlite-query db "SELECT * FROM users WHERE id = ?" user-id) + +;; This is rejected by the safe prelude — no parameter: +(sqlite-query db "SELECT 1") -;; This is rejected — multi-statement detected: +;; This is rejected by the runtime heuristic if you explicitly leave the +;; safe prelude and call the lower-level wrappers: (sqlite-exec db "SELECT 1; DROP TABLE users") ;; => raises &db-query-error with SQL injection warning ``` @@ -308,7 +318,7 @@ queries as your primary defense**: ;; GOOD — parameterized: (sqlite-query db "SELECT * FROM users WHERE id = ?" user-id) -;; BAD — string interpolation (the lint rule will also warn): +;; BAD — string interpolation (the lint rule reports this as an error): (sqlite-query db (string-append "SELECT * FROM users WHERE id = " user-id)) ``` --- a/lib/jerboa/prelude/safe.ss +++ b/lib/jerboa/prelude/safe.ss @@ -9,8 +9,8 @@ ;;; (only (jerboa core) def) ;;; (only (jerboa core) def)) ;;; (with-resource ([db (sqlite-open "test.db")]) -;;; (sqlite-exec db "CREATE TABLE t(x)") -;;; (sqlite-query db "SELECT * FROM t")) +;;; (sqlite-execute db "INSERT INTO t(x) VALUES (?)" "value") +;;; (sqlite-query db "SELECT * FROM t WHERE x = ?" "value")) ;;; ;;; The safe versions: ;;; - Validate arguments before FFI calls @@ -99,9 +99,8 @@ ;; ---- Safe APIs under STANDARD names ---- - ;; SQLite (safe wrappers, renamed to standard names) + ;; SQLite (literal-SQL macros over safe parameterized wrappers) sqlite-open sqlite-close sqlite-exec sqlite-execute sqlite-query - sqlite-prepare sqlite-finalize sqlite-step sqlite-bind ;; TCP (safe wrappers, renamed to standard names) tcp-connect tcp-listen tcp-accept tcp-close @@ -227,16 +226,59 @@ ;; Re-export safe APIs under standard names ;; ========================================================================= - ;; SQLite + ;; SQLite. The safe prelude exposes only literal-SQL, parameterized calls + ;; under the standard names. Low-level raw-string prepare/bind/step/finalize + ;; remain outside this prelude so string-built SQL is not a safe-surface API. (def sqlite-open safe:safe-sqlite-open) (def sqlite-close safe:safe-sqlite-close) - (def sqlite-exec safe:safe-sqlite-exec) - (def sqlite-execute safe:safe-sqlite-execute) - (def sqlite-query safe:safe-sqlite-query) - (def sqlite-prepare safe:safe-sqlite-prepare) - (def sqlite-finalize safe:safe-sqlite-finalize) - (def sqlite-step safe:safe-sqlite-step) - (def sqlite-bind safe:safe-sqlite-bind) + + (define-syntax sqlite-execute + (lambda (stx) + (syntax-case stx () + ((_ db sql param ...) + (if (string? (syntax->datum #'sql)) + #'(safe:safe-sqlite-execute db sql param ...) + (syntax-violation 'sqlite-execute + "safe prelude SQL must be a literal string" + stx + #'sql))) + ((_ db sql) + (syntax-violation 'sqlite-execute + "safe prelude SQL calls require at least one parameter" + stx + #'sql))))) + + (define-syntax sqlite-query + (lambda (stx) + (syntax-case stx () + ((_ db sql param ...) + (if (string? (syntax->datum #'sql)) + #'(safe:safe-sqlite-query db sql param ...) + (syntax-violation 'sqlite-query + "safe prelude SQL must be a literal string" + stx + #'sql))) + ((_ db sql) + (syntax-violation 'sqlite-query + "safe prelude SQL calls require at least one parameter" + stx + #'sql))))) + + (define-syntax sqlite-exec + (lambda (stx) + (syntax-case stx () + ((_ db sql param ...) + (if (string? (syntax->datum #'sql)) + #'(safe:safe-sqlite-execute db sql param ...) + (syntax-violation 'sqlite-exec + "safe prelude SQL must be a literal string" + stx + #'sql))) + ((_ db sql) + (syntax-violation 'sqlite-exec + "safe prelude SQL calls require at least one parameter" + stx + #'sql))))) ;; TCP (def tcp-connect safe:safe-tcp-connect) --- a/lib/std/lint.ss +++ b/lib/std/lint.ss @@ -374,7 +374,7 @@ ;;; ---- sql-interpolation rule ---- ;; - ;; Warns when SQL-looking strings are built via string-append or format + ;; Errors when SQL-looking strings are built via string-append or format ;; inside sqlite-*/safe-sqlite-* calls. This catches: ;; (sqlite-exec db (string-append "SELECT * FROM " table)) ;; (sqlite-query db (format "SELECT * FROM ~a" table)) @@ -395,7 +395,7 @@ (memq (car sql-arg) '(string-append format string-concatenate))) (set! results - (cons (make-result severity-warn + (cons (make-result severity-error (format "SQL built via ~a — use parameterized queries instead" (car sql-arg)) 'sql-interpolation) --- a/tests/test-safe-prelude.ss +++ b/tests/test-safe-prelude.ss @@ -237,6 +237,19 @@ (if (memq 'sql-interpolation rules) #t #f)) #t) +(test "sql-interpolation: interpolation is an error" + (let* ([linter (make-linter)] + [results (lint-string linter + "(sqlite-query db (format \"SELECT * FROM ~a\" table))")] + [match (let loop ([xs results]) + (cond + [(null? xs) #f] + [(eq? (lint-result-rule (car xs)) 'sql-interpolation) + (car xs)] + [else (loop (cdr xs))]))]) + (and match (eq? (lint-result-severity match) severity-error))) + #t) + (test "sql-interpolation: literal string not flagged" (let* ([linter (make-linter)] [results (lint-string linter @@ -245,6 +258,34 @@ (if (memq 'sql-interpolation rules) #f #t)) #t) +(test "safe prelude: parameterized literal SQL expands" + (loader-ok? + (run-script-loader + "(import (jerboa prelude safe))\n(def (lookup db id) (sqlite-query db \"SELECT * FROM users WHERE id = ?\" id))\n" + #f)) + #t) + +(test "safe prelude: sqlite-query requires a parameter" + (loader-error? + (run-script-loader + "(import (jerboa prelude safe))\n(sqlite-query 'not-a-db \"SELECT 1\")\n" + #f)) + #t) + +(test "safe prelude: sqlite-query rejects built SQL" + (loader-error? + (run-script-loader + "(import (jerboa prelude safe))\n(sqlite-query 'not-a-db (string-append \"SELECT \" \"1\") 1)\n" + #f)) + #t) + +(test "safe prelude: low-level sqlite prepare is not exported" + (loader-error? + (run-script-loader + "(import (jerboa prelude safe))\n(def raw sqlite-prepare)\n" + #f)) + #t) + ;; ========================================================================= ;; 5. Lint: duplicate-import rule ;; =========================================================================