Skip to content

Commit 31a9780

Browse files
littledivyry
authored andcommitted
fix(ext/node): restrict ATTACH DATABASE statement (#28513)
Disable `ATTACH DATABASE` statement in `node:sqlite` since it is not supervised by Deno's permission system
1 parent 5e264d4 commit 31a9780

File tree

7 files changed

+107
-43
lines changed

7 files changed

+107
-43
lines changed

Cargo.lock

Lines changed: 23 additions & 32 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,10 @@ eszip = "0.83.0"
7070
napi_sym = { version = "0.125.0", path = "./ext/napi/sym" }
7171
test_util = { package = "test_server", path = "./tests/util/server" }
7272

73-
denokv_proto = "0.9.0"
74-
denokv_remote = "0.9.0"
73+
denokv_proto = "0.10.0"
74+
denokv_remote = "0.10.0"
7575
# denokv_sqlite brings in bundled sqlite if we don't disable the default features
76-
denokv_sqlite = { default-features = false, version = "0.9.0" }
76+
denokv_sqlite = { default-features = false, version = "0.10.0" }
7777

7878
# exts
7979
deno_broadcast_channel = { version = "0.189.0", path = "./ext/broadcast_channel" }
@@ -269,7 +269,7 @@ regex = "^1.7.0"
269269
reqwest = { version = "=0.12.5", default-features = false, features = ["rustls-tls", "stream", "gzip", "brotli", "socks", "json", "http2"] } # pinned because of https://github.com/seanmonstar/reqwest/pull/1955
270270
ring = "^0.17.0"
271271
ripemd = "0.1.3"
272-
rusqlite = { version = "0.32.0", features = ["unlock_notify", "bundled", "session"] }
272+
rusqlite = { version = "0.34.0", features = ["unlock_notify", "bundled", "session", "modern_sqlite", "limits"] } # "modern_sqlite": need sqlite >= 3.49.0 for some db configs
273273
rustc-hash = "2.1.1"
274274
rustls = { version = "0.23.11", default-features = false, features = ["logging", "std", "tls12", "ring"] }
275275
rustls-pemfile = "2"

ext/node/Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@ idna.workspace = true
5757
ipnetwork.workspace = true
5858
k256.workspace = true
5959
libc.workspace = true
60-
libsqlite3-sys.workspace = true
6160
libz-sys.workspace = true
6261
md-5 = { workspace = true, features = ["oid"] }
6362
md4.workspace = true

ext/node/ops/sqlite/database.rs

Lines changed: 67 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,18 @@ use deno_core::v8;
1515
use deno_core::GarbageCollected;
1616
use deno_core::OpState;
1717
use deno_permissions::PermissionsContainer;
18+
use rusqlite::ffi as libsqlite3_sys;
19+
use rusqlite::limits::Limit;
1820
use serde::Deserialize;
1921

2022
use super::session::SessionOptions;
2123
use super::Session;
2224
use super::SqliteError;
2325
use super::StatementSync;
2426

27+
const SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION: i32 = 1005;
28+
const SQLITE_DBCONFIG_ENABLE_ATTACH_WRITE: i32 = 1021;
29+
2530
#[derive(Deserialize)]
2631
#[serde(rename_all = "camelCase")]
2732
struct DatabaseSyncOptions {
@@ -61,31 +66,89 @@ pub struct DatabaseSync {
6166

6267
impl GarbageCollected for DatabaseSync {}
6368

69+
fn set_db_config(
70+
conn: &rusqlite::Connection,
71+
config: i32,
72+
value: bool,
73+
) -> bool {
74+
// SAFETY: call to sqlite3_db_config is safe because the connection
75+
// handle is valid and the parameters are correct.
76+
unsafe {
77+
let mut set = 0;
78+
let r = libsqlite3_sys::sqlite3_db_config(
79+
conn.handle(),
80+
config,
81+
value as i32,
82+
&mut set,
83+
);
84+
85+
if r != libsqlite3_sys::SQLITE_OK {
86+
panic!("Failed to set db config");
87+
}
88+
89+
set == value as i32
90+
}
91+
}
92+
6493
fn open_db(
6594
state: &mut OpState,
6695
readonly: bool,
6796
location: &str,
6897
) -> Result<rusqlite::Connection, SqliteError> {
6998
if location == ":memory:" {
70-
return Ok(rusqlite::Connection::open_in_memory()?);
99+
let conn = rusqlite::Connection::open_in_memory()?;
100+
assert!(set_db_config(
101+
&conn,
102+
SQLITE_DBCONFIG_ENABLE_ATTACH_WRITE,
103+
false
104+
));
105+
assert!(set_db_config(
106+
&conn,
107+
SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION,
108+
false
109+
));
110+
111+
conn.set_limit(Limit::SQLITE_LIMIT_ATTACHED, 0)?;
112+
return Ok(conn);
71113
}
72114

73115
state
74116
.borrow::<PermissionsContainer>()
75117
.check_read_with_api_name(location, Some("node:sqlite"))?;
76118

77119
if readonly {
78-
return Ok(rusqlite::Connection::open_with_flags(
120+
let conn = rusqlite::Connection::open_with_flags(
79121
location,
80122
rusqlite::OpenFlags::SQLITE_OPEN_READ_ONLY,
81-
)?);
123+
)?;
124+
assert!(set_db_config(
125+
&conn,
126+
SQLITE_DBCONFIG_ENABLE_ATTACH_WRITE,
127+
false
128+
));
129+
assert!(set_db_config(
130+
&conn,
131+
SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION,
132+
false
133+
));
134+
135+
conn.set_limit(Limit::SQLITE_LIMIT_ATTACHED, 0)?;
136+
return Ok(conn);
82137
}
83138

84139
state
85140
.borrow::<PermissionsContainer>()
86141
.check_write_with_api_name(location, Some("node:sqlite"))?;
87142

88-
Ok(rusqlite::Connection::open(location)?)
143+
let conn = rusqlite::Connection::open(location)?;
144+
assert!(set_db_config(
145+
&conn,
146+
SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION,
147+
false
148+
));
149+
conn.set_limit(Limit::SQLITE_LIMIT_ATTACHED, 0)?;
150+
151+
Ok(conn)
89152
}
90153

91154
// Represents a single connection to a SQLite database.

ext/node/ops/sqlite/session.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use std::rc::Rc;
77

88
use deno_core::op2;
99
use deno_core::GarbageCollected;
10-
use libsqlite3_sys as ffi;
10+
use rusqlite::ffi;
1111
use serde::Deserialize;
1212

1313
use super::SqliteError;

ext/node/ops/sqlite/statement.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use deno_core::op2;
88
use deno_core::v8;
99
use deno_core::v8::GetPropertyNamesArgs;
1010
use deno_core::GarbageCollected;
11-
use libsqlite3_sys as ffi;
11+
use rusqlite::ffi;
1212
use serde::Serialize;
1313

1414
use super::SqliteError;

tests/unit_node/sqlite_test.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,17 @@ Deno.test({
153153
);
154154
db.close();
155155
}
156+
{
157+
const db = new DatabaseSync(":memory:");
158+
assertThrows(
159+
() => {
160+
db.exec("ATTACH DATABASE 'test.db' AS test");
161+
},
162+
Error,
163+
"too many attached databases - max 0",
164+
);
165+
db.close();
166+
}
156167
},
157168
});
158169

0 commit comments

Comments
 (0)