Skip to content

fix(cpp): bind native ATTACH/DETACH parameters and use SQLCipher key API#409

Merged
ospfranco merged 2 commits into
OP-Engineering:mainfrom
pbbadenhorst:fix/sql-injection-attach-pragma
May 19, 2026
Merged

fix(cpp): bind native ATTACH/DETACH parameters and use SQLCipher key API#409
ospfranco merged 2 commits into
OP-Engineering:mainfrom
pbbadenhorst:fix/sql-injection-attach-pragma

Conversation

@pbbadenhorst
Copy link
Copy Markdown
Contributor

Summary

Hardens four native bridge surfaces that previously concatenated user-controlled values into SQL:

  • ATTACH/DETACH (SQLite, libsql, Turso bridges) — switches to bound parameters (ATTACH DATABASE ? AS ?, DETACH DATABASE ?). SQLite, libsql, and Turso all accept binding in both the path slot and the schema-name slot, so no escape logic is needed at all.
  • SQLCipher key handling — switches from PRAGMA key = '<key>' to the sqlite3_key_v2 C API.
  • Android CMakeLists.txt — orders cpp/sqlcipher ahead of cpp/ on the include path so the SQLCipher header (not the plain SQLite header) wins under USE_SQLCIPHER. Latent before this change because nothing referenced SQLCipher-only symbols; required now for sqlite3_key_v2 to link.
  • JS bridge — rejects zero bytes in attach/detach inputs before binding.
  • Adds regression tests for attach-alias injection, attach-path quoting, detach injection, and SQLCipher keys containing quotes.

Security framing

Not all of the above are security fixes; the diff is more honest if labelled:

  • Real injection class fixed: quote-based payloads in the SQLCipher key ('; ATTACH DATABASE '/tmp/evil.db' AS x; --) and in ATTACH/DETACH aliases. If those inputs were ever attacker-influenced (URL, deep link, IPC payload), the pre-fix code would have executed arbitrary SQL on the connection. The C API and parameter binding kill that.
  • Correctness, not security: zero-byte rejection in attach/detach. Zero-byte truncation in an attacker-controlled input only shortens the attacker's own value, so it isn't a useful exploit class. The realistic failure mode is a caller passing a path or alias that happens to contain a zero byte — silently truncated by libsql pre-fix, now rejected explicitly.
  • Defense in depth, not exploit: the SQLCipher key no longer enters SQL text and is therefore not visible to trace/log callbacks.

Notes

Turso attach/detach code was updated for parity, but the example tests keep the existing Turso skip because ATTACH support is not currently exercised there.

@pbbadenhorst
Copy link
Copy Markdown
Contributor Author

The zero byte checks might be too much - I am just paranoid doing these sort of changes.

@ospfranco
Copy link
Copy Markdown
Contributor

Cool, thanks!

@ospfranco ospfranco merged commit dff1c04 into OP-Engineering:main May 19, 2026
10 checks passed
@pbbadenhorst pbbadenhorst deleted the fix/sql-injection-attach-pragma branch May 19, 2026 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants