sqlite: reject statement-less SQL in SQLTagStore by TrevorBurnham · Pull Request #65157 · nodejs/node

TrevorBurnham · 2026-08-09T01:45:16Z

sqlite3_prepare_v2() returns SQLITE_OK without producing a statement when its input holds no SQL, such as a comment or an empty string. Neither caller checked for this.

SQLTagStore cached a StatementSync wrapping the null sqlite3_stmt. Executing it reached sqlite3_clear_bindings(), which only guards against a null statement under SQLITE_ENABLE_API_ARMOR, and segfaulted:

constdb=newDatabaseSync(':memory:');conststore=db.createTagStore();store.run`-- comment`;// segfault

All four query methods (run, get, all, iterate) were affected.

db.prepare() returned a StatementSync whose statement_ was null. Every method on it threw statement has been finalized, which was misleading because nothing had been finalized. It was also still inserted into statements_, and because IsFinalized() is true for a null statement, its destructor skipped UntrackStatement() and left a dangling pointer that a later close() would finalize.

Both now throw ERR_INVALID_ARG_VALUE at preparation. SQL that merely contains comments is unaffected.

Fixes: #65149

nodejs-github-bot · 2026-08-09T01:45:21Z

Review requested:

@nodejs/sqlite

Apply the same check to DatabaseSync::Prepare() so that statement-less SQL is rejected at preparation instead of on first use. Previously db.prepare('-- comment') returned a StatementSync whose statement_ was null. Every method on it threw "statement has been finalized", which was misleading because nothing had been finalized, and the object was still inserted into statements_. Since IsFinalized() is true for a null statement, its destructor skipped UntrackStatement() and left a dangling pointer in the set that a later close() would finalize. Refs: nodejs#65157 (comment) Signed-off-by: Trevor Burnham <trevorburnham@gmail.com>

Apply the same check to DatabaseSync::Prepare() so that statement-less SQL is rejected at preparation instead of on first use. This matches SQLite's own oo1 JavaScript API, which throws when the SQL contains no statements rather than exposing the C API's null statement pointer. Previously db.prepare('-- comment') returned a StatementSync whose statement_ was null. Every method on it threw "statement has been finalized", which was misleading because nothing had been finalized, and the object was still inserted into statements_. Since IsFinalized() is true for a null statement, its destructor skipped UntrackStatement() and left a dangling pointer in the set that a later close() would finalize. Refs: nodejs#65157 (comment) Refs: https://sqlite.org/wasm/doc/trunk/api-oo1.md Signed-off-by: Trevor Burnham <trevorburnham@gmail.com>

Renegade334

👍👍

Apply the same check to DatabaseSync::Prepare() so that statement-less SQL is rejected at preparation instead of on first use. This matches SQLite's own oo1 JavaScript API, which throws when the SQL contains no statements rather than exposing the C API's null statement pointer. Previously db.prepare('-- comment') returned a StatementSync whose statement_ was null. Every method on it threw "statement has been finalized", which was misleading because nothing had been finalized, and the object was still inserted into statements_. Since IsFinalized() is true for a null statement, its destructor skipped UntrackStatement() and left a dangling pointer in the set that a later close() would finalize. Refs: nodejs#65157 (comment) Refs: https://sqlite.org/wasm/doc/trunk/api-oo1.md Signed-off-by: Trevor Burnham <trevorburnham@gmail.com>

codecov · 2026-08-10T01:39:09Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.33%. Comparing base (b2b7405) to head (d7ad352).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #65157 +/- ##
==========================================
- Coverage 90.33% 90.33% -0.01% 
==========================================
Files 760 760 Lines 248523 248529 +6 Branches 46906 46895 -11 ==========================================
- Hits 224511 224509 -2 + Misses 15451 15450 -1 - Partials 8561 8570 +9

Files with missing lines	Coverage Δ
src/node_sqlite.cc	`81.49% <100.00%> (+0.25%)`	⬆️

... and 23 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

sqlite3_prepare_v2() returns SQLITE_OK without producing a statement when its input holds no SQL, such as a comment. PrepareStatement() only checked the return code, so it cached a StatementSync wrapping a null sqlite3_stmt. Executing it reached sqlite3_clear_bindings(), which only guards against a null statement under SQLITE_ENABLE_API_ARMOR, and segfaulted. Reject such input instead of caching it. The StatementSync methods already avoid the crash because their IsFinalized() guard treats a null statement as finalized. Fixes: nodejs#65149 Signed-off-by: Trevor Burnham <trevorburnham@gmail.com>

Apply the same check to DatabaseSync::Prepare() so that statement-less SQL is rejected at preparation instead of on first use. This matches SQLite's own oo1 JavaScript API, which throws when the SQL contains no statements rather than exposing the C API's null statement pointer. Previously db.prepare('-- comment') returned a StatementSync whose statement_ was null. Every method on it threw "statement has been finalized", which was misleading because nothing had been finalized, and the object was still inserted into statements_. Since IsFinalized() is true for a null statement, its destructor skipped UntrackStatement() and left a dangling pointer in the set that a later close() would finalize. Refs: nodejs#65157 (comment) Refs: https://sqlite.org/wasm/doc/trunk/api-oo1.md Signed-off-by: Trevor Burnham <trevorburnham@gmail.com>

nodejs-github-bot · 2026-08-10T03:59:26Z

CI: https://ci.nodejs.org/job/node-test-pull-request/75723/

nodejs-github-bot · 2026-08-11T03:14:54Z

Landed in a576f1c...cb9bb66

sqlite3_prepare_v2() returns SQLITE_OK without producing a statement when its input holds no SQL, such as a comment. PrepareStatement() only checked the return code, so it cached a StatementSync wrapping a null sqlite3_stmt. Executing it reached sqlite3_clear_bindings(), which only guards against a null statement under SQLITE_ENABLE_API_ARMOR, and segfaulted. Reject such input instead of caching it. The StatementSync methods already avoid the crash because their IsFinalized() guard treats a null statement as finalized. Fixes: #65149 Signed-off-by: Trevor Burnham <trevorburnham@gmail.com> PR-URL: #65157Fixes: #65149 Reviewed-By: René <contact.9a5d6388@renegade334.me.uk> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>

Apply the same check to DatabaseSync::Prepare() so that statement-less SQL is rejected at preparation instead of on first use. This matches SQLite's own oo1 JavaScript API, which throws when the SQL contains no statements rather than exposing the C API's null statement pointer. Previously db.prepare('-- comment') returned a StatementSync whose statement_ was null. Every method on it threw "statement has been finalized", which was misleading because nothing had been finalized, and the object was still inserted into statements_. Since IsFinalized() is true for a null statement, its destructor skipped UntrackStatement() and left a dangling pointer in the set that a later close() would finalize. Refs: #65157 (comment) Refs: https://sqlite.org/wasm/doc/trunk/api-oo1.md Signed-off-by: Trevor Burnham <trevorburnham@gmail.com> PR-URL: #65157Fixes: #65149 Reviewed-By: René <contact.9a5d6388@renegade334.me.uk> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>

sqlite3_prepare_v2() returns SQLITE_OK without producing a statement when its input holds no SQL, such as a comment. PrepareStatement() only checked the return code, so it cached a StatementSync wrapping a null sqlite3_stmt. Executing it reached sqlite3_clear_bindings(), which only guards against a null statement under SQLITE_ENABLE_API_ARMOR, and segfaulted. Reject such input instead of caching it. The StatementSync methods already avoid the crash because their IsFinalized() guard treats a null statement as finalized. Fixes: #65149 Signed-off-by: Trevor Burnham <trevorburnham@gmail.com> PR-URL: #65157Fixes: #65149 Reviewed-By: René <contact.9a5d6388@renegade334.me.uk> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>

Apply the same check to DatabaseSync::Prepare() so that statement-less SQL is rejected at preparation instead of on first use. This matches SQLite's own oo1 JavaScript API, which throws when the SQL contains no statements rather than exposing the C API's null statement pointer. Previously db.prepare('-- comment') returned a StatementSync whose statement_ was null. Every method on it threw "statement has been finalized", which was misleading because nothing had been finalized, and the object was still inserted into statements_. Since IsFinalized() is true for a null statement, its destructor skipped UntrackStatement() and left a dangling pointer in the set that a later close() would finalize. Refs: #65157 (comment) Refs: https://sqlite.org/wasm/doc/trunk/api-oo1.md Signed-off-by: Trevor Burnham <trevorburnham@gmail.com> PR-URL: #65157Fixes: #65149 Reviewed-By: René <contact.9a5d6388@renegade334.me.uk> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>

sqlite3_prepare_v2() returns SQLITE_OK without producing a statement when its input holds no SQL, such as a comment. PrepareStatement() only checked the return code, so it cached a StatementSync wrapping a null sqlite3_stmt. Executing it reached sqlite3_clear_bindings(), which only guards against a null statement under SQLITE_ENABLE_API_ARMOR, and segfaulted. Reject such input instead of caching it. The StatementSync methods already avoid the crash because their IsFinalized() guard treats a null statement as finalized. Fixes: #65149 Signed-off-by: Trevor Burnham <trevorburnham@gmail.com> PR-URL: #65157Fixes: #65149 Reviewed-By: René <contact.9a5d6388@renegade334.me.uk> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>

Apply the same check to DatabaseSync::Prepare() so that statement-less SQL is rejected at preparation instead of on first use. This matches SQLite's own oo1 JavaScript API, which throws when the SQL contains no statements rather than exposing the C API's null statement pointer. Previously db.prepare('-- comment') returned a StatementSync whose statement_ was null. Every method on it threw "statement has been finalized", which was misleading because nothing had been finalized, and the object was still inserted into statements_. Since IsFinalized() is true for a null statement, its destructor skipped UntrackStatement() and left a dangling pointer in the set that a later close() would finalize. Refs: #65157 (comment) Refs: https://sqlite.org/wasm/doc/trunk/api-oo1.md Signed-off-by: Trevor Burnham <trevorburnham@gmail.com> PR-URL: #65157Fixes: #65149 Reviewed-By: René <contact.9a5d6388@renegade334.me.uk> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>

sqlite3_prepare_v2() returns SQLITE_OK without producing a statement when its input holds no SQL, such as a comment. PrepareStatement() only checked the return code, so it cached a StatementSync wrapping a null sqlite3_stmt. Executing it reached sqlite3_clear_bindings(), which only guards against a null statement under SQLITE_ENABLE_API_ARMOR, and segfaulted. Reject such input instead of caching it. The StatementSync methods already avoid the crash because their IsFinalized() guard treats a null statement as finalized. Fixes: #65149 Signed-off-by: Trevor Burnham <trevorburnham@gmail.com> PR-URL: #65157Fixes: #65149 Reviewed-By: René <contact.9a5d6388@renegade334.me.uk> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>

Apply the same check to DatabaseSync::Prepare() so that statement-less SQL is rejected at preparation instead of on first use. This matches SQLite's own oo1 JavaScript API, which throws when the SQL contains no statements rather than exposing the C API's null statement pointer. Previously db.prepare('-- comment') returned a StatementSync whose statement_ was null. Every method on it threw "statement has been finalized", which was misleading because nothing had been finalized, and the object was still inserted into statements_. Since IsFinalized() is true for a null statement, its destructor skipped UntrackStatement() and left a dangling pointer in the set that a later close() would finalize. Refs: #65157 (comment) Refs: https://sqlite.org/wasm/doc/trunk/api-oo1.md Signed-off-by: Trevor Burnham <trevorburnham@gmail.com> PR-URL: #65157Fixes: #65149 Reviewed-By: René <contact.9a5d6388@renegade334.me.uk> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>

TrevorBurnham · 2026-08-09T01:45:16Z

sqlite3_prepare_v2() returns SQLITE_OK without producing a statement when its input holds no SQL, such as a comment or an empty string. Neither caller checked for this.

SQLTagStore cached a StatementSync wrapping the null sqlite3_stmt. Executing it reached sqlite3_clear_bindings(), which only guards against a null statement under SQLITE_ENABLE_API_ARMOR, and segfaulted:

constdb=newDatabaseSync(':memory:');conststore=db.createTagStore();store.run`-- comment`;// segfault

All four query methods (run, get, all, iterate) were affected.

db.prepare() returned a StatementSync whose statement_ was null. Every method on it threw statement has been finalized, which was misleading because nothing had been finalized. It was also still inserted into statements_, and because IsFinalized() is true for a null statement, its destructor skipped UntrackStatement() and left a dangling pointer that a later close() would finalize.

Both now throw ERR_INVALID_ARG_VALUE at preparation. SQL that merely contains comments is unaffected.

Fixes: #65149

nodejs-github-bot · 2026-08-09T01:45:21Z

Review requested:

@nodejs/sqlite

Apply the same check to DatabaseSync::Prepare() so that statement-less SQL is rejected at preparation instead of on first use. Previously db.prepare('-- comment') returned a StatementSync whose statement_ was null. Every method on it threw "statement has been finalized", which was misleading because nothing had been finalized, and the object was still inserted into statements_. Since IsFinalized() is true for a null statement, its destructor skipped UntrackStatement() and left a dangling pointer in the set that a later close() would finalize. Refs: nodejs#65157 (comment) Signed-off-by: Trevor Burnham <trevorburnham@gmail.com>

Apply the same check to DatabaseSync::Prepare() so that statement-less SQL is rejected at preparation instead of on first use. This matches SQLite's own oo1 JavaScript API, which throws when the SQL contains no statements rather than exposing the C API's null statement pointer. Previously db.prepare('-- comment') returned a StatementSync whose statement_ was null. Every method on it threw "statement has been finalized", which was misleading because nothing had been finalized, and the object was still inserted into statements_. Since IsFinalized() is true for a null statement, its destructor skipped UntrackStatement() and left a dangling pointer in the set that a later close() would finalize. Refs: nodejs#65157 (comment) Refs: https://sqlite.org/wasm/doc/trunk/api-oo1.md Signed-off-by: Trevor Burnham <trevorburnham@gmail.com>

Renegade334

👍👍

Apply the same check to DatabaseSync::Prepare() so that statement-less SQL is rejected at preparation instead of on first use. This matches SQLite's own oo1 JavaScript API, which throws when the SQL contains no statements rather than exposing the C API's null statement pointer. Previously db.prepare('-- comment') returned a StatementSync whose statement_ was null. Every method on it threw "statement has been finalized", which was misleading because nothing had been finalized, and the object was still inserted into statements_. Since IsFinalized() is true for a null statement, its destructor skipped UntrackStatement() and left a dangling pointer in the set that a later close() would finalize. Refs: nodejs#65157 (comment) Refs: https://sqlite.org/wasm/doc/trunk/api-oo1.md Signed-off-by: Trevor Burnham <trevorburnham@gmail.com>

codecov · 2026-08-10T01:39:09Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.33%. Comparing base (b2b7405) to head (d7ad352).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #65157 +/- ##
==========================================
- Coverage 90.33% 90.33% -0.01% 
==========================================
Files 760 760 Lines 248523 248529 +6 Branches 46906 46895 -11 ==========================================
- Hits 224511 224509 -2 + Misses 15451 15450 -1 - Partials 8561 8570 +9

Files with missing lines	Coverage Δ
src/node_sqlite.cc	`81.49% <100.00%> (+0.25%)`	⬆️

... and 23 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

sqlite3_prepare_v2() returns SQLITE_OK without producing a statement when its input holds no SQL, such as a comment. PrepareStatement() only checked the return code, so it cached a StatementSync wrapping a null sqlite3_stmt. Executing it reached sqlite3_clear_bindings(), which only guards against a null statement under SQLITE_ENABLE_API_ARMOR, and segfaulted. Reject such input instead of caching it. The StatementSync methods already avoid the crash because their IsFinalized() guard treats a null statement as finalized. Fixes: nodejs#65149 Signed-off-by: Trevor Burnham <trevorburnham@gmail.com>

Apply the same check to DatabaseSync::Prepare() so that statement-less SQL is rejected at preparation instead of on first use. This matches SQLite's own oo1 JavaScript API, which throws when the SQL contains no statements rather than exposing the C API's null statement pointer. Previously db.prepare('-- comment') returned a StatementSync whose statement_ was null. Every method on it threw "statement has been finalized", which was misleading because nothing had been finalized, and the object was still inserted into statements_. Since IsFinalized() is true for a null statement, its destructor skipped UntrackStatement() and left a dangling pointer in the set that a later close() would finalize. Refs: nodejs#65157 (comment) Refs: https://sqlite.org/wasm/doc/trunk/api-oo1.md Signed-off-by: Trevor Burnham <trevorburnham@gmail.com>

nodejs-github-bot · 2026-08-10T03:59:26Z

CI: https://ci.nodejs.org/job/node-test-pull-request/75723/

nodejs-github-bot · 2026-08-11T03:14:54Z