From 89990be56c85257f2aed0565f3f056b64d01b6c7 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 | 33 +++++++++++++++---- test/parallel/test-sqlite-session.js | 48 ++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 7 deletions(-) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 038af9812f9e..4430e7bf7f59 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -23,6 +23,7 @@ namespace sqlite { using v8::Array; using v8::ArrayBuffer; +using v8::BackingStore; using v8::BackingStoreInitializationMode; using v8::BackingStoreOnFailureMode; using v8::BigInt; @@ -2404,16 +2405,34 @@ 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_, - buf.length(), - const_cast(static_cast(buf.data())), - context.filterCallback ? xFilter : nullptr, - xConflict, - static_cast(&context)); + r = sqlite3changeset_apply(db->connection_, + static_cast(buf.length()), + changeset ? changeset->Data() : nullptr, + context.filterCallback ? xFilter : nullptr, + xConflict, + static_cast(&context)); } if (r == SQLITE_OK) { args.GetReturnValue().Set(true); diff --git a/test/parallel/test-sqlite-session.js b/test/parallel/test-sqlite-session.js index c36b4352a341..9f5161da5501 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; @@ -406,6 +425,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 385708b38b3fa5fa18b89d29fa0a114f32f2bf4d 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 | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 4430e7bf7f59..a58780af1dac 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -2397,11 +2397,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]); @@ -2410,8 +2407,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(), @@ -2427,12 +2428,14 @@ void DatabaseSync::ApplyChangeset(const FunctionCallbackInfo& args) { int r; { CallbackDepthGuard guard(db); - r = sqlite3changeset_apply(db->connection_, - static_cast(buf.length()), - changeset ? changeset->Data() : nullptr, - context.filterCallback ? xFilter : nullptr, - xConflict, - static_cast(&context)); + r = sqlite3changeset_apply( + db->connection_, + static_cast(buf.length()), + changeset ? changeset->Data() + : const_cast(static_cast(buf.data())), + context.filterCallback ? xFilter : nullptr, + xConflict, + static_cast(&context)); } if (r == SQLITE_OK) { args.GetReturnValue().Set(true);