sqlite: fix use-after-free in StatementSync due to premature GC

nodejs · Feb 3, 2025 · 7185964 · 7185964
1 parent db7a31e
commit 7185964
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 11 deletions.
diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc
@@ -511,7 +511,8 @@ void DatabaseSync::Prepare(const FunctionCallbackInfo<Value>& args) {
   sqlite3_stmt* s = nullptr;
   int r = sqlite3_prepare_v2(db->connection_, *sql, -1, &s, 0);
   CHECK_ERROR_OR_THROW(env->isolate(), db->connection_, r, SQLITE_OK, void());
-  BaseObjectPtr<StatementSync> stmt = StatementSync::Create(env, db, s);
+  BaseObjectPtr<StatementSync> stmt =
+      StatementSync::Create(env, BaseObjectPtr<DatabaseSync>(db), s);
   db->statements_.insert(stmt.get());
   args.GetReturnValue().Set(stmt->object());
 }
@@ -923,11 +924,10 @@ void DatabaseSync::LoadExtension(const FunctionCallbackInfo<Value>& args) {
 
 StatementSync::StatementSync(Environment* env,
                              Local<Object> object,
-                             DatabaseSync* db,
+                             BaseObjectPtr<DatabaseSync> db,
                              sqlite3_stmt* stmt)
-    : BaseObject(env, object) {
+    : BaseObject(env, object), db_(std::move(db)) {
   MakeWeak();
-  db_ = db;
   statement_ = stmt;
   // In the future, some of these options could be set at the database
   // connection level and inherited by statements to reduce boilerplate.
@@ -1580,9 +1580,8 @@ Local<FunctionTemplate> StatementSync::GetConstructorTemplate(
   return tmpl;
 }
 
-BaseObjectPtr<StatementSync> StatementSync::Create(Environment* env,
-                                                   DatabaseSync* db,
-                                                   sqlite3_stmt* stmt) {
+BaseObjectPtr<StatementSync> StatementSync::Create(
+    Environment* env, BaseObjectPtr<DatabaseSync> db, sqlite3_stmt* stmt) {
   Local<Object> obj;
   if (!GetConstructorTemplate(env)
            ->InstanceTemplate()
@@ -1591,7 +1590,7 @@ BaseObjectPtr<StatementSync> StatementSync::Create(Environment* env,
     return BaseObjectPtr<StatementSync>();
   }
 
-  return MakeBaseObject<StatementSync>(env, obj, db, stmt);
+  return MakeBaseObject<StatementSync>(env, obj, std::move(db), stmt);
 }
 
 Session::Session(Environment* env,

diff --git a/src/node_sqlite.h b/src/node_sqlite.h
@@ -91,13 +91,13 @@ class StatementSync : public BaseObject {
  public:
   StatementSync(Environment* env,
                 v8::Local<v8::Object> object,
-                DatabaseSync* db,
+                BaseObjectPtr<DatabaseSync> db,
                 sqlite3_stmt* stmt);
   void MemoryInfo(MemoryTracker* tracker) const override;
   static v8::Local<v8::FunctionTemplate> GetConstructorTemplate(
       Environment* env);
   static BaseObjectPtr<StatementSync> Create(Environment* env,
-                                             DatabaseSync* db,
+                                             BaseObjectPtr<DatabaseSync> db,
                                              sqlite3_stmt* stmt);
   static void All(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void Iterate(const v8::FunctionCallbackInfo<v8::Value>& args);
@@ -117,7 +117,7 @@ class StatementSync : public BaseObject {
 
  private:
   ~StatementSync() override;
-  DatabaseSync* db_;
+  BaseObjectPtr<DatabaseSync> db_;
   sqlite3_stmt* statement_;
   bool use_big_ints_;
   bool allow_bare_named_params_;

diff --git a/test/parallel/test-sqlite-statement-sync.js b/test/parallel/test-sqlite-statement-sync.js
@@ -87,12 +87,14 @@ suite('StatementSync.prototype.all()', () => {
 suite('StatementSync.prototype.iterate()', () => {
   test('executes a query and returns an empty iterator on no results', (t) => {
     const db = new DatabaseSync(nextDb());
+    t.after(() => { db.close(); });
     const stmt = db.prepare('CREATE TABLE storage(key TEXT, val TEXT)');
     t.assert.deepStrictEqual(stmt.iterate().toArray(), []);
   });
 
   test('executes a query and returns all results', (t) => {
     const db = new DatabaseSync(nextDb());
+    t.after(() => { db.close(); });
     let stmt = db.prepare('CREATE TABLE storage(key TEXT, val TEXT)');
     t.assert.deepStrictEqual(stmt.run(), { changes: 0, lastInsertRowid: 0 });
     stmt = db.prepare('INSERT INTO storage (key, val) VALUES (?, ?)');