sqlite: support db.loadExtension

himself65 · targos · commit 53cc0cc744b1 · 2024-12-13T16:54:38.000+01:00
PR-URL: #53900 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
diff --git a/Makefile b/Makefile
@@ -294,6 +294,7 @@ coverage-report-js: ## Report JavaScript coverage results.
 cctest: all ## Run the C++ tests using the built `cctest` executable.
 	@out/$(BUILDTYPE)/$@ --gtest_filter=$(GTEST_FILTER)
 	$(NODE) ./test/embedding/test-embedding.js
+	$(NODE) ./test/sqlite/test-sqlite-extensions.mjs
 
 .PHONY: list-gtests
 list-gtests: ## List all available C++ gtests.
@@ -574,6 +575,7 @@ test-ci: | clear-stalled bench-addons-build build-addons build-js-native-api-tes
 		--mode=$(BUILDTYPE_LOWER) --flaky-tests=$(FLAKY_TESTS) \
 		$(TEST_CI_ARGS) $(CI_JS_SUITES) $(CI_NATIVE_SUITES) $(CI_DOC)
 	$(NODE) ./test/embedding/test-embedding.js
+	$(NODE) ./test/sqlite/test-sqlite-extensions.mjs
 	$(info Clean up any leftover processes, error if found.)
 	ps awwx | grep Release/node | grep -v grep | cat
 	@PS_OUT=`ps awwx | grep Release/node | grep -v grep | awk '{print $$1}'`; \
@@ -1432,6 +1434,7 @@ LINT_CPP_FILES = $(filter-out $(LINT_CPP_EXCLUDE), $(wildcard \
 	test/cctest/*.h \
 	test/embedding/*.cc \
 	test/embedding/*.h \
+	test/sqlite/*.c \
 	test/fixtures/*.c \
 	test/js-native-api/*/*.cc \
 	test/node-api/*/*.cc \
diff --git a/doc/api/errors.md b/doc/api/errors.md
@@ -2172,6 +2172,16 @@ added:
 An ESM loader hook returned without calling `next()` and without explicitly
 signaling a short circuit.
 
+<a id="ERR_LOAD_SQLITE_EXTENSION"></a>
+
+### `ERR_LOAD_SQLITE_EXTENSION`
+
+<!-- YAML
+added: REPLACEME
+-->
+
+An error occurred while loading a SQLite extension.
+
 <a id="ERR_MEMORY_ALLOCATION_FAILED"></a>
 
 ### `ERR_MEMORY_ALLOCATION_FAILED`
diff --git a/doc/api/permissions.md b/doc/api/permissions.md
@@ -147,6 +147,8 @@ There are constraints you need to know before using this system:
   flags that can be set via runtime through `v8.setFlagsFromString`.
 * OpenSSL engines cannot be requested at runtime when the Permission
   Model is enabled, affecting the built-in crypto, https, and tls modules.
+* Run-Time Loadable Extensions cannot be loaded when the Permission Model is
+  enabled, affecting the sqlite module.
 * Using existing file descriptors via the `node:fs` module bypasses the
   Permission Model.
 
diff --git a/doc/api/sqlite.md b/doc/api/sqlite.md
@@ -108,6 +108,10 @@ added: v22.5.0
     [double-quoted string literals][]. This is not recommended but can be
     enabled for compatibility with legacy database schemas.
     **Default:** `false`.
+  * `allowExtension` {boolean} If `true`, the `loadExtension` SQL function
+    and the `loadExtension()` method are enabled.
+    You can call `enableLoadExtension(false)` later to disable this feature.
+    **Default:** `false`.
 
 Constructs a new `DatabaseSync` instance.
 
@@ -120,6 +124,30 @@ added: v22.5.0
 Closes the database connection. An exception is thrown if the database is not
 open. This method is a wrapper around [`sqlite3_close_v2()`][].
 
+### `database.loadExtension(path)`
+
+<!-- YAML
+added: REPLACEME
+-->
+
+* `path` {string} The path to the shared library to load.
+
+Loads a shared library into the database connection. This method is a wrapper
+around [`sqlite3_load_extension()`][]. It is required to enable the
+`allowExtension` option when constructing the `DatabaseSync` instance.
+
+### `database.enableLoadExtension(allow)`
+
+<!-- YAML
+added: REPLACEME
+-->
+
+* `allow` {boolean} Whether to allow loading extensions.
+
+Enables or disables the `loadExtension` SQL function, and the `loadExtension()`
+method. When `allowExtension` is `false` when constructing, you cannot enable
+loading extensions for security reasons.
+
 ### `database.exec(sql)`
 
 <!-- YAML
@@ -457,6 +485,7 @@ The following constants are meant for use with [`database.applyChangeset()`](#da
 [`sqlite3_exec()`]: https://www.sqlite.org/c3ref/exec.html
 [`sqlite3_expanded_sql()`]: https://www.sqlite.org/c3ref/expanded_sql.html
 [`sqlite3_last_insert_rowid()`]: https://www.sqlite.org/c3ref/last_insert_rowid.html
+[`sqlite3_load_extension()`]: https://www.sqlite.org/c3ref/load_extension.html
 [`sqlite3_prepare_v2()`]: https://www.sqlite.org/c3ref/prepare.html
 [`sqlite3_sql()`]: https://www.sqlite.org/c3ref/expanded_sql.html
 [`sqlite3changeset_apply()`]: https://www.sqlite.org/session/sqlite3changeset_apply.html
diff --git a/node.gyp b/node.gyp
@@ -1296,6 +1296,26 @@
       ],
     }, # embedtest
 
+    {
+      'target_name': 'sqlite_extension',
+      'type': 'shared_library',
+      'sources': [
+        'test/sqlite/extension.c'
+      ],
+
+      'include_dirs': [
+        'test/sqlite',
+        'deps/sqlite',
+      ],
+
+      'cflags': [
+        '-fPIC',
+        '-Wall',
+        '-Wextra',
+        '-O3',
+      ],
+    }, # sqlitetest
+
     {
       'target_name': 'overlapped-checker',
       'type': 'executable',
diff --git a/src/node_errors.h b/src/node_errors.h
@@ -91,6 +91,7 @@ void OOMErrorHandler(const char* location, const v8::OOMDetails& details);
   V(ERR_INVALID_THIS, TypeError)                                               \
   V(ERR_INVALID_URL, TypeError)                                                \
   V(ERR_INVALID_URL_SCHEME, TypeError)                                         \
+  V(ERR_LOAD_SQLITE_EXTENSION, Error)                                          \
   V(ERR_MEMORY_ALLOCATION_FAILED, Error)                                       \
   V(ERR_MESSAGE_TARGET_CONTEXT_UNAVAILABLE, Error)                             \
   V(ERR_MISSING_ARGS, TypeError)                                               \
@@ -191,6 +192,7 @@ ERRORS_WITH_CODE(V)
   V(ERR_INVALID_STATE, "Invalid state")                                        \
   V(ERR_INVALID_THIS, "Value of \"this\" is the wrong type")                   \
   V(ERR_INVALID_URL_SCHEME, "The URL must be of scheme file:")                 \
+  V(ERR_LOAD_SQLITE_EXTENSION, "Failed to load SQLite extension")              \
   V(ERR_MEMORY_ALLOCATION_FAILED, "Failed to allocate memory")                 \
   V(ERR_OSSL_EVP_INVALID_DIGEST, "Invalid digest used")                        \
   V(ERR_MESSAGE_TARGET_CONTEXT_UNAVAILABLE,                                    \
diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc
@@ -1,4 +1,5 @@
 #include "node_sqlite.h"
+#include <path.h>
 #include "base_object-inl.h"
 #include "debug_utils-inl.h"
 #include "env-inl.h"
@@ -114,10 +115,13 @@ inline void THROW_ERR_SQLITE_ERROR(Isolate* isolate, const char* message) {
 DatabaseSync::DatabaseSync(Environment* env,
                            Local<Object> object,
                            DatabaseOpenConfiguration&& open_config,
-                           bool open)
+                           bool open,
+                           bool allow_load_extension)
     : BaseObject(env, object), open_config_(std::move(open_config)) {
   MakeWeak();
   connection_ = nullptr;
+  allow_load_extension_ = allow_load_extension;
+  enable_load_extension_ = allow_load_extension;
 
   if (open) {
     Open();
@@ -182,6 +186,19 @@ bool DatabaseSync::Open() {
   CHECK_ERROR_OR_THROW(env()->isolate(), connection_, r, SQLITE_OK, false);
   CHECK_EQ(foreign_keys_enabled, open_config_.get_enable_foreign_keys());
 
+  if (allow_load_extension_) {
+    if (env()->permission()->enabled()) [[unlikely]] {
+      THROW_ERR_LOAD_SQLITE_EXTENSION(env(),
+                                      "Cannot load SQLite extensions when the "
+                                      "permission model is enabled.");
+      return false;
+    }
+    const int load_extension_ret = sqlite3_db_config(
+        connection_, SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION, 1, nullptr);
+    CHECK_ERROR_OR_THROW(
+        env()->isolate(), connection_, load_extension_ret, SQLITE_OK, false);
+  }
+
   return true;
 }
 
@@ -227,6 +244,7 @@ void DatabaseSync::New(const FunctionCallbackInfo<Value>& args) {
   DatabaseOpenConfiguration open_config(std::move(location));
 
   bool open = true;
+  bool allow_load_extension = false;
 
   if (args.Length() > 1) {
     if (!args[1]->IsObject()) {
@@ -302,9 +320,28 @@ void DatabaseSync::New(const FunctionCallbackInfo<Value>& args) {
       }
       open_config.set_enable_dqs(enable_dqs_v.As<Boolean>()->Value());
     }
+
+    Local<String> allow_extension_string =
+        FIXED_ONE_BYTE_STRING(env->isolate(), "allowExtension");
+    Local<Value> allow_extension_v;
+    if (!options->Get(env->context(), allow_extension_string)
+             .ToLocal(&allow_extension_v)) {
+      return;
+    }
+
+    if (!allow_extension_v->IsUndefined()) {
+      if (!allow_extension_v->IsBoolean()) {
+        THROW_ERR_INVALID_ARG_TYPE(
+            env->isolate(),
+            "The \"options.allowExtension\" argument must be a boolean.");
+        return;
+      }
+      allow_load_extension = allow_extension_v.As<Boolean>()->Value();
+    }
   }
 
-  new DatabaseSync(env, args.This(), std::move(open_config), open);
+  new DatabaseSync(
+      env, args.This(), std::move(open_config), open, allow_load_extension);
 }
 
 void DatabaseSync::Open(const FunctionCallbackInfo<Value>& args) {
@@ -526,6 +563,70 @@ void DatabaseSync::ApplyChangeset(const FunctionCallbackInfo<Value>& args) {
   args.GetReturnValue().Set(true);
 }
 
+void DatabaseSync::EnableLoadExtension(
+    const FunctionCallbackInfo<Value>& args) {
+  DatabaseSync* db;
+  ASSIGN_OR_RETURN_UNWRAP(&db, args.This());
+  Environment* env = Environment::GetCurrent(args);
+  if (!args[0]->IsBoolean()) {
+    THROW_ERR_INVALID_ARG_TYPE(env->isolate(),
+                               "The \"allow\" argument must be a boolean.");
+    return;
+  }
+
+  const int enable = args[0].As<Boolean>()->Value();
+  auto isolate = env->isolate();
+
+  if (db->allow_load_extension_ == false && enable == true) {
+    THROW_ERR_INVALID_STATE(
+        isolate,
+        "Cannot enable extension loading because it was disabled at database "
+        "creation.");
+    return;
+  }
+  db->enable_load_extension_ = enable;
+  const int load_extension_ret = sqlite3_db_config(
+      db->connection_, SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION, enable, nullptr);
+  CHECK_ERROR_OR_THROW(
+      isolate, db->connection_, load_extension_ret, SQLITE_OK, void());
+}
+
+void DatabaseSync::LoadExtension(const FunctionCallbackInfo<Value>& args) {
+  DatabaseSync* db;
+  ASSIGN_OR_RETURN_UNWRAP(&db, args.This());
+  Environment* env = Environment::GetCurrent(args);
+  THROW_AND_RETURN_ON_BAD_STATE(
+      env, db->connection_ == nullptr, "database is not open");
+  THROW_AND_RETURN_ON_BAD_STATE(
+      env, !db->allow_load_extension_, "extension loading is not allowed");
+  THROW_AND_RETURN_ON_BAD_STATE(
+      env, !db->enable_load_extension_, "extension loading is not allowed");
+
+  if (!args[0]->IsString()) {
+    THROW_ERR_INVALID_ARG_TYPE(env->isolate(),
+                               "The \"path\" argument must be a string.");
+    return;
+  }
+
+  auto isolate = env->isolate();
+
+  BufferValue path(isolate, args[0]);
+  BufferValue entryPoint(isolate, args[1]);
+  CHECK_NOT_NULL(*path);
+  ToNamespacedPath(env, &path);
+  if (*entryPoint == nullptr) {
+    ToNamespacedPath(env, &entryPoint);
+  }
+  THROW_IF_INSUFFICIENT_PERMISSIONS(
+      env, permission::PermissionScope::kFileSystemRead, path.ToStringView());
+  char* errmsg = nullptr;
+  const int r =
+      sqlite3_load_extension(db->connection_, *path, *entryPoint, &errmsg);
+  if (r != SQLITE_OK) {
+    isolate->ThrowException(ERR_LOAD_SQLITE_EXTENSION(isolate, errmsg));
+  }
+}
+
 StatementSync::StatementSync(Environment* env,
                              Local<Object> object,
                              DatabaseSync* db,
@@ -1312,6 +1413,12 @@ static void Initialize(Local<Object> target,
       isolate, db_tmpl, "createSession", DatabaseSync::CreateSession);
   SetProtoMethod(
       isolate, db_tmpl, "applyChangeset", DatabaseSync::ApplyChangeset);
+  SetProtoMethod(isolate,
+                 db_tmpl,
+                 "enableLoadExtension",
+                 DatabaseSync::EnableLoadExtension);
+  SetProtoMethod(
+      isolate, db_tmpl, "loadExtension", DatabaseSync::LoadExtension);
   SetConstructorFunction(context, target, "DatabaseSync", db_tmpl);
   SetConstructorFunction(context,
                          target,
diff --git a/src/node_sqlite.h b/src/node_sqlite.h
@@ -49,7 +49,8 @@ class DatabaseSync : public BaseObject {
   DatabaseSync(Environment* env,
                v8::Local<v8::Object> object,
                DatabaseOpenConfiguration&& open_config,
-               bool open);
+               bool open,
+               bool allow_load_extension);
   void MemoryInfo(MemoryTracker* tracker) const override;
   static void New(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void Open(const v8::FunctionCallbackInfo<v8::Value>& args);
@@ -58,6 +59,9 @@ class DatabaseSync : public BaseObject {
   static void Exec(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void CreateSession(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void ApplyChangeset(const v8::FunctionCallbackInfo<v8::Value>& args);
+  static void EnableLoadExtension(
+      const v8::FunctionCallbackInfo<v8::Value>& args);
+  static void LoadExtension(const v8::FunctionCallbackInfo<v8::Value>& args);
   void FinalizeStatements();
   void UntrackStatement(StatementSync* statement);
   bool IsOpen();
@@ -72,6 +76,8 @@ class DatabaseSync : public BaseObject {
 
   ~DatabaseSync() override;
   DatabaseOpenConfiguration open_config_;
+  bool allow_load_extension_;
+  bool enable_load_extension_;
   sqlite3* connection_;
 
   std::set<sqlite3_session*> sessions_;
diff --git a/test/parallel/test-permission-sqlite-load-extension.js b/test/parallel/test-permission-sqlite-load-extension.js
@@ -0,0 +1,18 @@
+'use strict';
+const common = require('../common');
+const assert = require('node:assert');
+const childProcess = require('child_process');
+
+const code = `const sqlite = require('node:sqlite');
+const db = new sqlite.DatabaseSync(':memory:', { allowExtension: true });
+db.loadExtension('nonexistent');`.replace(/\n/g, ' ');
+
+childProcess.exec(
+  `${process.execPath} --experimental-permission -e "${code}"`,
+  {},
+  common.mustCall((err, _, stderr) => {
+    assert.strictEqual(err.code, 1);
+    assert.match(stderr, /Error: Cannot load SQLite extensions when the permission model is enabled/);
+    assert.match(stderr, /code: 'ERR_LOAD_SQLITE_EXTENSION'/);
+  })
+);
diff --git a/test/sqlite/extension.c b/test/sqlite/extension.c
diff --git a/test/sqlite/test-sqlite-extensions.mjs b/test/sqlite/test-sqlite-extensions.mjs
diff --git a/test/sqlite/testcfg.py b/test/sqlite/testcfg.py
diff --git a/tools/test.py b/tools/test.py
