From 2aabe662e9287719df2795cf4d2c33ea22f8e4ef Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Fri, 14 Aug 2026 16:31:01 +0200 Subject: [PATCH 1/2] sqlite: copy changeset before applying it SQLite can invoke JavaScript conflict and filter callbacks while applying a changeset. Copy the input first so detaching or modifying its backing buffer during a callback does not affect the active operation. Signed-off-by: Matteo Collina --- src/node_sqlite.cc | 24 ++++++++++++-- test/parallel/test-sqlite-session.js | 48 ++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 2 deletions(-) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 05c4904fdd9..f3d23301c25 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -25,6 +25,7 @@ namespace sqlite { using v8::Array; using v8::ArrayBuffer; +using v8::BackingStore; using v8::BackingStoreInitializationMode; using v8::BackingStoreOnFailureMode; using v8::BigInt; @@ -2592,13 +2593,32 @@ void DatabaseSync::ApplyChangeset(const FunctionCallbackInfo& args) { BaseObjectPtr guard(db); ArrayBufferViewContents buf(args[0]); + if (buf.length() > std::numeric_limits::max()) { + THROW_ERR_OUT_OF_RANGE(env, "The changeset is too large."); + return; + } + + std::unique_ptr changeset; + if (buf.length() > 0) { + changeset = ArrayBuffer::NewBackingStore( + env->isolate(), + buf.length(), + BackingStoreInitializationMode::kUninitialized, + BackingStoreOnFailureMode::kReturnNull); + if (!changeset) { + THROW_ERR_MEMORY_ALLOCATION_FAILED(env); + return; + } + std::memcpy(changeset->Data(), buf.data(), buf.length()); + } + int r; { CallbackDepthGuard guard(db); r = sqlite3changeset_apply( db->connection_.get(), - buf.length(), - const_cast(static_cast(buf.data())), + static_cast(buf.length()), + changeset ? changeset->Data() : nullptr, context.filterCallback ? xFilter : nullptr, xConflict, static_cast(&context)); diff --git a/test/parallel/test-sqlite-session.js b/test/parallel/test-sqlite-session.js index a8bbaa77d06..68b2baed88a 100644 --- a/test/parallel/test-sqlite-session.js +++ b/test/parallel/test-sqlite-session.js @@ -282,6 +282,25 @@ suite('conflict resolution', () => { [{ value: 'world' }]); // unchanged }); + test('database.applyChangeset() - changeset detached by onConflict', (t) => { + const { database2, changeset } = prepareConflict(); + const result = database2.applyChangeset(changeset, { + onConflict: () => { + const transferred = structuredClone(changeset.buffer, { + transfer: [changeset.buffer], + }); + new Uint8Array(transferred).fill(0); + return constants.SQLITE_CHANGESET_REPLACE; + } + }); + + t.assert.strictEqual(result, true); + t.assert.strictEqual(changeset.byteLength, 0); + deepStrictEqual(t)( + database2.prepare('SELECT * FROM data ORDER BY key').all(), + [{ key: 1, value: 'hello' }, { key: 2, value: 'foo' }]); + }); + test('database.applyChangeset() - SQLITE_CHANGESET_DATA conflict handled with SQLITE_CHANGESET_REPLACE', (t) => { const { database2, changeset } = prepareDataConflict(); let conflictType = null; @@ -413,6 +432,35 @@ test('filter handler throws', (t) => { }); }); +test('database.applyChangeset() - changeset detached by filter', (t) => { + const database1 = new DatabaseSync(':memory:'); + const database2 = new DatabaseSync(':memory:'); + database1.exec('CREATE TABLE data(key INTEGER PRIMARY KEY)'); + database2.exec('CREATE TABLE data(key INTEGER PRIMARY KEY)'); + + const session = database1.createSession(); + database1.exec('INSERT INTO data VALUES (1), (2), (3)'); + const changeset = session.changeset(); + + const result = database2.applyChangeset(changeset, { + filter: () => { + const transferred = structuredClone(changeset.buffer, { + transfer: [changeset.buffer], + }); + new Uint8Array(transferred).fill(0); + return true; + } + }); + + t.assert.strictEqual(result, true); + t.assert.strictEqual(changeset.byteLength, 0); + deepStrictEqual(t)(database2.prepare('SELECT * FROM data').all(), [ + { key: 1 }, + { key: 2 }, + { key: 3 }, + ]); +}); + test('database.createSession() - filter changes', (t) => { const database1 = new DatabaseSync(':memory:'); const database2 = new DatabaseSync(':memory:'); From 994c5d3853e4b730563125cdb3fd852b2e2f5960 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Wed, 19 Aug 2026 17:10:00 +0000 Subject: [PATCH 2/2] sqlite: skip copying changeset when no callbacks are set Refactor: only copy the changeset before applying it when a filter or onConflict callback is present, since SQLite can only invoke JavaScript mid-apply in that case. Without callbacks, the input buffer cannot be detached or modified during sqlite3changeset_apply(), so no copy is needed and the buffer is passed through directly. Address review comment from geeksilva97. Signed-off-by: Matteo Collina --- src/node_sqlite.cc | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index f3d23301c25..c15d377c461 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -2585,11 +2585,8 @@ void DatabaseSync::ApplyChangeset(const FunctionCallbackInfo& args) { } } - // Keep the database alive during sqlite3changeset_apply(), which may - // call conflict or filter callbacks that trigger JavaScript execution. - // If the JavaScript callback drops all references to the database, - // the DatabaseSync could otherwise be garbage-collected while the - // callback is still executing, causing a use-after-free. + // Keep the database alive in case a callback drops all references to it, + // which could otherwise let it be garbage-collected mid-callback. BaseObjectPtr guard(db); ArrayBufferViewContents buf(args[0]); @@ -2598,8 +2595,12 @@ void DatabaseSync::ApplyChangeset(const FunctionCallbackInfo& args) { return; } + // A callback may detach/modify the input buffer mid-apply, so copy it. + // With no callbacks, no JS runs during sqlite3changeset_apply(), so no + // copy is needed. std::unique_ptr changeset; - if (buf.length() > 0) { + if (buf.length() > 0 && + (context.filterCallback || context.conflictCallback)) { changeset = ArrayBuffer::NewBackingStore( env->isolate(), buf.length(), @@ -2618,7 +2619,8 @@ void DatabaseSync::ApplyChangeset(const FunctionCallbackInfo& args) { r = sqlite3changeset_apply( db->connection_.get(), static_cast(buf.length()), - changeset ? changeset->Data() : nullptr, + changeset ? changeset->Data() + : const_cast(static_cast(buf.data())), context.filterCallback ? xFilter : nullptr, xConflict, static_cast(&context));