From b5dfc66efe251c5f399616f25ae889a9b17fbb83 Mon Sep 17 00:00:00 2001 From: Trivikram Kamat <16024985+trivikr@users.noreply.github.com> Date: Sun, 4 Oct 2026 18:58:40 -0700 Subject: [PATCH] sqlite: throw on oversized strings instead of aborting Several conversions from SQLite text to V8 strings passed a length of -1 to String::NewFromUtf8(). With that length, V8 skips its length check and aborts the process when the UTF-8 text exceeds String::kMaxLength. SQLite error messages embed the offending identifier or token, so CreateSQLiteErrorImpl() could hit this. NullableSQLiteStringToValue(), used for setAuthorizer() callback arguments and statement.columns() metadata, had the same problem, and the authorizer also called ToLocalChecked() on each argument. Convert error messages with Utf8StringMaybeOneByte(), and check the length up front in NullableSQLiteStringToValue(), so oversized strings throw ERR_STRING_TOO_LONG as column values already do. In the authorizer, deny the action and let the pending error reach the caller. Signed-off-by: Trivikram Kamat <16024985+trivikr@users.noreply.github.com> Assisted-by: claude:opus-5.5 --- src/node_sqlite.cc | 32 +++++++++++++++++--------- test/parallel/test-sqlite-statement.js | 30 +++++++++++++++++++++++- 2 files changed, 50 insertions(+), 12 deletions(-) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 8978670d07ed..8f9e2018d3bd 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -284,7 +284,9 @@ MaybeLocal CreateSQLiteErrorImpl(Isolate* isolate, Local context = isolate->GetCurrentContext(); Local js_msg; Local e; - if (!String::NewFromUtf8(isolate, message).ToLocal(&js_msg) || + // SQLite error messages embed the offending identifier or token, so they + // can exceed String::kMaxLength. + if (!Utf8StringMaybeOneByte(isolate, message).ToLocal(&js_msg) || !Exception::Error(js_msg)->ToObject(context).ToLocal(&e) || e->Set(context, env->code_string(), env->err_sqlite_error_string()) .IsNothing()) { @@ -399,7 +401,15 @@ inline MaybeLocal NullableSQLiteStringToValue(Isolate* isolate, return Null(isolate); } - return String::NewFromUtf8(isolate, str, NewStringType::kInternalized) + // With the default length of -1, V8 aborts on strings over kMaxLength. + const size_t len = strlen(str); + if (len > static_cast(String::kMaxLength)) [[unlikely]] { + isolate->ThrowException(node::ERR_STRING_TOO_LONG(isolate)); + return MaybeLocal(); + } + + return String::NewFromUtf8( + isolate, str, NewStringType::kInternalized, static_cast(len)) .As(); } @@ -3631,15 +3641,15 @@ int Database::AuthorizerCallback(void* user_data, Local callback = cb.As(); - LocalVector js_argv( - isolate, - { - Integer::New(isolate, action_code), - NullableSQLiteStringToValue(isolate, param1).ToLocalChecked(), - NullableSQLiteStringToValue(isolate, param2).ToLocalChecked(), - NullableSQLiteStringToValue(isolate, param3).ToLocalChecked(), - NullableSQLiteStringToValue(isolate, param4).ToLocalChecked(), - }); + LocalVector js_argv(isolate, {Integer::New(isolate, action_code)}); + for (const char* param : {param1, param2, param3, param4}) { + Local arg; + if (!NullableSQLiteStringToValue(isolate, param).ToLocal(&arg)) { + db->SetIgnoreNextSQLiteError(true); + return SQLITE_DENY; + } + js_argv.push_back(arg); + } MaybeLocal retval = callback->Call( context, Undefined(isolate), js_argv.size(), js_argv.data()); diff --git a/test/parallel/test-sqlite-statement.js b/test/parallel/test-sqlite-statement.js index d06b9018bf8e..79e72a89d18a 100644 --- a/test/parallel/test-sqlite-statement.js +++ b/test/parallel/test-sqlite-statement.js @@ -2,7 +2,11 @@ 'use strict'; const { enoughTestMem, skipIfSQLiteMissing } = require('../common'); skipIfSQLiteMissing(); -const { Database, Statement } = require('node:sqlite'); +const { + Database, + Statement, + constants: { SQLITE_OK }, +} = require('node:sqlite'); const { constants } = require('node:buffer'); const { suite, test } = require('node:test'); @@ -1448,6 +1452,9 @@ suite('values larger than the maximum string length', { skip: !enoughTestMem }, // hex() doubles its input, so this is the smallest blob whose text form // exceeds what V8 can hold in a string. const blobSize = (constants.MAX_STRING_LENGTH >>> 1) + 1; + // '\u20ac' is 1 UTF-16 code unit but 3 UTF-8 bytes, so this identifier is a + // valid JS string whose UTF-8 form exceeds the limit. + const longIdentifier = '\u20ac'.repeat(Math.ceil(constants.MAX_STRING_LENGTH / 3) + 10); const tooLong = { code: 'ERR_STRING_TOO_LONG', name: 'Error' }; test('get() throws instead of returning undefined', (t) => { @@ -1458,6 +1465,27 @@ suite('values larger than the maximum string length', { skip: !enoughTestMem }, }, tooLong); }); + test('prepare() throws when the SQLite error message is too long', (t) => { + using db = new Database(':memory:'); + // SQLite repeats the identifier in the error. + t.assert.throws(() => { + db.prepare(`SELECT 1 FROM "${longIdentifier}"`); + }, tooLong); + }); + + test('prepare() throws for an oversized authorizer argument', (t) => { + using db = new Database(':memory:'); + db.setAuthorizer(() => SQLITE_OK); + t.assert.throws(() => db.prepare(`CREATE TABLE "${longIdentifier}" (x)`), tooLong); + }); + + test('columns() throws for an oversized declared type', (t) => { + using db = new Database(':memory:'); + db.exec(`CREATE TABLE t (x "${longIdentifier}")`); + using stmt = db.prepare('SELECT x FROM t'); + t.assert.throws(() => stmt.columns(), tooLong); + }); + test('exec() surfaces the error from a user-defined function', (t) => { using db = new Database(':memory:'); db.exec('CREATE TABLE data(val TEXT)');