Skip to content

sqlite: reject connection access from authorizer callbacks #65156

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
TrevorBurnham wants to merge 1 commit into
base: main
Choose a base branch
from
+277 −2
Draft

sqlite: reject connection access from authorizer callbacks #65156

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions doc/api/sqlite.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,14 @@ The callback must return one of the following constants:
* `SQLITE_DENY` - Deny the operation (causes an error).
* `SQLITE_IGNORE` - Ignore the operation (silently skip).

SQLite requires that the authorizer callback not modify the database connection
that invoked it, which includes preparing and stepping statements. Methods on
that connection, its statements, its iterators, and its tag stores therefore
throw an error with code `ERR_INVALID_STATE` while the callback is on the
stack. The callback can also be invoked from within `statement.run()`,
`statement.get()`, and similar methods, because SQLite may re-prepare a
statement during execution after a schema change.

```cjs
const { DatabaseSync, constants } = require('node:sqlite');
const db = new DatabaseSync(':memory:');
Expand Down
45 changes: 45 additions & 0 deletions src/node_sqlite.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,15 @@ inline MaybeLocal<String> Utf8StringMaybeOneByte(Isolate* isolate,
} \
} while (0)

// SQLite requires that an authorizer callback not modify the connection that
// invoked it. Preparing and stepping statements both count as modifying it.
// See https://www.sqlite.org/c3ref/set_authorizer.html.
#define THROW_AND_RETURN_IF_IN_AUTHORIZER(env, db) \
THROW_AND_RETURN_ON_BAD_STATE( \
(env), \
(db)->IsInAuthorizerCallback(), \
"database cannot be accessed from an authorizer callback")

#define SQLITE_VALUE_TO_JS(from, isolate, use_big_int_args, result, ...) \
do { \
switch (sqlite3_##from##_type(__VA_ARGS__)) { \
Expand Down Expand Up @@ -819,6 +828,12 @@ Intercepted DatabaseSyncLimits::LimitsSetter(
return Intercepted::kYes;
}

if (limits->database_->IsInAuthorizerCallback()) {
THROW_ERR_INVALID_STATE(
env, "database cannot be accessed from an authorizer callback");
return Intercepted::kYes;
}

if (!value->IsNumber()) {
THROW_ERR_INVALID_ARG_TYPE(
isolate, "Limit value must be a non-negative integer or Infinity.");
Expand Down Expand Up @@ -1075,6 +1090,7 @@ void DatabaseSync::CreateTagStore(const FunctionCallbackInfo<Value>& args) {
THROW_ERR_INVALID_STATE(env, "database is not open");
return;
}
THROW_AND_RETURN_IF_IN_AUTHORIZER(env, db);
int capacity = 1000;
if (args.Length() > 0 && !args[0]->IsUndefined()) {
if (!args[0]->IsNumber()) {
Expand Down Expand Up @@ -1477,6 +1493,7 @@ void DatabaseSync::Prepare(const FunctionCallbackInfo<Value>& args) {
ASSIGN_OR_RETURN_UNWRAP(&db, args.This());
Environment* env = Environment::GetCurrent(args);
THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open");
THROW_AND_RETURN_IF_IN_AUTHORIZER(env, db);

if (!args[0]->IsString()) {
THROW_ERR_INVALID_ARG_TYPE(env->isolate(),
Expand Down Expand Up @@ -1600,6 +1617,7 @@ void DatabaseSync::Exec(const FunctionCallbackInfo<Value>& args) {
ASSIGN_OR_RETURN_UNWRAP(&db, args.This());
Environment* env = Environment::GetCurrent(args);
THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open");
THROW_AND_RETURN_IF_IN_AUTHORIZER(env, db);

if (!args[0]->IsString()) {
THROW_ERR_INVALID_ARG_TYPE(env->isolate(),
Expand All @@ -1624,6 +1642,7 @@ void DatabaseSync::CustomFunction(const FunctionCallbackInfo<Value>& args) {
ASSIGN_OR_RETURN_UNWRAP(&db, args.This());
Environment* env = Environment::GetCurrent(args);
THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open");
THROW_AND_RETURN_IF_IN_AUTHORIZER(env, db);

if (!args[0]->IsString()) {
THROW_ERR_INVALID_ARG_TYPE(env->isolate(),
Expand Down Expand Up @@ -1852,6 +1871,7 @@ void DatabaseSync::Deserialize(const FunctionCallbackInfo<Value>& args) {
ASSIGN_OR_RETURN_UNWRAP(&db, args.This());
Environment* env = Environment::GetCurrent(args);
THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open");
THROW_AND_RETURN_IF_IN_AUTHORIZER(env, db);

if (!args[0]->IsUint8Array()) {
THROW_ERR_INVALID_ARG_TYPE(env->isolate(),
Expand Down Expand Up @@ -1927,6 +1947,7 @@ void DatabaseSync::AggregateFunction(const FunctionCallbackInfo<Value>& args) {
ASSIGN_OR_RETURN_UNWRAP(&db, args.This());
Environment* env = Environment::GetCurrent(args);
THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open");
THROW_AND_RETURN_IF_IN_AUTHORIZER(env, db);
Utf8Value name(env->isolate(), args[0].As<String>());
Local<Object> options = args[1].As<Object>();
Local<Value> start_v;
Expand Down Expand Up @@ -2138,6 +2159,7 @@ void DatabaseSync::CreateSession(const FunctionCallbackInfo<Value>& args) {
DatabaseSync* db;
ASSIGN_OR_RETURN_UNWRAP(&db, args.This());
THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open");
THROW_AND_RETURN_IF_IN_AUTHORIZER(env, db);

sqlite3_session* pSession;
int r = sqlite3session_create(db->connection_, db_name.c_str(), &pSession);
Expand Down Expand Up @@ -2307,6 +2329,7 @@ void DatabaseSync::ApplyChangeset(const FunctionCallbackInfo<Value>& args) {
ASSIGN_OR_RETURN_UNWRAP(&db, args.This());
Environment* env = Environment::GetCurrent(args);
THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open");
THROW_AND_RETURN_IF_IN_AUTHORIZER(env, db);

if (!args[0]->IsUint8Array()) {
THROW_ERR_INVALID_ARG_TYPE(
Expand Down Expand Up @@ -2441,6 +2464,7 @@ void DatabaseSync::EnableLoadExtension(
ASSIGN_OR_RETURN_UNWRAP(&db, args.This());
Environment* env = Environment::GetCurrent(args);
THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open");
THROW_AND_RETURN_IF_IN_AUTHORIZER(env, db);

Isolate* isolate = env->isolate();
if (!args[0]->IsBoolean()) {
Expand Down Expand Up @@ -2469,6 +2493,7 @@ void DatabaseSync::EnableDefensive(const FunctionCallbackInfo<Value>& args) {
ASSIGN_OR_RETURN_UNWRAP(&db, args.This());
Environment* env = Environment::GetCurrent(args);
THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open");
THROW_AND_RETURN_IF_IN_AUTHORIZER(env, db);

Isolate* isolate = env->isolate();
if (!args[0]->IsBoolean()) {
Expand All @@ -2494,6 +2519,7 @@ void DatabaseSync::LoadExtension(const FunctionCallbackInfo<Value>& args) {
env, !db->allow_load_extension_, "extension loading is not allowed");
THROW_AND_RETURN_ON_BAD_STATE(
env, !db->enable_load_extension_, "extension loading is not allowed");
THROW_AND_RETURN_IF_IN_AUTHORIZER(env, db);

if (!args[0]->IsString()) {
THROW_ERR_INVALID_ARG_TYPE(env->isolate(),
Expand Down Expand Up @@ -2522,6 +2548,7 @@ void DatabaseSync::SetAuthorizer(const FunctionCallbackInfo<Value>& args) {
ASSIGN_OR_RETURN_UNWRAP(&db, args.This());
Environment* env = Environment::GetCurrent(args);
THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open");
THROW_AND_RETURN_IF_IN_AUTHORIZER(env, db);

Isolate* isolate = env->isolate();

Expand Down Expand Up @@ -2558,6 +2585,7 @@ int DatabaseSync::AuthorizerCallback(void* user_data,
const char* param4) {
DatabaseSync* db = static_cast<DatabaseSync*>(user_data);
CallbackDepthGuard guard(db);
AuthorizerDepthGuard authorizer_guard(db);
Environment* env = db->env();
Isolate* isolate = env->isolate();
HandleScope handle_scope(isolate);
Expand Down Expand Up @@ -2671,12 +2699,15 @@ void StatementSync::Close(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
THROW_AND_RETURN_ON_BAD_STATE(
env, stmt->IsFinalized(), "statement has been finalized");
THROW_AND_RETURN_IF_IN_AUTHORIZER(env, stmt->db_.get());
stmt->Close();
}

void StatementSync::Dispose(const FunctionCallbackInfo<Value>& args) {
StatementSync* stmt;
ASSIGN_OR_RETURN_UNWRAP(&stmt, args.This());
Environment* env = Environment::GetCurrent(args);
THROW_AND_RETURN_IF_IN_AUTHORIZER(env, stmt->db_.get());
stmt->Close();
}

Expand Down Expand Up @@ -3121,6 +3152,7 @@ void StatementSync::All(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
THROW_AND_RETURN_ON_BAD_STATE(
env, stmt->IsFinalized(), "statement has been finalized");
THROW_AND_RETURN_IF_IN_AUTHORIZER(env, stmt->db_.get());
Isolate* isolate = env->isolate();
int r = stmt->ResetStatement();
CHECK_ERROR_OR_THROW(isolate, stmt->db_.get(), r, SQLITE_OK, void());
Expand Down Expand Up @@ -3148,6 +3180,7 @@ void StatementSync::Iterate(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
THROW_AND_RETURN_ON_BAD_STATE(
env, stmt->IsFinalized(), "statement has been finalized");
THROW_AND_RETURN_IF_IN_AUTHORIZER(env, stmt->db_.get());
int r = stmt->ResetStatement();
CHECK_ERROR_OR_THROW(env->isolate(), stmt->db_.get(), r, SQLITE_OK, void());

Expand All @@ -3171,6 +3204,7 @@ void StatementSync::Get(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
THROW_AND_RETURN_ON_BAD_STATE(
env, stmt->IsFinalized(), "statement has been finalized");
THROW_AND_RETURN_IF_IN_AUTHORIZER(env, stmt->db_.get());
int r = stmt->ResetStatement();
CHECK_ERROR_OR_THROW(env->isolate(), stmt->db_.get(), r, SQLITE_OK, void());

Expand All @@ -3195,6 +3229,7 @@ void StatementSync::Run(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
THROW_AND_RETURN_ON_BAD_STATE(
env, stmt->IsFinalized(), "statement has been finalized");
THROW_AND_RETURN_IF_IN_AUTHORIZER(env, stmt->db_.get());
int r = stmt->ResetStatement();
CHECK_ERROR_OR_THROW(env->isolate(), stmt->db_.get(), r, SQLITE_OK, void());

Expand Down Expand Up @@ -3477,6 +3512,7 @@ void SQLTagStore::Run(const FunctionCallbackInfo<Value>& args) {

THROW_AND_RETURN_ON_BAD_STATE(
env, !session->database_->IsOpen(), "database is not open");
THROW_AND_RETURN_IF_IN_AUTHORIZER(env, session->database_.get());

BaseObjectPtr<StatementSync> stmt = PrepareStatement(args);

Expand All @@ -3503,6 +3539,7 @@ void SQLTagStore::Iterate(const FunctionCallbackInfo<Value>& args) {

THROW_AND_RETURN_ON_BAD_STATE(
env, !session->database_->IsOpen(), "database is not open");
THROW_AND_RETURN_IF_IN_AUTHORIZER(env, session->database_.get());

BaseObjectPtr<StatementSync> stmt = PrepareStatement(args);

Expand Down Expand Up @@ -3531,6 +3568,7 @@ void SQLTagStore::Get(const FunctionCallbackInfo<Value>& args) {

THROW_AND_RETURN_ON_BAD_STATE(
env, !session->database_->IsOpen(), "database is not open");
THROW_AND_RETURN_IF_IN_AUTHORIZER(env, session->database_.get());

BaseObjectPtr<StatementSync> stmt = PrepareStatement(args);

Expand Down Expand Up @@ -3560,6 +3598,7 @@ void SQLTagStore::All(const FunctionCallbackInfo<Value>& args) {

THROW_AND_RETURN_ON_BAD_STATE(
env, !session->database_->IsOpen(), "database is not open");
THROW_AND_RETURN_IF_IN_AUTHORIZER(env, session->database_.get());

BaseObjectPtr<StatementSync> stmt = PrepareStatement(args);

Expand All @@ -3586,6 +3625,10 @@ void SQLTagStore::All(const FunctionCallbackInfo<Value>& args) {
void SQLTagStore::Clear(const FunctionCallbackInfo<Value>& args) {
SQLTagStore* store;
ASSIGN_OR_RETURN_UNWRAP(&store, args.This());
Environment* env = Environment::GetCurrent(args);
if (store->database_) {
THROW_AND_RETURN_IF_IN_AUTHORIZER(env, store->database_.get());
}
store->sql_tags_.Clear();
}

Expand Down Expand Up @@ -3779,6 +3822,7 @@ void StatementSyncIterator::Next(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
THROW_AND_RETURN_ON_BAD_STATE(
env, iter->stmt_->IsFinalized(), "statement has been finalized");
THROW_AND_RETURN_IF_IN_AUTHORIZER(env, iter->stmt_->db_.get());
Isolate* isolate = env->isolate();

auto iter_template = getLazyIterTemplate(env);
Expand Down Expand Up @@ -3856,6 +3900,7 @@ void StatementSyncIterator::Return(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
THROW_AND_RETURN_ON_BAD_STATE(
env, iter->stmt_->IsFinalized(), "statement has been finalized");
THROW_AND_RETURN_IF_IN_AUTHORIZER(env, iter->stmt_->db_.get());
Isolate* isolate = env->isolate();

sqlite3_reset(iter->stmt_->statement_);
Expand Down
21 changes: 21 additions & 0 deletions src/node_sqlite.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,13 @@ class DatabaseSync : public BaseObject {
void DecrementCallbackDepth() { --callback_depth_; }
bool IsInCallback() const { return callback_depth_ > 0; }

// SQLite forbids an authorizer callback from doing anything that modifies
// the database connection that invoked it, which includes preparing and
// stepping statements. See https://www.sqlite.org/c3ref/set_authorizer.html.
void IncrementAuthorizerDepth() { ++authorizer_depth_; }
void DecrementAuthorizerDepth() { --authorizer_depth_; }
bool IsInAuthorizerCallback() const { return authorizer_depth_ > 0; }

SET_MEMORY_INFO_NAME(DatabaseSync)
SET_SELF_SIZE(DatabaseSync)

Expand All @@ -247,6 +254,7 @@ class DatabaseSync : public BaseObject {
sqlite3* connection_;
bool ignore_next_sqlite_error_;
int callback_depth_ = 0;
int authorizer_depth_ = 0;

std::set<BackupJob*> backups_;
std::unordered_set<Session*> sessions_;
Expand Down Expand Up @@ -426,6 +434,19 @@ class CallbackDepthGuard {
DatabaseSync* db_;
};

class AuthorizerDepthGuard {
public:
explicit AuthorizerDepthGuard(DatabaseSync* db) : db_(db) {
db_->IncrementAuthorizerDepth();
}
~AuthorizerDepthGuard() { db_->DecrementAuthorizerDepth(); }
AuthorizerDepthGuard(const AuthorizerDepthGuard&) = delete;
AuthorizerDepthGuard& operator=(const AuthorizerDepthGuard&) = delete;

private:
DatabaseSync* db_;
};

class UserDefinedFunction {
public:
UserDefinedFunction(Environment* env,
Expand Down
Loading
Loading