Skip to content

sqlite: validate the backup() source database #65831

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

Closed
lazerg wants to merge 1 commit into from
Closed

sqlite: validate the backup() source database #65831

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
1 change: 1 addition & 0 deletions src/env_properties.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,7 @@
V(socketaddress_constructor_template, v8::FunctionTemplate) \
V(space_stats_template, v8::DictionaryTemplate) \
V(sqlite_column_template, v8::DictionaryTemplate) \
V(sqlite_database_constructor_template, v8::FunctionTemplate) \
V(sqlite_limits_template, v8::ObjectTemplate) \
V(sqlite_run_result_template, v8::DictionaryTemplate) \
V(sqlite_statement_sync_constructor_template, v8::FunctionTemplate) \
Expand Down
109 changes: 63 additions & 46 deletions src/node_sqlite.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1840,8 +1840,12 @@ std::optional<std::string> ValidateDatabasePath(Environment* env,
} else if (path->IsObject()) { // When is URL
auto url = path.As<Object>();
Local<Value> href;
if (url->Get(env->context(), env->href_string()).ToLocal(&href) &&
href->IsString()) {
// Let an exception thrown by the href getter propagate instead of
// replacing it with ERR_INVALID_ARG_TYPE.
if (!url->Get(env->context(), env->href_string()).ToLocal(&href)) {
return std::nullopt;
}
if (href->IsString()) {
Utf8Value location_value(env->isolate(), href.As<String>());
auto location = location_value.ToStringView();
if (!has_null_bytes(location)) {
Expand Down Expand Up @@ -3194,9 +3198,13 @@ void Database::CreateSession(const FunctionCallbackInfo<Value>& args) {

void Backup(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
if (args.Length() < 1 || !args[0]->IsObject()) {
THROW_ERR_INVALID_ARG_TYPE(env->isolate(),
"The \"sourceDb\" argument must be an object.");
// Unlike the other unwrap sites in this file, which rely on V8's signature
// check for args.This(), this one takes a value out of args[] and so has to
// check the type itself before unwrapping it.
if (!Database::GetConstructorTemplate(env)->HasInstance(args[0])) {
THROW_ERR_INVALID_ARG_TYPE(
env->isolate(),
"The \"sourceDb\" argument must be an instance of Database.");
return;
}

Expand Down Expand Up @@ -4606,6 +4614,54 @@ static inline void SetSideEffectFreeGetter(
name, getter, Local<FunctionTemplate>(), DontDelete);
}

Local<FunctionTemplate> Database::GetConstructorTemplate(Environment* env) {
Local<FunctionTemplate> tmpl = env->sqlite_database_constructor_template();
if (tmpl.IsEmpty()) {
Isolate* isolate = env->isolate();
tmpl = NewFunctionTemplate(isolate, Database::New);
tmpl->InstanceTemplate()->SetInternalFieldCount(
Database::kInternalFieldCount);
SetProtoMethod(isolate, tmpl, "open", Database::Open);
SetProtoMethod(isolate, tmpl, "close", Database::Close);
SetProtoDispose(isolate, tmpl, Database::Dispose);
SetProtoMethod(isolate, tmpl, "prepare", Database::Prepare);
SetProtoMethod(isolate, tmpl, "exec", Database::Exec);
SetProtoMethod(isolate, tmpl, "function", Database::CustomFunction);
SetProtoMethod(isolate, tmpl, "createTagStore", Database::CreateTagStore);
SetProtoMethodNoSideEffect(isolate, tmpl, "location", Database::Location);
SetProtoMethod(isolate, tmpl, "aggregate", Database::AggregateFunction);
SetProtoMethod(isolate, tmpl, "createSession", Database::CreateSession);
SetProtoMethod(isolate, tmpl, "applyChangeset", Database::ApplyChangeset);
SetProtoMethod(
isolate, tmpl, "enableLoadExtension", Database::EnableLoadExtension);
SetProtoMethod(isolate, tmpl, "enableDefensive", Database::EnableDefensive);
SetProtoMethod(isolate, tmpl, "loadExtension", Database::LoadExtension);
SetProtoMethod(isolate, tmpl, "serialize", Database::Serialize);
SetProtoMethod(isolate, tmpl, "deserialize", Database::Deserialize);
SetProtoMethod(isolate, tmpl, "setAuthorizer", Database::SetAuthorizer);
SetProtoMethod(isolate, tmpl, "createModule", Database::CreateModule);
SetSideEffectFreeGetter(isolate,
tmpl,
FIXED_ONE_BYTE_STRING(isolate, "isOpen"),
Database::IsOpenGetter);
SetSideEffectFreeGetter(isolate,
tmpl,
FIXED_ONE_BYTE_STRING(isolate, "isTransaction"),
Database::IsTransactionGetter);
SetSideEffectFreeGetter(
isolate, tmpl, env->limits_string(), Database::LimitsGetter);
Local<String> sqlite_type_key =
FIXED_ONE_BYTE_STRING(isolate, "sqlite-type");
Local<v8::Symbol> sqlite_type_symbol =
v8::Symbol::For(isolate, sqlite_type_key);
Local<String> database_sync_string =
FIXED_ONE_BYTE_STRING(isolate, "node:sqlite");
tmpl->InstanceTemplate()->Set(sqlite_type_symbol, database_sync_string);
env->set_sqlite_database_constructor_template(tmpl);
}
return tmpl;
}

SQLTagStore::~SQLTagStore() {}

Local<FunctionTemplate> SQLTagStore::GetConstructorTemplate(Environment* env) {
Expand Down Expand Up @@ -5342,51 +5398,12 @@ static void Initialize(Local<Object> target,
}
});
}
Local<FunctionTemplate> db_tmpl = NewFunctionTemplate(isolate, Database::New);
db_tmpl->InstanceTemplate()->SetInternalFieldCount(
Database::kInternalFieldCount);
Local<Object> constants = Object::New(isolate);

DefineConstants(constants);

SetProtoMethod(isolate, db_tmpl, "open", Database::Open);
SetProtoMethod(isolate, db_tmpl, "close", Database::Close);
SetProtoDispose(isolate, db_tmpl, Database::Dispose);
SetProtoMethod(isolate, db_tmpl, "prepare", Database::Prepare);
SetProtoMethod(isolate, db_tmpl, "exec", Database::Exec);
SetProtoMethod(isolate, db_tmpl, "function", Database::CustomFunction);
SetProtoMethod(isolate, db_tmpl, "createTagStore", Database::CreateTagStore);
SetProtoMethodNoSideEffect(isolate, db_tmpl, "location", Database::Location);
SetProtoMethod(isolate, db_tmpl, "aggregate", Database::AggregateFunction);
SetProtoMethod(isolate, db_tmpl, "createSession", Database::CreateSession);
SetProtoMethod(isolate, db_tmpl, "applyChangeset", Database::ApplyChangeset);
SetProtoMethod(
isolate, db_tmpl, "enableLoadExtension", Database::EnableLoadExtension);
SetProtoMethod(
isolate, db_tmpl, "enableDefensive", Database::EnableDefensive);
SetProtoMethod(isolate, db_tmpl, "loadExtension", Database::LoadExtension);
SetProtoMethod(isolate, db_tmpl, "serialize", Database::Serialize);
SetProtoMethod(isolate, db_tmpl, "deserialize", Database::Deserialize);
SetProtoMethod(isolate, db_tmpl, "setAuthorizer", Database::SetAuthorizer);
SetProtoMethod(isolate, db_tmpl, "createModule", Database::CreateModule);
SetSideEffectFreeGetter(isolate,
db_tmpl,
FIXED_ONE_BYTE_STRING(isolate, "isOpen"),
Database::IsOpenGetter);
SetSideEffectFreeGetter(isolate,
db_tmpl,
FIXED_ONE_BYTE_STRING(isolate, "isTransaction"),
Database::IsTransactionGetter);
SetSideEffectFreeGetter(
isolate, db_tmpl, env->limits_string(), Database::LimitsGetter);
Local<String> sqlite_type_key = FIXED_ONE_BYTE_STRING(isolate, "sqlite-type");
Local<v8::Symbol> sqlite_type_symbol =
v8::Symbol::For(isolate, sqlite_type_key);
Local<String> database_sync_string =
FIXED_ONE_BYTE_STRING(isolate, "node:sqlite");
db_tmpl->InstanceTemplate()->Set(sqlite_type_symbol, database_sync_string);

SetConstructorFunction(context, target, "Database", db_tmpl);
SetConstructorFunction(
context, target, "Database", Database::GetConstructorTemplate(env));
SetConstructorFunction(context,
target,
"Statement",
Expand Down
2 changes: 2 additions & 0 deletions src/node_sqlite.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,8 @@ class Database : public BaseObject {
bool open,
bool allow_load_extension);
void MemoryInfo(MemoryTracker* tracker) const override;
static v8::Local<v8::FunctionTemplate> GetConstructorTemplate(
Environment* env);
static void New(const v8::FunctionCallbackInfo<v8::Value>& args);
static void Open(const v8::FunctionCallbackInfo<v8::Value>& args);
static void IsOpenGetter(const v8::FunctionCallbackInfo<v8::Value>& args);
Expand Down
23 changes: 22 additions & 1 deletion test/parallel/test-sqlite-backup.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,31 @@ describe('backup()', () => {
backup();
}, {
code: 'ERR_INVALID_ARG_TYPE',
message: 'The "sourceDb" argument must be an object.'
message: 'The "sourceDb" argument must be an instance of Database.'
});
});

test('throws if the source database is not a Database', (t) => {
const database = makeSourceDb();
const values = [
{},
[],
{ p0: 1, p1: 2, p2: 3, p3: 4 },
{ __proto__: Database.prototype },
database.prepare('SELECT 1'),
database.createSession(),
];

for (const value of values) {
t.assert.throws(() => {
backup(value, nextDb());
}, {
code: 'ERR_INVALID_ARG_TYPE',
message: 'The "sourceDb" argument must be an instance of Database.'
});
}
});

test('throws if path is not a string, URL, or Buffer', (t) => {
const database = makeSourceDb();

Expand Down
9 changes: 9 additions & 0 deletions test/parallel/test-sqlite-database.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,15 @@ suite('Database() constructor', () => {
}, { code: 'ERR_INVALID_URL' });
});

test('propagates an exception thrown by the href getter', (t) => {
t.assert.throws(() => {
new Database({ get href() { throw new RangeError('boom'); } });
}, {
name: 'RangeError',
message: 'boom',
});
});

test('throws if options is provided but is not an object', (t) => {
t.assert.throws(() => {
new Database('foo', null);
Expand Down
Loading