sqlite: reject statement-less SQL in SQLTagStore

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: #65157
Fixes: #65149
Reviewed-By: René <contact.9a5d6388@renegade334.me.uk>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>

Author: TrevorBurnham

src/node_sqlite.cc

@@ -3619,6 +3619,14 @@ BaseObjectPtr<StatementSync> SQLTagStore::PrepareStatement(
       return BaseObjectPtr<StatementSync>();
     }
 
+    // sqlite3_prepare_v2() reports success without producing a statement when
+    // the input holds no SQL, such as a comment. Such a statement cannot be
+    // bound or executed, so reject it instead of caching it.
+    if (s == nullptr) {
+      THROW_ERR_INVALID_ARG_VALUE(env, "The SQL query contains no statements.");
+      return BaseObjectPtr<StatementSync>();
+    }
+
     BaseObjectPtr<StatementSync> stmt_obj = StatementSync::Create(
         env, BaseObjectPtr<DatabaseSync>(session->database_), s);
 

test/parallel/test-sqlite-template-tag.js

@@ -317,6 +317,29 @@ test('sql error messages are descriptive', () => {
   });
 });
 
+test('rejects SQL that contains no statements', () => {
+  const expectedError = {
+    code: 'ERR_INVALID_ARG_VALUE',
+    message: /contains no statements/,
+  };
+
+  for (const method of ['run', 'get', 'all', 'iterate']) {
+    assert.throws(() => {
+      // eslint-disable-next-line no-unused-expressions
+      sql[method]`-- comment`;
+    }, expectedError);
+
+    assert.throws(() => {
+      // eslint-disable-next-line no-unused-expressions
+      sql[method]``;
+    }, expectedError);
+  }
+
+  // A rejected statement must not be cached, so a later valid query with the
+  // same tag store still works.
+  assert.strictEqual(sql.run`INSERT INTO foo (text) VALUES (${'bob'})`.changes, 1);
+});
+
 test('a tag store keeps the database alive by itself', () => {
   const sql = new DatabaseSync(':memory:').createTagStore();