From 62ead87c4caeb63873263d526174528536eb8fc3 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Thu, 23 Apr 2026 20:07:23 +0000 Subject: [PATCH 1/4] sqlite: add permission model checks to DatabaseSync Add permission model enforcement to DatabaseSync::Open(). File-backed databases now check kFileSystemRead (readOnly) or kFileSystemWrite (read-write) before calling sqlite3_open_v2. In-memory databases (:memory:) are exempt. Refs: https://hackerone.com/reports/3686625 --- src/node_sqlite.cc | 13 +++ test/fixtures/permission/sqlite.db | Bin 0 -> 8192 bytes test/parallel/test-permission-sqlite.js | 146 ++++++++++++++++++++++++ 3 files changed, 159 insertions(+) create mode 100644 test/fixtures/permission/sqlite.db create mode 100644 test/parallel/test-permission-sqlite.js diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index b4fbfa88746a..46aa9e551d51 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -1631,6 +1631,19 @@ bool Database::Open() { return false; } + // Permission checks: skip for in-memory databases, enforce FS permissions + // for file-backed databases. + std::string_view db_path = open_config_.location(); + if (db_path != ":memory:" && !db_path.empty()) { + if (open_config_.get_read_only()) { + THROW_IF_INSUFFICIENT_PERMISSIONS( + env(), permission::PermissionScope::kFileSystemRead, db_path, false); + } else { + THROW_IF_INSUFFICIENT_PERMISSIONS( + env(), permission::PermissionScope::kFileSystemWrite, db_path, false); + } + } + // sqlite3_open_v2() assigns a database handle even when it fails. Such a // handle is in a "sick" state and may only be used to retrieve the error // and must then be released with sqlite3_close(). Close and reset the diff --git a/test/fixtures/permission/sqlite.db b/test/fixtures/permission/sqlite.db new file mode 100644 index 0000000000000000000000000000000000000000..2321b9eac82af67ad5f7624720168fcf24f09e13 GIT binary patch literal 8192 zcmeI#u?oU45C-5&6>*Wel&&`_h>I^^l_1qBS~J+OV#GqFpi%HKd>0?cr|4{}?sgUa zBbQ5#gTS|Cy$TYed7qt*vEc!=NeCJ;6Or}JqP#;Cd;NPZOL_kl7WMDXRxu^)2Neqe z0SG_<0uX=z1Rwwb2tWV=5P(2MfvIS?jw5a+-lp2 Date: Sun, 12 Jul 2026 14:48:19 +0000 Subject: [PATCH 2/4] sqlite: enforce permissions through a VFS shim Signed-off-by: Matteo Collina --- node.gyp | 2 + src/node_sqlite.cc | 115 +++- src/node_sqlite.h | 6 + src/node_sqlite_vfs.cc | 753 ++++++++++++++++++++++++ src/node_sqlite_vfs.h | 84 +++ src/permission/fs_permission.cc | 37 ++ src/permission/fs_permission.h | 2 + src/permission/permission.cc | 11 + src/permission/permission.h | 5 + src/permission/permission_base.h | 7 + test/parallel/test-permission-sqlite.js | 292 ++++++--- 11 files changed, 1206 insertions(+), 108 deletions(-) create mode 100644 src/node_sqlite_vfs.cc create mode 100644 src/node_sqlite_vfs.h diff --git a/node.gyp b/node.gyp index a8ec3d5351a5..64807d88dcf5 100644 --- a/node.gyp +++ b/node.gyp @@ -497,8 +497,10 @@ ], 'node_sqlite_sources': [ 'src/node_sqlite.cc', + 'src/node_sqlite_vfs.cc', 'src/node_webstorage.cc', 'src/node_sqlite.h', + 'src/node_sqlite_vfs.h', 'src/node_webstorage.h', ], 'node_ffi_sources': [ diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 46aa9e551d51..116af5b7d9b2 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -619,6 +619,7 @@ class BackupJob : public ThreadPoolWork { std::string source_db, std::string destination_name, std::string dest_db, + std::shared_ptr permission_vfs, int pages, Local progressFunc) : ThreadPoolWork(env, "node_sqlite3.BackupJob"), @@ -627,7 +628,8 @@ class BackupJob : public ThreadPoolWork { pages_(pages), source_db_(std::move(source_db)), destination_name_(std::move(destination_name)), - dest_db_(std::move(dest_db)) { + dest_db_(std::move(dest_db)), + permission_vfs_(std::move(permission_vfs)) { resolver_.Reset(env->isolate(), resolver); progressFunc_.Reset(env->isolate(), progressFunc); } @@ -640,7 +642,8 @@ class BackupJob : public ThreadPoolWork { destination_name_.c_str(), &dest_raw, SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE | SQLITE_OPEN_URI, - nullptr); + permission_vfs_ == nullptr ? nullptr + : permission_vfs_->name().c_str()); dest_.reset(dest_raw); Local resolver = Local::New(env()->isolate(), resolver_); @@ -746,6 +749,8 @@ class BackupJob : public ThreadPoolWork { backup_status_ = sqlite3_errcode(dest_.get()); dest_.reset(); } + + permission_vfs_.reset(); } private: @@ -794,6 +799,7 @@ class BackupJob : public ThreadPoolWork { std::string source_db_; std::string destination_name_; std::string dest_db_; + std::shared_ptr permission_vfs_; }; UserDefinedFunction::UserDefinedFunction(Environment* env, @@ -1625,22 +1631,58 @@ void Database::MemoryInfo(MemoryTracker* tracker) const { "open_config", sizeof(open_config_), "DatabaseOpenConfiguration"); } +namespace { + +bool IsSQLiteMemoryLocation(std::string_view location) { + return location == ":memory:" || + (location.starts_with("file:") && + (SQLitePathForPermission(location) == ":memory:" || + SQLiteUriParameterEquals(location, "mode", "memory"))); +} + +bool IsSQLiteReadOnlyLocation(std::string_view location, bool read_only) { + return read_only || + (location.starts_with("file:") && + SQLiteUriParameterEquals(location, "mode", "ro")); +} + +} // namespace + bool Database::Open() { if (IsOpen()) { THROW_ERR_INVALID_STATE(env(), "database is already open"); return false; } - // Permission checks: skip for in-memory databases, enforce FS permissions - // for file-backed databases. - std::string_view db_path = open_config_.location(); - if (db_path != ":memory:" && !db_path.empty()) { - if (open_config_.get_read_only()) { - THROW_IF_INSUFFICIENT_PERMISSIONS( - env(), permission::PermissionScope::kFileSystemRead, db_path, false); - } else { + // Permission checks: in-memory databases do not access the filesystem. + // SQLite URI modes are handled before the VFS is created so that the + // user-facing error remains Node's permission error. + std::string_view db_location = open_config_.location(); + if (!IsSQLiteMemoryLocation(db_location)) { + const std::string db_path = SQLitePathForPermission(db_location); + const bool read_only = + IsSQLiteReadOnlyLocation(db_location, open_config_.get_read_only()); + THROW_IF_INSUFFICIENT_PERMISSIONS( + env(), + permission::PermissionScope::kFileSystemRead, + db_path, + false); + if (!read_only) { THROW_IF_INSUFFICIENT_PERMISSIONS( - env(), permission::PermissionScope::kFileSystemWrite, db_path, false); + env(), + permission::PermissionScope::kFileSystemWrite, + db_path, + false); + } + } + + if (env()->permission()->enabled()) { + permission_vfs_ = std::make_shared(env()); + if (!permission_vfs_->Register()) { + THROW_ERR_SQLITE_ERROR(env()->isolate(), + "Unable to register the SQLite permission VFS"); + permission_vfs_.reset(); + return false; } } @@ -1662,13 +1704,20 @@ bool Database::Open() { ? SQLITE_OPEN_READONLY : SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE; sqlite3* raw_conn = nullptr; - int r = sqlite3_open_v2(open_config_.location().c_str(), - &raw_conn, - flags | default_flags, - nullptr); + int r = sqlite3_open_v2( + open_config_.location().c_str(), + &raw_conn, + flags | default_flags, + permission_vfs_ == nullptr ? nullptr : permission_vfs_->name().c_str()); connection_.reset(raw_conn); CHECK_ERROR_OR_THROW(env()->isolate(), this, r, SQLITE_OK, false); + if (permission_vfs_ != nullptr) { + r = sqlite3_set_authorizer( + connection_.get(), Database::AuthorizerCallback, this); + CHECK_ERROR_OR_THROW(env()->isolate(), this, r, SQLITE_OK, false); + } + r = sqlite3_db_config(connection_.get(), SQLITE_DBCONFIG_DQS_DML, static_cast(open_config_.get_enable_dqs()), @@ -1837,10 +1886,21 @@ std::optional ValidateDatabasePath(Environment* env, constexpr auto has_null_bytes = [](std::string_view str) { return str.find('\0') != std::string_view::npos; }; + const auto validate_uri_vfs = [&](std::string value) + -> std::optional { + if (env->permission()->enabled() && HasSQLiteVfsUriParameter(value)) { + THROW_ERR_INVALID_ARG_VALUE( + env->isolate(), + "The \"%s\" argument must not specify a SQLite VFS.", + field_name); + return std::nullopt; + } + return value; + }; if (path->IsString()) { Utf8Value location(env->isolate(), path.As()); if (!has_null_bytes(location.ToStringView())) { - return location.ToString(); + return validate_uri_vfs(location.ToString()); } } else if (path->IsUint8Array()) { Local buffer = path.As(); @@ -1849,7 +1909,8 @@ std::optional ValidateDatabasePath(Environment* env, auto data = static_cast(buffer->Buffer()->Data()) + byteOffset; if (std::find(data, data + byteLength, 0) == data + byteLength) { - return std::string(reinterpret_cast(data), byteLength); + return validate_uri_vfs( + std::string(reinterpret_cast(data), byteLength)); } } else if (path->IsObject()) { // When is URL auto url = path.As(); @@ -1869,7 +1930,7 @@ std::optional ValidateDatabasePath(Environment* env, return std::nullopt; } - return location_value.ToString(); + return validate_uri_vfs(location_value.ToString()); } } } @@ -2193,6 +2254,7 @@ void Database::Close(const FunctionCallbackInfo& args) { int r = sqlite3_close_v2(db->connection_.get()); CHECK_ERROR_OR_THROW(env->isolate(), db, r, SQLITE_OK, void()); db->connection_.release(); + db->permission_vfs_.reset(); // Backups can defer SQLite destruction until after the connection is closed. db->user_defined_functions_.clear(); } @@ -3338,6 +3400,7 @@ void Backup(const FunctionCallbackInfo& args) { std::move(source_db), dest_path.value(), std::move(dest_db), + db->PermissionVFS(), rate, progressFunc); db->AddBackup(job); @@ -3624,9 +3687,13 @@ void Database::SetAuthorizer(const FunctionCallbackInfo& args) { Isolate* isolate = env->isolate(); if (args[0]->IsNull()) { - // Clear the authorizer - sqlite3_set_authorizer(db->connection_.get(), nullptr, nullptr); db->object()->SetInternalField(kAuthorizerCallback, Null(isolate)); + if (db->permission_vfs_ == nullptr) { + sqlite3_set_authorizer(db->connection_.get(), nullptr, nullptr); + } else { + sqlite3_set_authorizer( + db->connection_.get(), Database::AuthorizerCallback, db); + } return; } @@ -3655,6 +3722,11 @@ int Database::AuthorizerCallback(void* user_data, const char* param3, const char* param4) { Database* db = static_cast(user_data); + if (db->permission_vfs_ != nullptr && action_code == SQLITE_ATTACH && + param1 != nullptr && HasSQLiteVfsUriParameter(param1)) { + return SQLITE_DENY; + } + CallbackDepthGuard guard(db); AuthorizerDepthGuard authorizer_guard(db); Environment* env = db->env(); @@ -3664,8 +3736,7 @@ int Database::AuthorizerCallback(void* user_data, Local cb = db->object()->GetInternalField(kAuthorizerCallback).template As(); - - CHECK(cb->IsFunction()); + if (!cb->IsFunction()) return SQLITE_OK; Local callback = cb.As(); diff --git a/src/node_sqlite.h b/src/node_sqlite.h index 784f957acaf7..247efde223f8 100644 --- a/src/node_sqlite.h +++ b/src/node_sqlite.h @@ -6,6 +6,7 @@ #include "base_object.h" #include "lru_cache-inl.h" #include "node_mem.h" +#include "node_sqlite_vfs.h" #include "sqlite3.h" #include "util.h" @@ -13,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -278,6 +280,9 @@ class Database : public BaseObject { } sqlite3* Connection(); void SetLimit(int sqlite_limit_id, int value); + std::shared_ptr PermissionVFS() const { + return permission_vfs_; + } // In some situations, such as when using custom functions, it is possible // that SQLite reports an error while JavaScript already has a pending @@ -347,6 +352,7 @@ class Database : public BaseObject { int authorizer_depth_ = 0; int trace_suppression_depth_ = 0; std::vector stepping_statements_; + std::shared_ptr permission_vfs_; // SQLite owns these scalar and aggregate/window function registrations. Its // destroy callbacks untrack replaced functions and failed registrations. diff --git a/src/node_sqlite_vfs.cc b/src/node_sqlite_vfs.cc new file mode 100644 index 000000000000..8b160c307f1a --- /dev/null +++ b/src/node_sqlite_vfs.cc @@ -0,0 +1,753 @@ +#include "node_sqlite_vfs.h" + +#include "env-inl.h" +#include "permission/permission.h" + +#include +#include +#include +#include +#include +#include + +namespace node { +namespace sqlite { + +namespace { + +std::atomic next_vfs_id{0}; + +int HexValue(const char c) { + if (c >= '0' && c <= '9') return c - '0'; + if (c >= 'a' && c <= 'f') return c - 'a' + 10; + if (c >= 'A' && c <= 'F') return c - 'A' + 10; + return -1; +} + +void DecodeUriComponent(std::string_view component, std::string* decoded) { + decoded->clear(); + decoded->reserve(component.size()); + for (size_t i = 0; i < component.size(); ++i) { + if (component[i] == '%' && i + 2 < component.size()) { + const int high = HexValue(component[i + 1]); + const int low = HexValue(component[i + 2]); + if (high >= 0 && low >= 0) { + const char value = static_cast((high << 4) | low); + if (value == '\0') break; + decoded->push_back(value); + i += 2; + continue; + } + } + decoded->push_back(component[i]); + } +} + +bool GetSQLiteUriParameter(std::string_view path, + std::string_view parameter, + std::string* value) { + if (!path.starts_with("file:")) return false; + + const size_t query_start = path.find('?', 5); + const size_t fragment_start = path.find('#', 5); + if (query_start == std::string_view::npos || + (fragment_start != std::string_view::npos && + fragment_start < query_start)) { + return false; + } + + const size_t query_end = fragment_start == std::string_view::npos + ? path.size() + : fragment_start; + size_t parameter_start = query_start + 1; + std::string decoded_key; + std::string decoded_value; + while (parameter_start <= query_end) { + size_t parameter_end = path.find('&', parameter_start); + if (parameter_end == std::string_view::npos || parameter_end > query_end) { + parameter_end = query_end; + } + + const size_t equals = path.find('=', parameter_start); + const size_t key_end = equals == std::string_view::npos || + equals > parameter_end + ? parameter_end + : equals; + DecodeUriComponent(path.substr(parameter_start, key_end - parameter_start), + &decoded_key); + if (decoded_key == parameter) { + if (equals == std::string_view::npos || equals > parameter_end) { + value->clear(); + } else { + DecodeUriComponent(path.substr(equals + 1, parameter_end - equals - 1), + &decoded_value); + *value = decoded_value; + } + return true; + } + + if (parameter_end == query_end) break; + parameter_start = parameter_end + 1; + } + return false; +} + +bool IsSidecarPath(std::string_view path, std::string* database_path) { + constexpr std::string_view suffixes[] = {"-journal", "-wal", "-shm"}; + for (const std::string_view suffix : suffixes) { + if (path.size() > suffix.size() && path.ends_with(suffix)) { + *database_path = path.substr(0, path.size() - suffix.size()); + return true; + } + } + + // SQLite names super-journals as -mj. A + // permission granted for the database also covers its super-journal. + const size_t marker = path.rfind("-mj"); + if (marker != std::string_view::npos && marker > 0 && + path.size() > marker + 3) { + *database_path = path.substr(0, marker); + return true; + } + + return false; +} + +std::string CanonicalPath(SQLitePermissionVFS* vfs, const char* path) { + if (path == nullptr) return {}; + + // SQLite normally passes a full pathname to xOpen, but xAccess and xDelete + // may receive names produced by SQLite itself. Canonicalize those names + // before consulting Node's path permission tree. + std::string result(static_cast(vfs->parent()->mxPathname) + 1, '\0'); + if (vfs->parent()->xFullPathname(vfs->parent(), + path, + static_cast(result.size()), + result.data()) != SQLITE_OK) { + return {}; + } + result.resize(strlen(result.c_str())); + return result; +} + +struct PermissionFile { + sqlite3_file base; + SQLitePermissionVFS* vfs; + bool can_read; + bool can_write; + bool can_write_sidecar; + sqlite3_io_methods methods; +}; + +sqlite3_file* RealFile(PermissionFile* file) { + return reinterpret_cast( + reinterpret_cast(file) + sizeof(PermissionFile)); +} + +PermissionFile* PermissionFileFromBase(sqlite3_file* file) { + return reinterpret_cast(file); +} + +int PermissionClose(sqlite3_file* file); +int PermissionRead(sqlite3_file*, void*, int, sqlite3_int64); +int PermissionWrite(sqlite3_file*, const void*, int, sqlite3_int64); +int PermissionTruncate(sqlite3_file*, sqlite3_int64); +int PermissionSync(sqlite3_file*, int); +int PermissionFileSize(sqlite3_file*, sqlite3_int64*); +int PermissionLock(sqlite3_file*, int); +int PermissionUnlock(sqlite3_file*, int); +int PermissionCheckReservedLock(sqlite3_file*, int*); +int PermissionFileControl(sqlite3_file*, int, void*); +int PermissionSectorSize(sqlite3_file*); +int PermissionDeviceCharacteristics(sqlite3_file*); +int PermissionShmMap(sqlite3_file*, int, int, int, void volatile**); +int PermissionShmLock(sqlite3_file*, int, int, int); +void PermissionShmBarrier(sqlite3_file*); +int PermissionShmUnmap(sqlite3_file*, int); +int PermissionFetch(sqlite3_file*, sqlite3_int64, int, void**); +int PermissionUnfetch(sqlite3_file*, sqlite3_int64, void*); + +const sqlite3_io_methods kPermissionIoMethods = { + 3, + PermissionClose, + PermissionRead, + PermissionWrite, + PermissionTruncate, + PermissionSync, + PermissionFileSize, + PermissionLock, + PermissionUnlock, + PermissionCheckReservedLock, + PermissionFileControl, + PermissionSectorSize, + PermissionDeviceCharacteristics, + PermissionShmMap, + PermissionShmLock, + PermissionShmBarrier, + PermissionShmUnmap, + PermissionFetch, + PermissionUnfetch, +}; + +bool HasReadPermission(const PermissionFile* file) { + return file->can_read; +} + +bool HasWritePermission(const PermissionFile* file) { + return file->can_write; +} + +int PermissionClose(sqlite3_file* file) { + PermissionFile* permission_file = PermissionFileFromBase(file); + sqlite3_file* real = RealFile(permission_file); + const int result = real->pMethods == nullptr + ? SQLITE_OK + : real->pMethods->xClose(real); + file->pMethods = nullptr; + return result; +} + +int PermissionRead(sqlite3_file* file, + void* buffer, + int amount, + sqlite3_int64 offset) { + PermissionFile* permission_file = PermissionFileFromBase(file); + if (!HasReadPermission(permission_file)) return SQLITE_PERM; + sqlite3_file* real = RealFile(permission_file); + return real->pMethods->xRead(real, buffer, amount, offset); +} + +int PermissionWrite(sqlite3_file* file, + const void* buffer, + int amount, + sqlite3_int64 offset) { + PermissionFile* permission_file = PermissionFileFromBase(file); + if (!HasWritePermission(permission_file)) return SQLITE_PERM; + sqlite3_file* real = RealFile(permission_file); + return real->pMethods->xWrite(real, buffer, amount, offset); +} + +int PermissionTruncate(sqlite3_file* file, sqlite3_int64 size) { + PermissionFile* permission_file = PermissionFileFromBase(file); + if (!HasWritePermission(permission_file)) return SQLITE_PERM; + sqlite3_file* real = RealFile(permission_file); + return real->pMethods->xTruncate(real, size); +} + +int PermissionSync(sqlite3_file* file, int flags) { + PermissionFile* permission_file = PermissionFileFromBase(file); + if (!HasWritePermission(permission_file)) return SQLITE_PERM; + sqlite3_file* real = RealFile(permission_file); + return real->pMethods->xSync(real, flags); +} + +int PermissionFileSize(sqlite3_file* file, sqlite3_int64* size) { + PermissionFile* permission_file = PermissionFileFromBase(file); + sqlite3_file* real = RealFile(permission_file); + return real->pMethods->xFileSize(real, size); +} + +int PermissionLock(sqlite3_file* file, int lock) { + PermissionFile* permission_file = PermissionFileFromBase(file); + sqlite3_file* real = RealFile(permission_file); + return real->pMethods->xLock(real, lock); +} + +int PermissionUnlock(sqlite3_file* file, int lock) { + PermissionFile* permission_file = PermissionFileFromBase(file); + sqlite3_file* real = RealFile(permission_file); + return real->pMethods->xUnlock(real, lock); +} + +int PermissionCheckReservedLock(sqlite3_file* file, int* result) { + PermissionFile* permission_file = PermissionFileFromBase(file); + sqlite3_file* real = RealFile(permission_file); + return real->pMethods->xCheckReservedLock(real, result); +} + +bool FileControlRequiresWrite(int operation) { + switch (operation) { + case SQLITE_FCNTL_SIZE_HINT: + case SQLITE_FCNTL_OVERWRITE: + case SQLITE_FCNTL_SYNC: + case SQLITE_FCNTL_COMMIT_PHASETWO: + case SQLITE_FCNTL_BEGIN_ATOMIC_WRITE: + case SQLITE_FCNTL_COMMIT_ATOMIC_WRITE: + case SQLITE_FCNTL_ROLLBACK_ATOMIC_WRITE: + return true; + default: + return false; + } +} + +int PermissionFileControl(sqlite3_file* file, int operation, void* argument) { + PermissionFile* permission_file = PermissionFileFromBase(file); + if (FileControlRequiresWrite(operation) && + !HasWritePermission(permission_file)) { + return SQLITE_PERM; + } + + if (operation == SQLITE_FCNTL_SET_LOCKPROXYFILE && argument != nullptr) { + const std::string_view proxy_path = static_cast(argument); + const bool allowed = + proxy_path == ":auto:" + ? permission_file->vfs->AllowsTemporaryFile(true, true) + : permission_file->vfs->AllowsPath(proxy_path, true, true); + if (!allowed) return SQLITE_PERM; + } + + sqlite3_file* real = RealFile(permission_file); + return real->pMethods->xFileControl(real, operation, argument); +} + +int PermissionSectorSize(sqlite3_file* file) { + PermissionFile* permission_file = PermissionFileFromBase(file); + sqlite3_file* real = RealFile(permission_file); + return real->pMethods->xSectorSize(real); +} + +int PermissionDeviceCharacteristics(sqlite3_file* file) { + PermissionFile* permission_file = PermissionFileFromBase(file); + sqlite3_file* real = RealFile(permission_file); + return real->pMethods->xDeviceCharacteristics(real); +} + +int PermissionShmMap(sqlite3_file* file, + int page, + int page_size, + int extend, + void volatile** result) { + PermissionFile* permission_file = PermissionFileFromBase(file); + if (!HasReadPermission(permission_file)) return SQLITE_PERM; + + // OS VFS implementations can open or create the SHM file read-write even + // when extend is false. + if (!permission_file->can_write_sidecar) return SQLITE_PERM; + + sqlite3_file* real = RealFile(permission_file); + if (real->pMethods->xShmMap == nullptr) return SQLITE_NOTFOUND; + return real->pMethods->xShmMap(real, page, page_size, extend, result); +} + +int PermissionShmLock(sqlite3_file* file, int offset, int amount, int flags) { + PermissionFile* permission_file = PermissionFileFromBase(file); + sqlite3_file* real = RealFile(permission_file); + if (real->pMethods->xShmLock == nullptr) return SQLITE_NOTFOUND; + return real->pMethods->xShmLock(real, offset, amount, flags); +} + +void PermissionShmBarrier(sqlite3_file* file) { + PermissionFile* permission_file = PermissionFileFromBase(file); + sqlite3_file* real = RealFile(permission_file); + if (real->pMethods->xShmBarrier != nullptr) { + real->pMethods->xShmBarrier(real); + } +} + +int PermissionShmUnmap(sqlite3_file* file, int delete_flag) { + PermissionFile* permission_file = PermissionFileFromBase(file); + if (delete_flag && !permission_file->can_write_sidecar) return SQLITE_PERM; + sqlite3_file* real = RealFile(permission_file); + if (real->pMethods->xShmUnmap == nullptr) return SQLITE_NOTFOUND; + return real->pMethods->xShmUnmap(real, delete_flag); +} + +int PermissionFetch(sqlite3_file* file, + sqlite3_int64 offset, + int amount, + void** result) { + PermissionFile* permission_file = PermissionFileFromBase(file); + if (!HasReadPermission(permission_file)) return SQLITE_PERM; + sqlite3_file* real = RealFile(permission_file); + if (real->pMethods->xFetch == nullptr) { + *result = nullptr; + return SQLITE_OK; + } + return real->pMethods->xFetch(real, offset, amount, result); +} + +int PermissionUnfetch(sqlite3_file* file, sqlite3_int64 offset, void* page) { + PermissionFile* permission_file = PermissionFileFromBase(file); + sqlite3_file* real = RealFile(permission_file); + if (real->pMethods->xUnfetch == nullptr) return SQLITE_OK; + return real->pMethods->xUnfetch(real, offset, page); +} + +int PermissionVfsOpen(sqlite3_vfs* vfs, + sqlite3_filename name, + sqlite3_file* file, + int flags, + int* out_flags) { + return static_cast(vfs->pAppData)->Open( + name, file, flags, out_flags); +} + +int PermissionVfsDelete(sqlite3_vfs* vfs, const char* name, int sync_dir) { + return static_cast(vfs->pAppData)->Delete(name, + sync_dir); +} + +int PermissionVfsAccess(sqlite3_vfs* vfs, + const char* name, + int flags, + int* result) { + return static_cast(vfs->pAppData)->Access(name, + flags, + result); +} + +int PermissionVfsFullPathname(sqlite3_vfs* vfs, + const char* name, + int length, + char* output) { + return static_cast(vfs->pAppData)->FullPathname( + name, length, output); +} + +void* PermissionVfsDlOpen(sqlite3_vfs* vfs, const char* name) { + return static_cast(vfs->pAppData)->DlOpen(name); +} + +void PermissionVfsDlError(sqlite3_vfs* vfs, int length, char* message) { + static_cast(vfs->pAppData)->DlError(length, message); +} + +SQLiteDlSym PermissionVfsDlSym(sqlite3_vfs* vfs, + void* handle, + const char* symbol) { + return static_cast(vfs->pAppData)->DlSym(handle, + symbol); +} + +void PermissionVfsDlClose(sqlite3_vfs* vfs, void* handle) { + static_cast(vfs->pAppData)->DlClose(handle); +} + +int PermissionVfsRandomness(sqlite3_vfs* vfs, int length, char* output) { + return static_cast(vfs->pAppData)->Randomness(length, + output); +} + +int PermissionVfsSleep(sqlite3_vfs* vfs, int microseconds) { + return static_cast(vfs->pAppData)->Sleep(microseconds); +} + +int PermissionVfsCurrentTime(sqlite3_vfs* vfs, double* time) { + return static_cast(vfs->pAppData)->CurrentTime(time); +} + +int PermissionVfsGetLastError(sqlite3_vfs* vfs, int length, char* message) { + auto* permission_vfs = + static_cast(vfs->pAppData); + return permission_vfs->GetLastError(length, message); +} + +int PermissionVfsCurrentTimeInt64(sqlite3_vfs* vfs, sqlite3_int64* time) { + return static_cast(vfs->pAppData)->CurrentTimeInt64( + time); +} + +int PermissionVfsSetSystemCall(sqlite3_vfs* vfs, + const char* name, + sqlite3_syscall_ptr callback) { + return static_cast(vfs->pAppData)->SetSystemCall( + name, callback); +} + +sqlite3_syscall_ptr PermissionVfsGetSystemCall(sqlite3_vfs* vfs, + const char* name) { + return static_cast(vfs->pAppData)->GetSystemCall(name); +} + +const char* PermissionVfsNextSystemCall(sqlite3_vfs* vfs, const char* name) { + return static_cast(vfs->pAppData)->NextSystemCall( + name); +} + +} // namespace + +bool HasSQLiteVfsUriParameter(std::string_view path) { + std::string value; + return GetSQLiteUriParameter(path, "vfs", &value); +} + +bool SQLiteUriParameterEquals(std::string_view path, + std::string_view parameter, + std::string_view value) { + std::string actual; + return GetSQLiteUriParameter(path, parameter, &actual) && actual == value; +} + +std::string SQLitePathForPermission(std::string_view path) { + if (!path.starts_with("file:")) return std::string(path); + + const size_t path_end = path.find_first_of("?#", 5); + std::string decoded; + DecodeUriComponent( + path.substr(5, + path_end == std::string_view::npos + ? std::string_view::npos + : path_end - 5), + &decoded); + + if (decoded.starts_with("//")) { + const size_t pathname_start = decoded.find('/', 2); + const std::string_view authority = + pathname_start == std::string::npos + ? std::string_view(decoded).substr(2) + : std::string_view(decoded).substr(2, pathname_start - 2); + if (authority.empty() || authority == "localhost") { + decoded = pathname_start == std::string::npos + ? std::string() + : decoded.substr(pathname_start); + } + } + +#ifdef _WIN32 + if (decoded.size() >= 3 && decoded[0] == '/' && + std::isalpha(static_cast(decoded[1])) && + decoded[2] == ':') { + decoded.erase(0, 1); + } +#endif + + return decoded; +} + +SQLitePermissionVFS::SQLitePermissionVFS(Environment* env) + : parent_(sqlite3_vfs_find(nullptr)), + permission_(env->permission()), + name_("node-permission-" + std::to_string(++next_vfs_id)) { + CHECK_NOT_NULL(parent_); + + vfs_.iVersion = parent_->iVersion; + vfs_.szOsFile = sizeof(PermissionFile) + parent_->szOsFile; + vfs_.mxPathname = parent_->mxPathname; + vfs_.zName = name_.c_str(); + vfs_.pAppData = this; + vfs_.xOpen = PermissionVfsOpen; + vfs_.xDelete = PermissionVfsDelete; + vfs_.xAccess = PermissionVfsAccess; + vfs_.xFullPathname = PermissionVfsFullPathname; + vfs_.xDlOpen = PermissionVfsDlOpen; + vfs_.xDlError = PermissionVfsDlError; + vfs_.xDlSym = PermissionVfsDlSym; + vfs_.xDlClose = PermissionVfsDlClose; + vfs_.xRandomness = PermissionVfsRandomness; + vfs_.xSleep = PermissionVfsSleep; + vfs_.xCurrentTime = PermissionVfsCurrentTime; + vfs_.xGetLastError = PermissionVfsGetLastError; + vfs_.xCurrentTimeInt64 = parent_->xCurrentTimeInt64 == nullptr + ? nullptr + : PermissionVfsCurrentTimeInt64; + vfs_.xSetSystemCall = parent_->xSetSystemCall == nullptr + ? nullptr + : PermissionVfsSetSystemCall; + vfs_.xGetSystemCall = parent_->xGetSystemCall == nullptr + ? nullptr + : PermissionVfsGetSystemCall; + vfs_.xNextSystemCall = parent_->xNextSystemCall == nullptr + ? nullptr + : PermissionVfsNextSystemCall; +} + +SQLitePermissionVFS::~SQLitePermissionVFS() { + Unregister(); +} + +bool SQLitePermissionVFS::Register() { + if (registered_) return true; + registered_ = sqlite3_vfs_register(&vfs_, 0) == SQLITE_OK; + return registered_; +} + +void SQLitePermissionVFS::Unregister() { + if (!registered_) return; + sqlite3_vfs_unregister(&vfs_); + registered_ = false; +} + +bool SQLitePermissionVFS::AllowsPath(std::string_view path, + bool read, + bool write) const { + if (permission_->warning_only()) return true; + + const std::string canonical = CanonicalPath( + const_cast(this), std::string(path).c_str()); + if (canonical.empty()) return false; + + const auto allowed = [&](std::string_view candidate) { + return (!read || permission_->is_granted_no_side_effects( + permission::PermissionScope::kFileSystemRead, + candidate)) && + (!write || permission_->is_granted_no_side_effects( + permission::PermissionScope::kFileSystemWrite, + candidate)); + }; + + if (allowed(canonical)) return true; + + std::string database_path; + return IsSidecarPath(canonical, &database_path) && allowed(database_path); +} + +bool SQLitePermissionVFS::AllowsTemporaryFile(bool read, bool write) const { + if (permission_->warning_only()) return true; + + // SQLite does not provide a pathname for anonymous temporary files. Only + // allow them when the corresponding global permission is granted; a + // path-specific grant cannot safely authorize an unknown temporary path. + return (!read || permission_->is_granted_no_side_effects( + permission::PermissionScope::kFileSystemRead, "")) && + (!write || permission_->is_granted_no_side_effects( + permission::PermissionScope::kFileSystemWrite, "")); +} + +int SQLitePermissionVFS::Open(sqlite3_filename name, + sqlite3_file* file, + int flags, + int* out_flags) { + auto* permission_file = reinterpret_cast(file); + memset(permission_file, 0, vfs_.szOsFile); + permission_file->vfs = this; + + const bool read = + (flags & (SQLITE_OPEN_READONLY | SQLITE_OPEN_READWRITE)) != 0; + const bool write = (flags & SQLITE_OPEN_READWRITE) != 0; + bool can_write_sidecar = write; + bool allowed; + + if (flags & SQLITE_OPEN_MEMORY) { + allowed = true; + } else if (name == nullptr) { + allowed = AllowsTemporaryFile(read, write); + } else { + const char* permission_name = name; + if (flags & (SQLITE_OPEN_MAIN_JOURNAL | SQLITE_OPEN_WAL)) { + permission_name = sqlite3_filename_database(name); + } + allowed = permission_name != nullptr && + !HasSQLiteVfsUriParameter(permission_name) && + AllowsPath(permission_name, read, write); + if (permission_name != nullptr && (flags & SQLITE_OPEN_MAIN_DB)) { + can_write_sidecar = AllowsPath(permission_name, false, true); + } + } + + if (!allowed) return SQLITE_PERM; + + sqlite3_file* real = RealFile(permission_file); + const int result = parent_->xOpen(parent_, name, real, flags, out_flags); + if (real->pMethods == nullptr) { + file->pMethods = nullptr; + return result; + } + + permission_file->can_read = read; + permission_file->can_write = write && + (out_flags == nullptr || + (*out_flags & SQLITE_OPEN_READONLY) == 0); + permission_file->can_write_sidecar = can_write_sidecar; + permission_file->methods = kPermissionIoMethods; + permission_file->methods.iVersion = std::min( + permission_file->methods.iVersion, real->pMethods->iVersion); + if (permission_file->methods.iVersion < 2) { + permission_file->methods.xShmMap = nullptr; + permission_file->methods.xShmLock = nullptr; + permission_file->methods.xShmBarrier = nullptr; + permission_file->methods.xShmUnmap = nullptr; + } + if (permission_file->methods.iVersion < 3) { + permission_file->methods.xFetch = nullptr; + permission_file->methods.xUnfetch = nullptr; + } + file->pMethods = &permission_file->methods; + return result; +} + +int SQLitePermissionVFS::Delete(const char* name, int sync_dir) { + if (name == nullptr || !AllowsPath(name, false, true)) return SQLITE_PERM; + return parent_->xDelete(parent_, name, sync_dir); +} + +int SQLitePermissionVFS::Access(const char* name, int flags, int* result) { + if (name == nullptr) { + *result = 0; + return SQLITE_OK; + } + + const bool read = true; + const bool write = flags == SQLITE_ACCESS_READWRITE; + if (!AllowsPath(name, read, write)) { + *result = 0; + return SQLITE_OK; + } + return parent_->xAccess(parent_, name, flags, result); +} + +int SQLitePermissionVFS::FullPathname(const char* name, + int length, + char* output) { + return parent_->xFullPathname(parent_, name, length, output); +} + +void* SQLitePermissionVFS::DlOpen(const char* name) { + if (!permission_->warning_only()) return nullptr; + return parent_->xDlOpen == nullptr + ? nullptr + : parent_->xDlOpen(parent_, name); +} + +void SQLitePermissionVFS::DlError(int length, char* message) { + if (parent_->xDlError != nullptr) { + parent_->xDlError(parent_, length, message); + } +} + +SQLiteDlSym SQLitePermissionVFS::DlSym(void* handle, const char* symbol) { + return parent_->xDlSym == nullptr + ? nullptr + : parent_->xDlSym(parent_, handle, symbol); +} + +void SQLitePermissionVFS::DlClose(void* handle) { + if (parent_->xDlClose != nullptr) parent_->xDlClose(parent_, handle); +} + +int SQLitePermissionVFS::Randomness(int length, char* output) { + return parent_->xRandomness(parent_, length, output); +} + +int SQLitePermissionVFS::Sleep(int microseconds) { + return parent_->xSleep(parent_, microseconds); +} + +int SQLitePermissionVFS::CurrentTime(double* time) { + return parent_->xCurrentTime(parent_, time); +} + +int SQLitePermissionVFS::GetLastError(int length, char* message) { + return parent_->xGetLastError == nullptr + ? SQLITE_OK + : parent_->xGetLastError(parent_, length, message); +} + +int SQLitePermissionVFS::CurrentTimeInt64(sqlite3_int64* time) { + return parent_->xCurrentTimeInt64(parent_, time); +} + +int SQLitePermissionVFS::SetSystemCall(const char* name, + sqlite3_syscall_ptr callback) { + return parent_->xSetSystemCall(parent_, name, callback); +} + +sqlite3_syscall_ptr SQLitePermissionVFS::GetSystemCall(const char* name) { + return parent_->xGetSystemCall(parent_, name); +} + +const char* SQLitePermissionVFS::NextSystemCall(const char* name) { + return parent_->xNextSystemCall(parent_, name); +} + +} // namespace sqlite +} // namespace node diff --git a/src/node_sqlite_vfs.h b/src/node_sqlite_vfs.h new file mode 100644 index 000000000000..d5d0f759a3de --- /dev/null +++ b/src/node_sqlite_vfs.h @@ -0,0 +1,84 @@ +#ifndef SRC_NODE_SQLITE_VFS_H_ +#define SRC_NODE_SQLITE_VFS_H_ + +#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS + +#include "sqlite3.h" + +#include +#include + +namespace node { + +class Environment; + +namespace permission { +class Permission; +} + +namespace sqlite { + +using SQLiteDlSym = void (*)(void); + +// Returns true if path contains a SQLite URI parameter named "vfs". +// URI-selected VFSes must not be allowed to bypass the permission VFS. +bool HasSQLiteVfsUriParameter(std::string_view path); +// Returns true if a SQLite URI parameter has the specified decoded value. +bool SQLiteUriParameterEquals(std::string_view path, + std::string_view parameter, + std::string_view value); +// Returns the decoded filesystem path portion of a SQLite URI filename. Plain +// filenames are returned unchanged. +std::string SQLitePathForPermission(std::string_view path); + +class SQLitePermissionVFS { + public: + explicit SQLitePermissionVFS(Environment* env); + ~SQLitePermissionVFS(); + + SQLitePermissionVFS(const SQLitePermissionVFS&) = delete; + SQLitePermissionVFS& operator=(const SQLitePermissionVFS&) = delete; + + bool Register(); + void Unregister(); + + const std::string& name() const { return name_; } + bool registered() const { return registered_; } + sqlite3_vfs* parent() const { return parent_; } + + int Open(sqlite3_filename name, + sqlite3_file* file, + int flags, + int* out_flags); + int Delete(const char* name, int sync_dir); + int Access(const char* name, int flags, int* result); + int FullPathname(const char* name, int length, char* output); + void* DlOpen(const char* name); + void DlError(int length, char* message); + SQLiteDlSym DlSym(void* handle, const char* symbol); + void DlClose(void* handle); + int Randomness(int length, char* output); + int Sleep(int microseconds); + int CurrentTime(double* time); + int GetLastError(int length, char* message); + int CurrentTimeInt64(sqlite3_int64* time); + int SetSystemCall(const char* name, sqlite3_syscall_ptr callback); + sqlite3_syscall_ptr GetSystemCall(const char* name); + const char* NextSystemCall(const char* name); + + bool AllowsPath(std::string_view path, bool read, bool write) const; + bool AllowsTemporaryFile(bool read, bool write) const; + + private: + sqlite3_vfs vfs_{}; + sqlite3_vfs* parent_ = nullptr; + permission::Permission* permission_ = nullptr; + std::string name_; + bool registered_ = false; +}; + +} // namespace sqlite +} // namespace node + +#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS +#endif // SRC_NODE_SQLITE_VFS_H_ diff --git a/src/permission/fs_permission.cc b/src/permission/fs_permission.cc index 0baf1291a966..51116b64b4eb 100644 --- a/src/permission/fs_permission.cc +++ b/src/permission/fs_permission.cc @@ -265,6 +265,43 @@ bool FSPermission::is_granted(Environment* env, } } +bool FSPermission::is_granted_no_side_effects(PermissionScope perm, + std::string_view param) const { + // VFS callbacks can execute on SQLite's worker threads. The path passed by + // the VFS has already been made absolute, so do not call PathResolve() or + // publish diagnostics here. + std::string path(param); +#ifdef _WIN32 + if (path.starts_with("\\\\?\\")) { + path.erase(0, 4); + } + if (path.starts_with("UNC\\")) { + path.erase(0, 4); + } + if (path.starts_with("//")) { + path.erase(0, 2); + } +#endif + + RwLock::ScopedReadLock lock(lock_); + switch (perm) { + case PermissionScope::kFileSystem: + return allow_all_in_ && allow_all_out_; + case PermissionScope::kFileSystemRead: + return path.empty() ? allow_all_in_ + : !deny_all_in_ && + (allow_all_in_ || + granted_in_fs_.Lookup(path, true)); + case PermissionScope::kFileSystemWrite: + return path.empty() ? allow_all_out_ + : !deny_all_out_ && + (allow_all_out_ || + granted_out_fs_.Lookup(path, true)); + default: + return false; + } +} + FSPermission::RadixTree::RadixTree() : root_node_(std::make_unique("")) {} FSPermission::RadixTree::~RadixTree() = default; diff --git a/src/permission/fs_permission.h b/src/permission/fs_permission.h index b009b66f6485..329112dbdb43 100644 --- a/src/permission/fs_permission.h +++ b/src/permission/fs_permission.h @@ -24,6 +24,8 @@ class FSPermission final : public PermissionBase { bool is_granted(Environment* env, PermissionScope perm, std::string_view param) const override; + bool is_granted_no_side_effects(PermissionScope perm, + std::string_view param) const override; struct RadixTree { struct Node { diff --git a/src/permission/permission.cc b/src/permission/permission.cc index 53d3a7657440..42fc090398a2 100644 --- a/src/permission/permission.cc +++ b/src/permission/permission.cc @@ -339,6 +339,17 @@ void Permission::AsyncThrowAccessDenied(Environment* env, // superseding error to be thrown. } +bool Permission::is_granted_no_side_effects(PermissionScope permission, + std::string_view resource) const { + if (!enabled_) return true; + + CHECK(permission != PermissionScope::kPermissionsRoot && + permission != PermissionScope::kPermissionsCount); + auto& perm_node = nodes_[static_cast(permission)]; + return perm_node && + perm_node->is_granted_no_side_effects(permission, resource); +} + void Permission::EnablePermissions() { if (!enabled_) { enabled_ = true; diff --git a/src/permission/permission.h b/src/permission/permission.h index 6debc4102b19..0b25bf6e32a6 100644 --- a/src/permission/permission.h +++ b/src/permission/permission.h @@ -112,6 +112,11 @@ class Permission { FORCE_INLINE bool enabled() const { return enabled_; } + // Side-effect-free permission query for native I/O callbacks. The resource + // must already be an absolute path. + bool is_granted_no_side_effects(PermissionScope permission, + std::string_view resource = "") const; + FORCE_INLINE bool warning_only() const { return warning_only_; } static PermissionScope StringToPermission(std::string_view perm); diff --git a/src/permission/permission_base.h b/src/permission/permission_base.h index fb7b2a85d430..d234007419cb 100644 --- a/src/permission/permission_base.h +++ b/src/permission/permission_base.h @@ -72,6 +72,13 @@ class PermissionBase { virtual bool is_granted(Environment* env, PermissionScope perm, std::string_view param) const = 0; + + // Used by native I/O callbacks. Implementations must not access V8, + // publish diagnostics, or resolve relative paths in this method. + virtual bool is_granted_no_side_effects(PermissionScope perm, + std::string_view param) const { + return false; + } }; } // namespace permission diff --git a/test/parallel/test-permission-sqlite.js b/test/parallel/test-permission-sqlite.js index 9dafe5e7abe1..357dbbb0497b 100644 --- a/test/parallel/test-permission-sqlite.js +++ b/test/parallel/test-permission-sqlite.js @@ -4,143 +4,263 @@ const common = require('../common'); common.skipIfSQLiteMissing(); -const { isMainThread } = require('worker_threads'); +const { isMainThread } = require('node:worker_threads'); if (!isMainThread) { common.skip('This test only works on a main thread'); } const assert = require('node:assert'); +const fs = require('node:fs'); const { spawnSync } = require('node:child_process'); const fixtures = require('../common/fixtures'); +const tmpdir = require('../common/tmpdir'); -const spawnOpts = { - encoding: 'utf8', -}; - -const sqliteCode = { - // Open database in read-only mode (should require fs.read) - readOnly: `const sqlite = require('node:sqlite'); - const db = new sqlite.DatabaseSync(process.env.DB_PATH, { readOnly: true }); - db.close();`, - - // Open database in read-write mode (should require fs.write) - readWrite: `const sqlite = require('node:sqlite'); - const db = new sqlite.DatabaseSync(process.env.DB_PATH); - db.close();`, - - // Open database with options.open = false (no FS access expected at construction) - noOpen: `const sqlite = require('node:sqlite'); - const db = new sqlite.DatabaseSync(process.env.DB_PATH, { open: false }); - db.open(); - db.close();`, -}; +tmpdir.refresh(); const dbPath = fixtures.path('permission', 'sqlite.db'); +const deniedPath = tmpdir.resolve('denied.sqlite'); +const backupPath = tmpdir.resolve('backup.sqlite'); +const allowedBackupPath = tmpdir.resolve('allowed-backup.sqlite'); +const sidecarDbPath = tmpdir.resolve('sidecars.sqlite'); -{ - // Opening a database read-only should be blocked when fs.read is not granted - const code = sqliteCode.readOnly.replace(/\n/g, ' '); - const { status, stderr } = spawnSync( +function run(code, permissions, env = {}) { + return spawnSync( process.execPath, [ '--permission', - '--allow-fs-read=/nonexistent', - '--eval', code, + '--allow-env=DB_PATH,ATTACH_PATH,BACKUP_PATH,QUERY', + ...permissions, + '--eval', code.replace(/\n/g, ' '), ], { - ...spawnOpts, + encoding: 'utf8', env: { ...process.env, DB_PATH: dbPath, + ...env, }, }, ); +} + +const readOnly = `const { DatabaseSync } = require('node:sqlite'); + const db = new DatabaseSync(process.env.DB_PATH, { readOnly: true }); + db.close();`; + +const readWrite = `const { DatabaseSync } = require('node:sqlite'); + const db = new DatabaseSync(process.env.DB_PATH); + db.close();`; + +{ + const { status, stderr } = run( + readOnly, + ['--allow-fs-read=' + deniedPath], + ); assert.strictEqual(status, 1, `stderr: ${stderr}`); assert.match(stderr, /Access to this API has been restricted/); } { - // Opening a database read-write should be blocked when fs.write is not granted - const code = sqliteCode.readWrite.replace(/\n/g, ' '); - const { status, stderr } = spawnSync( - process.execPath, - [ - '--permission', - '--allow-fs-write=/nonexistent', - '--eval', code, - ], - { - ...spawnOpts, - env: { - ...process.env, - DB_PATH: dbPath, - }, - }, + // A read-write database requires both read and write permission. + const { status, stderr } = run( + readWrite, + ['--allow-fs-read=' + dbPath], ); assert.strictEqual(status, 1, `stderr: ${stderr}`); assert.match(stderr, /Access to this API has been restricted/); } { - // Opening a database read-write should be allowed when permissions are granted - const code = sqliteCode.readWrite.replace(/\n/g, ' '); - const { status, stderr } = spawnSync( - process.execPath, + const { status, stderr } = run( + readWrite, + ['--allow-fs-write=' + dbPath], + ); + assert.strictEqual(status, 1, `stderr: ${stderr}`); + assert.match(stderr, /Access to this API has been restricted/); +} + +{ + const { status, stderr } = run( + readWrite, [ - '--permission', - '--allow-fs-write=*', - '--allow-fs-read=*', - '--eval', code, + '--allow-fs-read=' + dbPath, + '--allow-fs-write=' + dbPath, ], - { - ...spawnOpts, - env: { - ...process.env, - DB_PATH: dbPath, - }, - }, ); assert.strictEqual(status, 0, `stderr: ${stderr}`); } { - // Opening with open: false should not check permissions at construction, - // but db.open() should still check permissions - const code = sqliteCode.noOpen.replace(/\n/g, ' '); - const { status, stderr } = spawnSync( - process.execPath, + // Delayed opening performs the same checks as opening in the constructor. + const code = `const { DatabaseSync } = require('node:sqlite'); + const db = new DatabaseSync(process.env.DB_PATH, { open: false }); + db.open();`; + const { status, stderr } = run( + code, [ - '--permission', - '--allow-fs-read=/nonexistent', - '--allow-fs-write=/nonexistent', - '--eval', code, + '--allow-fs-read=' + deniedPath, + '--allow-fs-write=' + deniedPath, ], - { - ...spawnOpts, - env: { - ...process.env, - DB_PATH: dbPath, - }, - }, ); assert.strictEqual(status, 1, `stderr: ${stderr}`); assert.match(stderr, /Access to this API has been restricted/); } { - // Memory database should not be restricted - const code = `const sqlite = require('node:sqlite'); - const db = new sqlite.DatabaseSync(':memory:'); - db.close();`.replace(/\n/g, ' '); - const { status, stderr } = spawnSync( - process.execPath, + // In-memory databases never require filesystem permission. + const code = `const { DatabaseSync } = require('node:sqlite'); + const db = new DatabaseSync(':memory:'); + db.close();`; + const { status, stderr } = run(code, []); + assert.strictEqual(status, 0, `stderr: ${stderr}`); +} + +{ + // SQLite URI filenames and URI access modes remain supported. + const code = `const { pathToFileURL } = require('node:url'); + const { DatabaseSync } = require('node:sqlite'); + const url = pathToFileURL(process.env.DB_PATH); + url.searchParams.set('mode', 'ro'); + const db = new DatabaseSync(url); + db.close();`; + const { status, stderr } = run( + code, + ['--allow-fs-read=' + dbPath], + ); + assert.strictEqual(status, 0, `stderr: ${stderr}`); +} + +{ + const code = `const { DatabaseSync } = require('node:sqlite'); + const db = new DatabaseSync('file:shared?mode=memory&cache=shared'); + db.exec('CREATE TABLE data(value)'); + db.close();`; + const { status, stderr } = run(code, []); + assert.strictEqual(status, 0, `stderr: ${stderr}`); +} + +{ + // A URI cannot select an unwrapped VFS while permissions are enabled. + const code = `const { pathToFileURL } = require('node:url'); + const { DatabaseSync } = require('node:sqlite'); + const url = pathToFileURL(process.env.DB_PATH); + url.search = '?' + process.env.QUERY; + new DatabaseSync(url);`; + for (const query of ['%76fs=unix', 'vfs%00ignored=unix']) { + const { status, stderr } = run( + code, + [ + '--allow-fs-read=' + dbPath, + '--allow-fs-write=' + dbPath, + ], + { QUERY: query }, + ); + assert.strictEqual(status, 1, `stderr: ${stderr}`); + assert.match(stderr, /must not specify a SQLite VFS/); + } +} + +{ + // Attached databases use the permission VFS too. + const code = `const { DatabaseSync } = require('node:sqlite'); + const db = new DatabaseSync(process.env.DB_PATH); + db.prepare('ATTACH DATABASE ? AS outside').run(process.env.ATTACH_PATH);`; + const { status, stderr } = run( + code, [ - '--permission', - '--allow-fs-read=/nonexistent', - '--eval', code, + '--allow-fs-read=' + dbPath, + '--allow-fs-write=' + dbPath, ], - spawnOpts, + { ATTACH_PATH: deniedPath }, + ); + assert.strictEqual(status, 1, `stderr: ${stderr}`); + assert.match(stderr, /unable to open database/); +} + +{ + // URI VFS overrides on ATTACH cannot bypass the permission VFS, even if + // the public authorizer callback is cleared. + const code = `const { DatabaseSync } = require('node:sqlite'); + const db = new DatabaseSync(':memory:'); + db.setAuthorizer(null); + db.exec("ATTACH 'file::memory:?%76fs=memdb' AS outside");`; + const { status, stderr } = run(code, []); + assert.strictEqual(status, 1, `stderr: ${stderr}`); + assert.match(stderr, /not authorized/); +} + +{ + // Backup destinations are opened through the permission VFS. + const code = `(async () => { + const { backup, DatabaseSync } = require('node:sqlite'); + const db = new DatabaseSync(process.env.DB_PATH, { readOnly: true }); + try { + await backup(db, process.env.BACKUP_PATH); + process.exitCode = 1; + } catch { + process.exitCode = 0; + } + })();`; + const { status, stderr } = run( + code, + ['--allow-fs-read=' + dbPath], + { BACKUP_PATH: backupPath }, ); assert.strictEqual(status, 0, `stderr: ${stderr}`); + assert.strictEqual(fs.existsSync(backupPath), false); +} + +{ + // Backup I/O on worker threads remains available when permissions allow it. + const code = `(async () => { + const { backup, DatabaseSync } = require('node:sqlite'); + const db = new DatabaseSync(process.env.DB_PATH, { readOnly: true }); + await backup(db, process.env.BACKUP_PATH); + db.close(); + })();`; + const { status, stderr } = run( + code, + [ + '--allow-fs-read=' + dbPath, + '--allow-fs-read=' + allowedBackupPath, + '--allow-fs-write=' + allowedBackupPath, + ], + { BACKUP_PATH: allowedBackupPath }, + ); + assert.strictEqual(status, 0, `stderr: ${stderr}`); + assert.strictEqual(fs.existsSync(allowedBackupPath), true); +} + +{ + // A database grant also covers SQLite-managed journal, WAL, and SHM files. + const code = `const { DatabaseSync } = require('node:sqlite'); + const db = new DatabaseSync(process.env.DB_PATH); + db.exec('PRAGMA journal_mode=WAL; CREATE TABLE data(value)'); + db.close();`; + const { status, stderr } = run( + code, + [ + '--allow-fs-read=' + sidecarDbPath, + '--allow-fs-write=' + sidecarDbPath, + ], + { DB_PATH: sidecarDbPath }, + ); + assert.strictEqual(status, 0, `stderr: ${stderr}`); +} + +{ + // Empty filenames create anonymous temporary files. A path-specific grant + // cannot safely authorize them. + const code = `const { DatabaseSync } = require('node:sqlite'); + new DatabaseSync('');`; + const { status, stderr } = run( + code, + [ + '--allow-fs-read=' + deniedPath, + '--allow-fs-write=' + deniedPath, + ], + ); + assert.strictEqual(status, 1, `stderr: ${stderr}`); + assert.match(stderr, /Access to this API has been restricted/); } From 191231b110247373a9a19cfb869b155f43565600 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Sun, 12 Jul 2026 17:44:09 +0000 Subject: [PATCH 3/4] sqlite: fix permission VFS edge cases Signed-off-by: Matteo Collina --- src/node_sqlite.cc | 3 +- src/node_sqlite_vfs.cc | 145 ++++++++++++++++++++---- src/node_sqlite_vfs.h | 5 + test/parallel/test-permission-sqlite.js | 131 ++++++++++++++++++--- 4 files changed, 251 insertions(+), 33 deletions(-) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 116af5b7d9b2..b46a7208c20a 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -1643,7 +1643,8 @@ bool IsSQLiteMemoryLocation(std::string_view location) { bool IsSQLiteReadOnlyLocation(std::string_view location, bool read_only) { return read_only || (location.starts_with("file:") && - SQLiteUriParameterEquals(location, "mode", "ro")); + (SQLiteUriParameterEquals(location, "mode", "ro") || + SQLiteUriBooleanParameter(location, "immutable"))); } } // namespace diff --git a/src/node_sqlite_vfs.cc b/src/node_sqlite_vfs.cc index 8b160c307f1a..489023f50678 100644 --- a/src/node_sqlite_vfs.cc +++ b/src/node_sqlite_vfs.cc @@ -6,9 +6,11 @@ #include #include #include +#include #include #include #include +#include namespace node { namespace sqlite { @@ -45,6 +47,7 @@ void DecodeUriComponent(std::string_view component, std::string* decoded) { bool GetSQLiteUriParameter(std::string_view path, std::string_view parameter, + bool use_last, std::string* value) { if (!path.starts_with("file:")) return false; @@ -62,6 +65,7 @@ bool GetSQLiteUriParameter(std::string_view path, size_t parameter_start = query_start + 1; std::string decoded_key; std::string decoded_value; + bool found = false; while (parameter_start <= query_end) { size_t parameter_end = path.find('&', parameter_start); if (parameter_end == std::string_view::npos || parameter_end > query_end) { @@ -83,13 +87,25 @@ bool GetSQLiteUriParameter(std::string_view path, &decoded_value); *value = decoded_value; } - return true; + found = true; + if (!use_last) return true; } if (parameter_end == query_end) break; parameter_start = parameter_end + 1; } - return false; + return found; +} + +bool EqualsIgnoreCase(std::string_view left, std::string_view right) { + if (left.size() != right.size()) return false; + for (size_t i = 0; i < left.size(); ++i) { + if (std::tolower(static_cast(left[i])) != + std::tolower(static_cast(right[i]))) { + return false; + } + } + return true; } bool IsSidecarPath(std::string_view path, std::string* database_path) { @@ -113,6 +129,25 @@ bool IsSidecarPath(std::string_view path, std::string* database_path) { return false; } +sqlite3_filename CreateReadOnlyShmFilename(sqlite3_filename name) { + std::vector parameters = {"readonly_shm", "1"}; + for (int index = 0;; ++index) { + const char* key = sqlite3_uri_key(name, index); + if (key == nullptr) break; + if (strcmp(key, "readonly_shm") == 0) continue; + + const char* value = sqlite3_uri_parameter(name, key); + parameters.push_back(key); + parameters.push_back(value == nullptr ? "" : value); + } + + return sqlite3_create_filename(sqlite3_filename_database(name), + sqlite3_filename_journal(name), + sqlite3_filename_wal(name), + static_cast(parameters.size() / 2), + parameters.data()); +} + std::string CanonicalPath(SQLitePermissionVFS* vfs, const char* path) { if (path == nullptr) return {}; @@ -133,9 +168,11 @@ std::string CanonicalPath(SQLitePermissionVFS* vfs, const char* path) { struct PermissionFile { sqlite3_file base; SQLitePermissionVFS* vfs; + sqlite3_filename parent_name; bool can_read; bool can_write; bool can_write_sidecar; + bool read_only_shm; sqlite3_io_methods methods; }; @@ -203,6 +240,8 @@ int PermissionClose(sqlite3_file* file) { const int result = real->pMethods == nullptr ? SQLITE_OK : real->pMethods->xClose(real); + sqlite3_free_filename(permission_file->parent_name); + permission_file->parent_name = nullptr; file->pMethods = nullptr; return result; } @@ -321,8 +360,12 @@ int PermissionShmMap(sqlite3_file* file, if (!HasReadPermission(permission_file)) return SQLITE_PERM; // OS VFS implementations can open or create the SHM file read-write even - // when extend is false. - if (!permission_file->can_write_sidecar) return SQLITE_PERM; + // when extend is false. A read-only clone of the SQLite filename is used + // when the caller has no permission to write sidecars. + if (!permission_file->can_write_sidecar && + !permission_file->read_only_shm) { + return SQLITE_PERM; + } sqlite3_file* real = RealFile(permission_file); if (real->pMethods->xShmMap == nullptr) return SQLITE_NOTFOUND; @@ -468,14 +511,27 @@ const char* PermissionVfsNextSystemCall(sqlite3_vfs* vfs, const char* name) { bool HasSQLiteVfsUriParameter(std::string_view path) { std::string value; - return GetSQLiteUriParameter(path, "vfs", &value); + return GetSQLiteUriParameter(path, "vfs", false, &value); } bool SQLiteUriParameterEquals(std::string_view path, std::string_view parameter, std::string_view value) { std::string actual; - return GetSQLiteUriParameter(path, parameter, &actual) && actual == value; + return GetSQLiteUriParameter(path, parameter, true, &actual) && + actual == value; +} + +bool SQLiteUriBooleanParameter(std::string_view path, + std::string_view parameter) { + std::string value; + if (!GetSQLiteUriParameter(path, parameter, false, &value)) return false; + + if (!value.empty() && std::isdigit(static_cast(value[0]))) { + return std::strtoll(value.c_str(), nullptr, 10) != 0; + } + return EqualsIgnoreCase(value, "on") || EqualsIgnoreCase(value, "yes") || + EqualsIgnoreCase(value, "true"); } std::string SQLitePathForPermission(std::string_view path) { @@ -516,12 +572,15 @@ std::string SQLitePathForPermission(std::string_view path) { SQLitePermissionVFS::SQLitePermissionVFS(Environment* env) : parent_(sqlite3_vfs_find(nullptr)), + memory_vfs_(sqlite3_vfs_find("memdb")), permission_(env->permission()), name_("node-permission-" + std::to_string(++next_vfs_id)) { CHECK_NOT_NULL(parent_); + CHECK_NOT_NULL(memory_vfs_); vfs_.iVersion = parent_->iVersion; - vfs_.szOsFile = sizeof(PermissionFile) + parent_->szOsFile; + vfs_.szOsFile = sizeof(PermissionFile) + + std::max(parent_->szOsFile, memory_vfs_->szOsFile); vfs_.mxPathname = parent_->mxPathname; vfs_.zName = name_.c_str(); vfs_.pAppData = this; @@ -585,18 +644,29 @@ bool SQLitePermissionVFS::AllowsPath(std::string_view path, candidate)); }; - if (allowed(canonical)) return true; + return allowed(canonical); +} + +bool SQLitePermissionVFS::AllowsSidecarPath(std::string_view path, + bool read, + bool write) const { + if (AllowsPath(path, read, write)) return true; + + const std::string canonical = CanonicalPath( + const_cast(this), std::string(path).c_str()); + if (canonical.empty()) return false; std::string database_path; - return IsSidecarPath(canonical, &database_path) && allowed(database_path); + return IsSidecarPath(canonical, &database_path) && + AllowsPath(database_path, read, write); } bool SQLitePermissionVFS::AllowsTemporaryFile(bool read, bool write) const { if (permission_->warning_only()) return true; - // SQLite does not provide a pathname for anonymous temporary files. Only - // allow them when the corresponding global permission is granted; a - // path-specific grant cannot safely authorize an unknown temporary path. + // SQLite does not provide a pathname for anonymous temporary files. Disk + // storage is allowed only with global permissions. Otherwise xOpen uses + // SQLite's memory VFS. return (!read || permission_->is_granted_no_side_effects( permission::PermissionScope::kFileSystemRead, "")) && (!write || permission_->is_granted_no_side_effects( @@ -613,32 +683,66 @@ int SQLitePermissionVFS::Open(sqlite3_filename name, const bool read = (flags & (SQLITE_OPEN_READONLY | SQLITE_OPEN_READWRITE)) != 0; - const bool write = (flags & SQLITE_OPEN_READWRITE) != 0; + const bool immutable = name != nullptr && (flags & SQLITE_OPEN_MAIN_DB) && + sqlite3_uri_boolean(name, "immutable", 0) != 0; + bool write = (flags & SQLITE_OPEN_READWRITE) != 0 && !immutable; + int parent_flags = flags; + sqlite3_vfs* selected_vfs = parent_; bool can_write_sidecar = write; + bool read_only_shm = false; bool allowed; if (flags & SQLITE_OPEN_MEMORY) { + selected_vfs = memory_vfs_; allowed = true; } else if (name == nullptr) { allowed = AllowsTemporaryFile(read, write); + if (!allowed) { + selected_vfs = memory_vfs_; + allowed = true; + } } else { const char* permission_name = name; - if (flags & (SQLITE_OPEN_MAIN_JOURNAL | SQLITE_OPEN_WAL)) { + const bool is_journal = + flags & (SQLITE_OPEN_MAIN_JOURNAL | SQLITE_OPEN_WAL); + if (is_journal) { permission_name = sqlite3_filename_database(name); + sqlite3_file* database_file = sqlite3_database_file_object(name); + if (database_file != nullptr && + !PermissionFileFromBase(database_file)->can_write_sidecar) { + write = false; + parent_flags &= ~(SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE); + parent_flags |= SQLITE_OPEN_READONLY; + } } + allowed = permission_name != nullptr && !HasSQLiteVfsUriParameter(permission_name) && - AllowsPath(permission_name, read, write); - if (permission_name != nullptr && (flags & SQLITE_OPEN_MAIN_DB)) { + ((flags & SQLITE_OPEN_SUPER_JOURNAL) + ? AllowsSidecarPath(permission_name, read, write) + : AllowsPath(permission_name, read, write)); + if (allowed && permission_name != nullptr && + (flags & SQLITE_OPEN_MAIN_DB)) { can_write_sidecar = AllowsPath(permission_name, false, true); + if (!can_write_sidecar) { + permission_file->parent_name = CreateReadOnlyShmFilename(name); + if (permission_file->parent_name == nullptr) return SQLITE_NOMEM; + read_only_shm = true; + } } } if (!allowed) return SQLITE_PERM; + sqlite3_filename parent_name = permission_file->parent_name == nullptr + ? name + : permission_file->parent_name; sqlite3_file* real = RealFile(permission_file); - const int result = parent_->xOpen(parent_, name, real, flags, out_flags); + const int result = selected_vfs->xOpen( + selected_vfs, parent_name, real, parent_flags, out_flags); if (real->pMethods == nullptr) { + sqlite3_free_filename(permission_file->parent_name); + permission_file->parent_name = nullptr; file->pMethods = nullptr; return result; } @@ -648,6 +752,7 @@ int SQLitePermissionVFS::Open(sqlite3_filename name, (out_flags == nullptr || (*out_flags & SQLITE_OPEN_READONLY) == 0); permission_file->can_write_sidecar = can_write_sidecar; + permission_file->read_only_shm = read_only_shm; permission_file->methods = kPermissionIoMethods; permission_file->methods.iVersion = std::min( permission_file->methods.iVersion, real->pMethods->iVersion); @@ -666,7 +771,9 @@ int SQLitePermissionVFS::Open(sqlite3_filename name, } int SQLitePermissionVFS::Delete(const char* name, int sync_dir) { - if (name == nullptr || !AllowsPath(name, false, true)) return SQLITE_PERM; + if (name == nullptr || !AllowsSidecarPath(name, false, true)) { + return SQLITE_PERM; + } return parent_->xDelete(parent_, name, sync_dir); } @@ -678,7 +785,7 @@ int SQLitePermissionVFS::Access(const char* name, int flags, int* result) { const bool read = true; const bool write = flags == SQLITE_ACCESS_READWRITE; - if (!AllowsPath(name, read, write)) { + if (!AllowsSidecarPath(name, read, write)) { *result = 0; return SQLITE_OK; } diff --git a/src/node_sqlite_vfs.h b/src/node_sqlite_vfs.h index d5d0f759a3de..559d2487a0ac 100644 --- a/src/node_sqlite_vfs.h +++ b/src/node_sqlite_vfs.h @@ -27,6 +27,9 @@ bool HasSQLiteVfsUriParameter(std::string_view path); bool SQLiteUriParameterEquals(std::string_view path, std::string_view parameter, std::string_view value); +// Returns true if the first matching SQLite URI parameter is a true boolean. +bool SQLiteUriBooleanParameter(std::string_view path, + std::string_view parameter); // Returns the decoded filesystem path portion of a SQLite URI filename. Plain // filenames are returned unchanged. std::string SQLitePathForPermission(std::string_view path); @@ -67,11 +70,13 @@ class SQLitePermissionVFS { const char* NextSystemCall(const char* name); bool AllowsPath(std::string_view path, bool read, bool write) const; + bool AllowsSidecarPath(std::string_view path, bool read, bool write) const; bool AllowsTemporaryFile(bool read, bool write) const; private: sqlite3_vfs vfs_{}; sqlite3_vfs* parent_ = nullptr; + sqlite3_vfs* memory_vfs_ = nullptr; permission::Permission* permission_ = nullptr; std::string name_; bool registered_ = false; diff --git a/test/parallel/test-permission-sqlite.js b/test/parallel/test-permission-sqlite.js index 357dbbb0497b..e4e36c4ed542 100644 --- a/test/parallel/test-permission-sqlite.js +++ b/test/parallel/test-permission-sqlite.js @@ -13,6 +13,7 @@ if (!isMainThread) { const assert = require('node:assert'); const fs = require('node:fs'); const { spawnSync } = require('node:child_process'); +const { DatabaseSync } = require('node:sqlite'); const fixtures = require('../common/fixtures'); const tmpdir = require('../common/tmpdir'); @@ -23,6 +24,10 @@ const deniedPath = tmpdir.resolve('denied.sqlite'); const backupPath = tmpdir.resolve('backup.sqlite'); const allowedBackupPath = tmpdir.resolve('allowed-backup.sqlite'); const sidecarDbPath = tmpdir.resolve('sidecars.sqlite'); +const walDbPath = tmpdir.resolve('read-only-wal.sqlite'); +const suffixDbPath = dbPath + '-wal'; + +fs.rmSync(suffixDbPath, { force: true }); function run(code, permissions, env = {}) { return spawnSync( @@ -140,6 +145,30 @@ const readWrite = `const { DatabaseSync } = require('node:sqlite'); assert.strictEqual(status, 0, `stderr: ${stderr}`); } +{ + // SQLite processes repeated mode parameters in order. + const code = `const { DatabaseSync } = require('node:sqlite'); + const db = new DatabaseSync('file:unused?mode=ro&mode=memory'); + db.close();`; + const { status, stderr } = run(code, []); + assert.strictEqual(status, 0, `stderr: ${stderr}`); +} + +{ + // Immutable URI databases are read-only and do not require write access. + const code = `const { pathToFileURL } = require('node:url'); + const { DatabaseSync } = require('node:sqlite'); + const url = pathToFileURL(process.env.DB_PATH); + url.searchParams.set('immutable', '1'); + const db = new DatabaseSync(url); + db.close();`; + const { status, stderr } = run( + code, + ['--allow-fs-read=' + dbPath], + ); + assert.strictEqual(status, 0, `stderr: ${stderr}`); +} + { // A URI cannot select an unwrapped VFS while permissions are enabled. const code = `const { pathToFileURL } = require('node:url'); @@ -162,20 +191,24 @@ const readWrite = `const { DatabaseSync } = require('node:sqlite'); } { - // Attached databases use the permission VFS too. + // Attached databases use the permission VFS too. A filename that resembles + // a sidecar does not inherit permission when opened as a main database. const code = `const { DatabaseSync } = require('node:sqlite'); const db = new DatabaseSync(process.env.DB_PATH); db.prepare('ATTACH DATABASE ? AS outside').run(process.env.ATTACH_PATH);`; - const { status, stderr } = run( - code, - [ - '--allow-fs-read=' + dbPath, - '--allow-fs-write=' + dbPath, - ], - { ATTACH_PATH: deniedPath }, - ); - assert.strictEqual(status, 1, `stderr: ${stderr}`); - assert.match(stderr, /unable to open database/); + for (const attachPath of [deniedPath, suffixDbPath]) { + const { status, stderr } = run( + code, + [ + '--allow-fs-read=' + dbPath, + '--allow-fs-write=' + dbPath, + ], + { ATTACH_PATH: attachPath }, + ); + assert.strictEqual(status, 1, `stderr: ${stderr}`); + assert.match(stderr, /unable to open database/); + } + assert.strictEqual(fs.existsSync(suffixDbPath), false); } { @@ -211,6 +244,30 @@ const readWrite = `const { DatabaseSync } = require('node:sqlite'); assert.strictEqual(fs.existsSync(backupPath), false); } +{ + // Backup destinations that resemble sidecars still require exact access. + const code = `(async () => { + const { backup, DatabaseSync } = require('node:sqlite'); + const db = new DatabaseSync(process.env.DB_PATH, { readOnly: true }); + try { + await backup(db, process.env.BACKUP_PATH); + process.exitCode = 1; + } catch { + process.exitCode = 0; + } + })();`; + const { status, stderr } = run( + code, + [ + '--allow-fs-read=' + dbPath, + '--allow-fs-write=' + dbPath, + ], + { BACKUP_PATH: suffixDbPath }, + ); + assert.strictEqual(status, 0, `stderr: ${stderr}`); + assert.strictEqual(fs.existsSync(suffixDbPath), false); +} + { // Backup I/O on worker threads remains available when permissions allow it. const code = `(async () => { @@ -250,8 +307,56 @@ const readWrite = `const { DatabaseSync } = require('node:sqlite'); } { - // Empty filenames create anonymous temporary files. A path-specific grant - // cannot safely authorize them. + // A read-only connection can consume an existing WAL without write access. + const writer = new DatabaseSync(walDbPath); + writer.exec(` + PRAGMA journal_mode=WAL; + PRAGMA wal_autocheckpoint=0; + CREATE TABLE data(value); + INSERT INTO data VALUES (1); + `); + + const code = `const { DatabaseSync } = require('node:sqlite'); + const db = new DatabaseSync(process.env.DB_PATH, { readOnly: true }); + db.prepare('SELECT * FROM data').all(); + db.close();`; + const { status, stderr } = run( + code, + ['--allow-fs-read=' + walDbPath], + { DB_PATH: walDbPath }, + ); + writer.close(); + assert.strictEqual(status, 0, `stderr: ${stderr}`); +} + +{ + // Internal temporary files fall back to the memory VFS when only + // path-specific filesystem permissions are available. + const code = `const { DatabaseSync } = require('node:sqlite'); + const db = new DatabaseSync(process.env.DB_PATH); + db.exec(\`PRAGMA temp_store=FILE; + PRAGMA temp.cache_size=1; + CREATE TEMP TABLE temp_data(value); + WITH RECURSIVE counter(value) AS ( + VALUES(1) UNION ALL + SELECT value + 1 FROM counter WHERE value < 1000 + ) + INSERT INTO temp_data SELECT randomblob(10000) FROM counter;\`); + db.close();`; + const { status, stderr } = run( + code, + [ + '--allow-fs-read=' + sidecarDbPath, + '--allow-fs-write=' + sidecarDbPath, + ], + { DB_PATH: sidecarDbPath }, + ); + assert.strictEqual(status, 0, `stderr: ${stderr}`); +} + +{ + // Empty filenames create anonymous temporary databases. A path-specific + // grant cannot authorize their initial database location. const code = `const { DatabaseSync } = require('node:sqlite'); new DatabaseSync('');`; const { status, stderr } = run( From e630e6b1cbec5426920647acbf1d0409954e2bf3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guilherme=20Ara=C3=BAjo?= Date: Fri, 23 Oct 2026 14:58:12 -0300 Subject: [PATCH 4/4] fixup! sqlite: fix permission VFS edge cases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deny ATTACH under the permission model when SQLite does not pass the filename to the authorizer. SQLite only does so for string literals, so a bound or computed filename could select a URI VFS and bypass the permission VFS. Resolve VFS paths lexically, as the permission model does, instead of following symbolic links, and match SQLite's exact super-journal name format. Share one permission VFS per Environment, release it when open() fails, skip the authorizer guards when no JavaScript callback is set, check each path once, and reuse FSPermission::is_granted() for side-effect-free checks. Assisted-by: Claude Code Signed-off-by: Guilherme Araújo --- src/node_sqlite.cc | 54 ++++--- src/node_sqlite.h | 1 + src/node_sqlite_vfs.cc | 205 ++++++++++++------------ src/node_sqlite_vfs.h | 3 + src/permission/fs_permission.cc | 51 ++---- test/parallel/test-permission-sqlite.js | 42 ++++- 6 files changed, 185 insertions(+), 171 deletions(-) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index b46a7208c20a..32bd34c6306f 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -642,8 +642,7 @@ class BackupJob : public ThreadPoolWork { destination_name_.c_str(), &dest_raw, SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE | SQLITE_OPEN_URI, - permission_vfs_ == nullptr ? nullptr - : permission_vfs_->name().c_str()); + permission_vfs_ == nullptr ? nullptr : permission_vfs_->name().c_str()); dest_.reset(dest_raw); Local resolver = Local::New(env()->isolate(), resolver_); @@ -1641,10 +1640,9 @@ bool IsSQLiteMemoryLocation(std::string_view location) { } bool IsSQLiteReadOnlyLocation(std::string_view location, bool read_only) { - return read_only || - (location.starts_with("file:") && - (SQLiteUriParameterEquals(location, "mode", "ro") || - SQLiteUriBooleanParameter(location, "immutable"))); + return read_only || (location.starts_with("file:") && + (SQLiteUriParameterEquals(location, "mode", "ro") || + SQLiteUriBooleanParameter(location, "immutable"))); } } // namespace @@ -1664,26 +1662,27 @@ bool Database::Open() { const bool read_only = IsSQLiteReadOnlyLocation(db_location, open_config_.get_read_only()); THROW_IF_INSUFFICIENT_PERMISSIONS( - env(), - permission::PermissionScope::kFileSystemRead, - db_path, - false); + env(), permission::PermissionScope::kFileSystemRead, db_path, false); if (!read_only) { THROW_IF_INSUFFICIENT_PERMISSIONS( - env(), - permission::PermissionScope::kFileSystemWrite, - db_path, - false); + env(), permission::PermissionScope::kFileSystemWrite, db_path, false); } } if (env()->permission()->enabled()) { - permission_vfs_ = std::make_shared(env()); - if (!permission_vfs_->Register()) { - THROW_ERR_SQLITE_ERROR(env()->isolate(), - "Unable to register the SQLite permission VFS"); - permission_vfs_.reset(); - return false; + // Connections of the same Environment share one permission VFS. + BindingData* binding = + env()->principal_realm()->GetBindingData(); + if (binding != nullptr) permission_vfs_ = binding->permission_vfs.lock(); + if (permission_vfs_ == nullptr) { + permission_vfs_ = std::make_shared(env()); + if (!permission_vfs_->Register()) { + THROW_ERR_SQLITE_ERROR(env()->isolate(), + "Unable to register the SQLite permission VFS"); + permission_vfs_.reset(); + return false; + } + if (binding != nullptr) binding->permission_vfs = permission_vfs_; } } @@ -1696,6 +1695,7 @@ bool Database::Open() { auto reset_connection_on_failure = OnScopeLeave([&]() { if (!opened) { connection_.reset(); + permission_vfs_.reset(); } }); @@ -1887,8 +1887,8 @@ std::optional ValidateDatabasePath(Environment* env, constexpr auto has_null_bytes = [](std::string_view str) { return str.find('\0') != std::string_view::npos; }; - const auto validate_uri_vfs = [&](std::string value) - -> std::optional { + const auto validate_uri_vfs = + [&](std::string value) -> std::optional { if (env->permission()->enabled() && HasSQLiteVfsUriParameter(value)) { THROW_ERR_INVALID_ARG_VALUE( env->isolate(), @@ -3723,22 +3723,24 @@ int Database::AuthorizerCallback(void* user_data, const char* param3, const char* param4) { Database* db = static_cast(user_data); + // SQLite only passes the ATTACH filename when it is a string literal. A + // bound or computed filename cannot be checked for a URI-selected VFS. if (db->permission_vfs_ != nullptr && action_code == SQLITE_ATTACH && - param1 != nullptr && HasSQLiteVfsUriParameter(param1)) { + (param1 == nullptr || HasSQLiteVfsUriParameter(param1))) { return SQLITE_DENY; } - CallbackDepthGuard guard(db); - AuthorizerDepthGuard authorizer_guard(db); Environment* env = db->env(); Isolate* isolate = env->isolate(); HandleScope handle_scope(isolate); - Local context = env->context(); Local cb = db->object()->GetInternalField(kAuthorizerCallback).template As(); if (!cb->IsFunction()) return SQLITE_OK; + CallbackDepthGuard guard(db); + AuthorizerDepthGuard authorizer_guard(db); + Local context = env->context(); Local callback = cb.As(); LocalVector js_argv(isolate, {Integer::New(isolate, action_code)}); diff --git a/src/node_sqlite.h b/src/node_sqlite.h index 247efde223f8..9c1fcb8629c8 100644 --- a/src/node_sqlite.h +++ b/src/node_sqlite.h @@ -203,6 +203,7 @@ class BindingData : public BaseObject { SET_SELF_SIZE(BindingData) std::unordered_set open_databases; + std::weak_ptr permission_vfs; static void CreatePerContextProperties(v8::Local target, v8::Local unused, diff --git a/src/node_sqlite_vfs.cc b/src/node_sqlite_vfs.cc index 489023f50678..d6167a4cf2c9 100644 --- a/src/node_sqlite_vfs.cc +++ b/src/node_sqlite_vfs.cc @@ -1,6 +1,7 @@ #include "node_sqlite_vfs.h" #include "env-inl.h" +#include "path.h" #include "permission/permission.h" #include @@ -59,9 +60,8 @@ bool GetSQLiteUriParameter(std::string_view path, return false; } - const size_t query_end = fragment_start == std::string_view::npos - ? path.size() - : fragment_start; + const size_t query_end = + fragment_start == std::string_view::npos ? path.size() : fragment_start; size_t parameter_start = query_start + 1; std::string decoded_key; std::string decoded_value; @@ -73,10 +73,10 @@ bool GetSQLiteUriParameter(std::string_view path, } const size_t equals = path.find('=', parameter_start); - const size_t key_end = equals == std::string_view::npos || - equals > parameter_end - ? parameter_end - : equals; + const size_t key_end = + equals == std::string_view::npos || equals > parameter_end + ? parameter_end + : equals; DecodeUriComponent(path.substr(parameter_start, key_end - parameter_start), &decoded_key); if (decoded_key == parameter) { @@ -117,13 +117,20 @@ bool IsSidecarPath(std::string_view path, std::string* database_path) { } } - // SQLite names super-journals as -mj. A - // permission granted for the database also covers its super-journal. - const size_t marker = path.rfind("-mj"); - if (marker != std::string_view::npos && marker > 0 && - path.size() > marker + 3) { - *database_path = path.substr(0, marker); - return true; + // SQLite names super-journals as -mjXXXXXX9XX, where X is a hex + // digit (see pagerIsSuperJrnlName()). A permission granted for the + // database also covers its super-journal. + constexpr size_t kSuperJournalSuffixLength = 12; + if (path.size() > kSuperJournalSuffixLength) { + const std::string_view suffix = + path.substr(path.size() - kSuperJournalSuffixLength); + if (suffix.starts_with("-mj") && suffix[9] == '9' && + std::all_of(suffix.begin() + 3, suffix.end(), [](char c) { + return std::isxdigit(static_cast(c)); + })) { + *database_path = path.substr(0, path.size() - suffix.size()); + return true; + } } return false; @@ -148,17 +155,14 @@ sqlite3_filename CreateReadOnlyShmFilename(sqlite3_filename name) { parameters.data()); } -std::string CanonicalPath(SQLitePermissionVFS* vfs, const char* path) { - if (path == nullptr) return {}; - +std::string CanonicalPath(SQLitePermissionVFS* vfs, std::string_view path) { // SQLite normally passes a full pathname to xOpen, but xAccess and xDelete // may receive names produced by SQLite itself. Canonicalize those names // before consulting Node's path permission tree. std::string result(static_cast(vfs->parent()->mxPathname) + 1, '\0'); - if (vfs->parent()->xFullPathname(vfs->parent(), - path, - static_cast(result.size()), - result.data()) != SQLITE_OK) { + if (vfs->FullPathname(std::string(path).c_str(), + static_cast(result.size()), + result.data()) != SQLITE_OK) { return {}; } result.resize(strlen(result.c_str())); @@ -177,8 +181,8 @@ struct PermissionFile { }; sqlite3_file* RealFile(PermissionFile* file) { - return reinterpret_cast( - reinterpret_cast(file) + sizeof(PermissionFile)); + return reinterpret_cast(reinterpret_cast(file) + + sizeof(PermissionFile)); } PermissionFile* PermissionFileFromBase(sqlite3_file* file) { @@ -226,20 +230,11 @@ const sqlite3_io_methods kPermissionIoMethods = { PermissionUnfetch, }; -bool HasReadPermission(const PermissionFile* file) { - return file->can_read; -} - -bool HasWritePermission(const PermissionFile* file) { - return file->can_write; -} - int PermissionClose(sqlite3_file* file) { PermissionFile* permission_file = PermissionFileFromBase(file); sqlite3_file* real = RealFile(permission_file); - const int result = real->pMethods == nullptr - ? SQLITE_OK - : real->pMethods->xClose(real); + const int result = + real->pMethods == nullptr ? SQLITE_OK : real->pMethods->xClose(real); sqlite3_free_filename(permission_file->parent_name); permission_file->parent_name = nullptr; file->pMethods = nullptr; @@ -251,7 +246,7 @@ int PermissionRead(sqlite3_file* file, int amount, sqlite3_int64 offset) { PermissionFile* permission_file = PermissionFileFromBase(file); - if (!HasReadPermission(permission_file)) return SQLITE_PERM; + if (!permission_file->can_read) return SQLITE_PERM; sqlite3_file* real = RealFile(permission_file); return real->pMethods->xRead(real, buffer, amount, offset); } @@ -261,21 +256,21 @@ int PermissionWrite(sqlite3_file* file, int amount, sqlite3_int64 offset) { PermissionFile* permission_file = PermissionFileFromBase(file); - if (!HasWritePermission(permission_file)) return SQLITE_PERM; + if (!permission_file->can_write) return SQLITE_PERM; sqlite3_file* real = RealFile(permission_file); return real->pMethods->xWrite(real, buffer, amount, offset); } int PermissionTruncate(sqlite3_file* file, sqlite3_int64 size) { PermissionFile* permission_file = PermissionFileFromBase(file); - if (!HasWritePermission(permission_file)) return SQLITE_PERM; + if (!permission_file->can_write) return SQLITE_PERM; sqlite3_file* real = RealFile(permission_file); return real->pMethods->xTruncate(real, size); } int PermissionSync(sqlite3_file* file, int flags) { PermissionFile* permission_file = PermissionFileFromBase(file); - if (!HasWritePermission(permission_file)) return SQLITE_PERM; + if (!permission_file->can_write) return SQLITE_PERM; sqlite3_file* real = RealFile(permission_file); return real->pMethods->xSync(real, flags); } @@ -322,7 +317,7 @@ bool FileControlRequiresWrite(int operation) { int PermissionFileControl(sqlite3_file* file, int operation, void* argument) { PermissionFile* permission_file = PermissionFileFromBase(file); if (FileControlRequiresWrite(operation) && - !HasWritePermission(permission_file)) { + !permission_file->can_write) { return SQLITE_PERM; } @@ -357,13 +352,12 @@ int PermissionShmMap(sqlite3_file* file, int extend, void volatile** result) { PermissionFile* permission_file = PermissionFileFromBase(file); - if (!HasReadPermission(permission_file)) return SQLITE_PERM; + if (!permission_file->can_read) return SQLITE_PERM; // OS VFS implementations can open or create the SHM file read-write even // when extend is false. A read-only clone of the SQLite filename is used // when the caller has no permission to write sidecars. - if (!permission_file->can_write_sidecar && - !permission_file->read_only_shm) { + if (!permission_file->can_write_sidecar && !permission_file->read_only_shm) { return SQLITE_PERM; } @@ -400,7 +394,7 @@ int PermissionFetch(sqlite3_file* file, int amount, void** result) { PermissionFile* permission_file = PermissionFileFromBase(file); - if (!HasReadPermission(permission_file)) return SQLITE_PERM; + if (!permission_file->can_read) return SQLITE_PERM; sqlite3_file* real = RealFile(permission_file); if (real->pMethods->xFetch == nullptr) { *result = nullptr; @@ -421,30 +415,29 @@ int PermissionVfsOpen(sqlite3_vfs* vfs, sqlite3_file* file, int flags, int* out_flags) { - return static_cast(vfs->pAppData)->Open( - name, file, flags, out_flags); + return static_cast(vfs->pAppData) + ->Open(name, file, flags, out_flags); } int PermissionVfsDelete(sqlite3_vfs* vfs, const char* name, int sync_dir) { - return static_cast(vfs->pAppData)->Delete(name, - sync_dir); + return static_cast(vfs->pAppData) + ->Delete(name, sync_dir); } int PermissionVfsAccess(sqlite3_vfs* vfs, const char* name, int flags, int* result) { - return static_cast(vfs->pAppData)->Access(name, - flags, - result); + return static_cast(vfs->pAppData) + ->Access(name, flags, result); } int PermissionVfsFullPathname(sqlite3_vfs* vfs, const char* name, int length, char* output) { - return static_cast(vfs->pAppData)->FullPathname( - name, length, output); + return static_cast(vfs->pAppData) + ->FullPathname(name, length, output); } void* PermissionVfsDlOpen(sqlite3_vfs* vfs, const char* name) { @@ -458,8 +451,8 @@ void PermissionVfsDlError(sqlite3_vfs* vfs, int length, char* message) { SQLiteDlSym PermissionVfsDlSym(sqlite3_vfs* vfs, void* handle, const char* symbol) { - return static_cast(vfs->pAppData)->DlSym(handle, - symbol); + return static_cast(vfs->pAppData) + ->DlSym(handle, symbol); } void PermissionVfsDlClose(sqlite3_vfs* vfs, void* handle) { @@ -467,8 +460,8 @@ void PermissionVfsDlClose(sqlite3_vfs* vfs, void* handle) { } int PermissionVfsRandomness(sqlite3_vfs* vfs, int length, char* output) { - return static_cast(vfs->pAppData)->Randomness(length, - output); + return static_cast(vfs->pAppData) + ->Randomness(length, output); } int PermissionVfsSleep(sqlite3_vfs* vfs, int microseconds) { @@ -480,21 +473,20 @@ int PermissionVfsCurrentTime(sqlite3_vfs* vfs, double* time) { } int PermissionVfsGetLastError(sqlite3_vfs* vfs, int length, char* message) { - auto* permission_vfs = - static_cast(vfs->pAppData); + auto* permission_vfs = static_cast(vfs->pAppData); return permission_vfs->GetLastError(length, message); } int PermissionVfsCurrentTimeInt64(sqlite3_vfs* vfs, sqlite3_int64* time) { - return static_cast(vfs->pAppData)->CurrentTimeInt64( - time); + return static_cast(vfs->pAppData) + ->CurrentTimeInt64(time); } int PermissionVfsSetSystemCall(sqlite3_vfs* vfs, const char* name, sqlite3_syscall_ptr callback) { - return static_cast(vfs->pAppData)->SetSystemCall( - name, callback); + return static_cast(vfs->pAppData) + ->SetSystemCall(name, callback); } sqlite3_syscall_ptr PermissionVfsGetSystemCall(sqlite3_vfs* vfs, @@ -503,8 +495,7 @@ sqlite3_syscall_ptr PermissionVfsGetSystemCall(sqlite3_vfs* vfs, } const char* PermissionVfsNextSystemCall(sqlite3_vfs* vfs, const char* name) { - return static_cast(vfs->pAppData)->NextSystemCall( - name); + return static_cast(vfs->pAppData)->NextSystemCall(name); } } // namespace @@ -541,9 +532,8 @@ std::string SQLitePathForPermission(std::string_view path) { std::string decoded; DecodeUriComponent( path.substr(5, - path_end == std::string_view::npos - ? std::string_view::npos - : path_end - 5), + path_end == std::string_view::npos ? std::string_view::npos + : path_end - 5), &decoded); if (decoded.starts_with("//")) { @@ -573,6 +563,7 @@ std::string SQLitePathForPermission(std::string_view path) { SQLitePermissionVFS::SQLitePermissionVFS(Environment* env) : parent_(sqlite3_vfs_find(nullptr)), memory_vfs_(sqlite3_vfs_find("memdb")), + env_(env), permission_(env->permission()), name_("node-permission-" + std::to_string(++next_vfs_id)) { CHECK_NOT_NULL(parent_); @@ -599,12 +590,10 @@ SQLitePermissionVFS::SQLitePermissionVFS(Environment* env) vfs_.xCurrentTimeInt64 = parent_->xCurrentTimeInt64 == nullptr ? nullptr : PermissionVfsCurrentTimeInt64; - vfs_.xSetSystemCall = parent_->xSetSystemCall == nullptr - ? nullptr - : PermissionVfsSetSystemCall; - vfs_.xGetSystemCall = parent_->xGetSystemCall == nullptr - ? nullptr - : PermissionVfsGetSystemCall; + vfs_.xSetSystemCall = + parent_->xSetSystemCall == nullptr ? nullptr : PermissionVfsSetSystemCall; + vfs_.xGetSystemCall = + parent_->xGetSystemCall == nullptr ? nullptr : PermissionVfsGetSystemCall; vfs_.xNextSystemCall = parent_->xNextSystemCall == nullptr ? nullptr : PermissionVfsNextSystemCall; @@ -626,39 +615,38 @@ void SQLitePermissionVFS::Unregister() { registered_ = false; } +bool SQLitePermissionVFS::AllowsCanonicalPath(std::string_view path, + bool read, + bool write) const { + return (!read || permission_->is_granted_no_side_effects( + permission::PermissionScope::kFileSystemRead, path)) && + (!write || permission_->is_granted_no_side_effects( + permission::PermissionScope::kFileSystemWrite, path)); +} + bool SQLitePermissionVFS::AllowsPath(std::string_view path, bool read, bool write) const { if (permission_->warning_only()) return true; - const std::string canonical = CanonicalPath( - const_cast(this), std::string(path).c_str()); - if (canonical.empty()) return false; - - const auto allowed = [&](std::string_view candidate) { - return (!read || permission_->is_granted_no_side_effects( - permission::PermissionScope::kFileSystemRead, - candidate)) && - (!write || permission_->is_granted_no_side_effects( - permission::PermissionScope::kFileSystemWrite, - candidate)); - }; - - return allowed(canonical); + const std::string canonical = + CanonicalPath(const_cast(this), path); + return !canonical.empty() && AllowsCanonicalPath(canonical, read, write); } bool SQLitePermissionVFS::AllowsSidecarPath(std::string_view path, bool read, bool write) const { - if (AllowsPath(path, read, write)) return true; + if (permission_->warning_only()) return true; - const std::string canonical = CanonicalPath( - const_cast(this), std::string(path).c_str()); + const std::string canonical = + CanonicalPath(const_cast(this), path); if (canonical.empty()) return false; + if (AllowsCanonicalPath(canonical, read, write)) return true; std::string database_path; return IsSidecarPath(canonical, &database_path) && - AllowsPath(database_path, read, write); + AllowsCanonicalPath(database_path, read, write); } bool SQLitePermissionVFS::AllowsTemporaryFile(bool read, bool write) const { @@ -668,9 +656,9 @@ bool SQLitePermissionVFS::AllowsTemporaryFile(bool read, bool write) const { // storage is allowed only with global permissions. Otherwise xOpen uses // SQLite's memory VFS. return (!read || permission_->is_granted_no_side_effects( - permission::PermissionScope::kFileSystemRead, "")) && + permission::PermissionScope::kFileSystemRead, "")) && (!write || permission_->is_granted_no_side_effects( - permission::PermissionScope::kFileSystemWrite, "")); + permission::PermissionScope::kFileSystemWrite, "")); } int SQLitePermissionVFS::Open(sqlite3_filename name, @@ -717,7 +705,6 @@ int SQLitePermissionVFS::Open(sqlite3_filename name, } allowed = permission_name != nullptr && - !HasSQLiteVfsUriParameter(permission_name) && ((flags & SQLITE_OPEN_SUPER_JOURNAL) ? AllowsSidecarPath(permission_name, read, write) : AllowsPath(permission_name, read, write)); @@ -748,14 +735,14 @@ int SQLitePermissionVFS::Open(sqlite3_filename name, } permission_file->can_read = read; - permission_file->can_write = write && - (out_flags == nullptr || - (*out_flags & SQLITE_OPEN_READONLY) == 0); + permission_file->can_write = + write && + (out_flags == nullptr || (*out_flags & SQLITE_OPEN_READONLY) == 0); permission_file->can_write_sidecar = can_write_sidecar; permission_file->read_only_shm = read_only_shm; permission_file->methods = kPermissionIoMethods; - permission_file->methods.iVersion = std::min( - permission_file->methods.iVersion, real->pMethods->iVersion); + permission_file->methods.iVersion = + std::min(permission_file->methods.iVersion, real->pMethods->iVersion); if (permission_file->methods.iVersion < 2) { permission_file->methods.xShmMap = nullptr; permission_file->methods.xShmLock = nullptr; @@ -795,14 +782,23 @@ int SQLitePermissionVFS::Access(const char* name, int flags, int* result) { int SQLitePermissionVFS::FullPathname(const char* name, int length, char* output) { +#ifdef _WIN32 return parent_->xFullPathname(parent_, name, length, output); +#else + // The parent VFS also resolves symbolic links, but the permission model + // checks lexically resolved paths. Resolve the same way so that the path + // SQLite opens is the path that was checked. + const std::string resolved = PathResolve(env_, {name}); + if (resolved.size() >= static_cast(length)) return SQLITE_CANTOPEN; + memcpy(output, resolved.c_str(), resolved.size() + 1); + return SQLITE_OK; +#endif } void* SQLitePermissionVFS::DlOpen(const char* name) { if (!permission_->warning_only()) return nullptr; - return parent_->xDlOpen == nullptr - ? nullptr - : parent_->xDlOpen(parent_, name); + return parent_->xDlOpen == nullptr ? nullptr + : parent_->xDlOpen(parent_, name); } void SQLitePermissionVFS::DlError(int length, char* message) { @@ -812,9 +808,8 @@ void SQLitePermissionVFS::DlError(int length, char* message) { } SQLiteDlSym SQLitePermissionVFS::DlSym(void* handle, const char* symbol) { - return parent_->xDlSym == nullptr - ? nullptr - : parent_->xDlSym(parent_, handle, symbol); + return parent_->xDlSym == nullptr ? nullptr + : parent_->xDlSym(parent_, handle, symbol); } void SQLitePermissionVFS::DlClose(void* handle) { diff --git a/src/node_sqlite_vfs.h b/src/node_sqlite_vfs.h index 559d2487a0ac..d5e6cce3610d 100644 --- a/src/node_sqlite_vfs.h +++ b/src/node_sqlite_vfs.h @@ -74,9 +74,12 @@ class SQLitePermissionVFS { bool AllowsTemporaryFile(bool read, bool write) const; private: + bool AllowsCanonicalPath(std::string_view path, bool read, bool write) const; + sqlite3_vfs vfs_{}; sqlite3_vfs* parent_ = nullptr; sqlite3_vfs* memory_vfs_ = nullptr; + Environment* env_ = nullptr; permission::Permission* permission_ = nullptr; std::string name_; bool registered_ = false; diff --git a/src/permission/fs_permission.cc b/src/permission/fs_permission.cc index 51116b64b4eb..998a3d3a1b97 100644 --- a/src/permission/fs_permission.cc +++ b/src/permission/fs_permission.cc @@ -34,7 +34,10 @@ bool is_tree_granted( node::Environment* env, const node::permission::FSPermission::RadixTree* granted_tree, std::string_view param) { - std::string resolved_param = node::PathResolve(env, {param}); + // Without an Environment (e.g. on SQLite worker threads), the path must + // already be absolute and no diagnostics are published. + std::string resolved_param = + env == nullptr ? std::string(param) : node::PathResolve(env, {param}); #ifdef _WIN32 // Remove leading "\\?\" from UNC path if (resolved_param.starts_with("\\\\?\\")) { @@ -51,11 +54,13 @@ bool is_tree_granted( } #endif auto _is_granted = granted_tree->Lookup(resolved_param, true); - node::Debug(env, - node::DebugCategory::PERMISSION_MODEL, - "Access %d to %s\n", - _is_granted, - param); + if (env != nullptr) { + node::Debug(env, + node::DebugCategory::PERMISSION_MODEL, + "Access %d to %s\n", + _is_granted, + param); + } return _is_granted; } @@ -267,39 +272,7 @@ bool FSPermission::is_granted(Environment* env, bool FSPermission::is_granted_no_side_effects(PermissionScope perm, std::string_view param) const { - // VFS callbacks can execute on SQLite's worker threads. The path passed by - // the VFS has already been made absolute, so do not call PathResolve() or - // publish diagnostics here. - std::string path(param); -#ifdef _WIN32 - if (path.starts_with("\\\\?\\")) { - path.erase(0, 4); - } - if (path.starts_with("UNC\\")) { - path.erase(0, 4); - } - if (path.starts_with("//")) { - path.erase(0, 2); - } -#endif - - RwLock::ScopedReadLock lock(lock_); - switch (perm) { - case PermissionScope::kFileSystem: - return allow_all_in_ && allow_all_out_; - case PermissionScope::kFileSystemRead: - return path.empty() ? allow_all_in_ - : !deny_all_in_ && - (allow_all_in_ || - granted_in_fs_.Lookup(path, true)); - case PermissionScope::kFileSystemWrite: - return path.empty() ? allow_all_out_ - : !deny_all_out_ && - (allow_all_out_ || - granted_out_fs_.Lookup(path, true)); - default: - return false; - } + return is_granted(nullptr, perm, param); } FSPermission::RadixTree::RadixTree() : root_node_(std::make_unique("")) {} diff --git a/test/parallel/test-permission-sqlite.js b/test/parallel/test-permission-sqlite.js index e4e36c4ed542..14da83c728ec 100644 --- a/test/parallel/test-permission-sqlite.js +++ b/test/parallel/test-permission-sqlite.js @@ -195,7 +195,7 @@ const readWrite = `const { DatabaseSync } = require('node:sqlite'); // a sidecar does not inherit permission when opened as a main database. const code = `const { DatabaseSync } = require('node:sqlite'); const db = new DatabaseSync(process.env.DB_PATH); - db.prepare('ATTACH DATABASE ? AS outside').run(process.env.ATTACH_PATH);`; + db.exec("ATTACH DATABASE '" + process.env.ATTACH_PATH + "' AS outside");`; for (const attachPath of [deniedPath, suffixDbPath]) { const { status, stderr } = run( code, @@ -223,6 +223,46 @@ const readWrite = `const { DatabaseSync } = require('node:sqlite'); assert.match(stderr, /not authorized/); } +{ + // SQLite does not expose bound or computed ATTACH filenames to the + // authorizer, so they cannot be checked for a URI VFS and are denied. + for (const attach of [ + "db.prepare('ATTACH ? AS outside').run('file::memory:?vfs=memdb');", + "db.exec(\"ATTACH 'file::memory:?vf' || 's=memdb' AS outside\");", + ]) { + const code = `const { DatabaseSync } = require('node:sqlite'); + const db = new DatabaseSync(':memory:'); + ${attach}`; + const { status, stderr } = run(code, []); + assert.strictEqual(status, 1, `stderr: ${stderr}`); + assert.match(stderr, /not authorized/); + } +} + +if (!common.isWindows) { + // Granted paths that go through a symbolic link are checked the same way + // as the fs module checks them, without resolving the link. + const realDir = tmpdir.resolve('sqlite-real'); + const linkDir = tmpdir.resolve('sqlite-link'); + fs.mkdirSync(realDir, { recursive: true }); + fs.rmSync(linkDir, { force: true }); + fs.symlinkSync(realDir, linkDir, 'dir'); + const linkedDbPath = tmpdir.resolve('sqlite-link', 'linked.sqlite'); + const code = `const { DatabaseSync } = require('node:sqlite'); + const db = new DatabaseSync(process.env.DB_PATH); + db.exec('CREATE TABLE t (x)'); + db.close();`; + const { status, stderr } = run( + code, + [ + '--allow-fs-read=' + linkDir, + '--allow-fs-write=' + linkDir, + ], + { DB_PATH: linkedDbPath }, + ); + assert.strictEqual(status, 0, `stderr: ${stderr}`); +} + { // Backup destinations are opened through the permission VFS. const code = `(async () => {