Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
146 changes: 135 additions & 11 deletions crates/bashkit/src/builtins/sqlite/dot_commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ pub(super) enum DotError {
InvalidValue { cmd: &'static str, value: String },
#[error("sqlite engine error: {0}")]
Engine(String),
#[error("sqlite output exceeds byte cap ({0} > {1})")]
OutputCap(usize, usize),
Comment on lines +55 to +56
}

const HELP_TEXT: &str = concat!(
Expand All @@ -69,12 +71,19 @@ const HELP_TEXT: &str = concat!(
".read PATH Execute SQL from a VFS file\n",
);

/// Dispatch a dot-command line.
///
/// `max_output_bytes` is the *remaining* output budget for the current
/// invocation. `.dump` enforces this cumulatively across schema and all
/// table rows so memory growth is bounded during construction, not only
/// after the full string is returned to the caller.
pub(super) fn dispatch(
line: &str,
engine: &SqliteEngine,
opts: &mut OutputOpts,
deadline: Deadline,
limits: QueryLimits,
max_output_bytes: usize,
) -> Result<DotOutcome, DotError> {
let (name, args) = tokenize_dot(line);
match name.as_str() {
Expand All @@ -89,7 +98,7 @@ pub(super) fn dispatch(
"indexes" | "indices" => {
indexes(args, engine, opts, deadline, limits).map(DotOutcome::Stdout)
}
"dump" => dump(engine, deadline, limits).map(DotOutcome::Stdout),
"dump" => dump(engine, deadline, limits, max_output_bytes).map(DotOutcome::Stdout),
"read" => {
let path = args
.into_iter()
Expand Down Expand Up @@ -276,15 +285,39 @@ fn indexes(
Ok(out)
}

/// Append `chunk` to `buf`, returning `Err(OutputCap)` if doing so would
/// exceed `max`. This enforces the output budget *during* construction so
/// memory never grows beyond the cap even for large multi-table dumps.
Comment on lines +288 to +290
/// THREAT[TM-DOS-091]: cumulative `.dump` output cap enforced here.
fn bounded_append(buf: &mut String, chunk: &str, max: usize) -> Result<(), DotError> {
let next = buf.len().saturating_add(chunk.len());
if next > max {
return Err(DotError::OutputCap(next, max));
}
buf.push_str(chunk);
Ok(())
}

/// Emit `BEGIN; <CREATE TABLE>...; <INSERT INTO ... VALUES (...)>; COMMIT;`.
/// This matches sqlite3's `.dump` for tables; views/triggers/indexes only get
/// their CREATE statement, no rows. Blob literals are emitted as `X'..'`.
///
/// `max_output_bytes` is the remaining output budget for the invocation.
/// The cap is applied cumulatively across schema lines and every row from
/// every table — construction stops as soon as the budget would be exceeded,
/// preventing unbounded memory growth from large multi-table databases.
fn dump(
engine: &SqliteEngine,
deadline: Deadline,
limits: QueryLimits,
max_output_bytes: usize,
) -> Result<String, DotError> {
let mut out = String::from("PRAGMA foreign_keys=OFF;\nBEGIN TRANSACTION;\n");
let mut out = String::new();
bounded_append(
&mut out,
"PRAGMA foreign_keys=OFF;\nBEGIN TRANSACTION;\n",
max_output_bytes,
)?;

// Schema first.
let schema_outcome = engine
Expand All @@ -296,8 +329,8 @@ fn dump(
.map_err(DotError::Engine)?;
for row in &schema_outcome.rows {
if let Some(Value::Text(sql)) = row.get(2) {
out.push_str(sql.as_str());
out.push_str(";\n");
bounded_append(&mut out, sql.as_str(), max_output_bytes)?;
bounded_append(&mut out, ";\n", max_output_bytes)?;
}
}

Expand All @@ -321,15 +354,12 @@ fn dump(
.map_err(DotError::Engine)?;
for data_row in &data.rows {
let values: Vec<String> = data_row.iter().map(format_sql_literal).collect();
out.push_str(&format!(
"INSERT INTO \"{}\" VALUES({});\n",
quoted,
values.join(",")
));
let line = format!("INSERT INTO \"{}\" VALUES({});\n", quoted, values.join(","));
bounded_append(&mut out, &line, max_output_bytes)?;
}
}

out.push_str("COMMIT;\n");
bounded_append(&mut out, "COMMIT;\n", max_output_bytes)?;
Ok(out)
}

Expand Down Expand Up @@ -385,7 +415,14 @@ mod tests {
engine: &SqliteEngine,
opts: &mut OutputOpts,
) -> Result<DotOutcome, DotError> {
dispatch(line, engine, opts, no_deadline(), unlimited_limits())
dispatch(
line,
engine,
opts,
no_deadline(),
unlimited_limits(),
usize::MAX,
)
}

#[test]
Expand Down Expand Up @@ -563,6 +600,93 @@ mod tests {
assert!(s.contains("COMMIT;"));
}

/// `.dump` must stop and return an error when the cumulative output would
/// exceed the configured output cap, preventing unbounded memory growth
/// across many tables (DeepSec issue #1869).
#[test]
fn dump_respects_output_cap() {
let engine = mk_engine();
// Two tables with several rows of text data.
engine
.execute("CREATE TABLE a(v TEXT)", no_deadline(), unlimited_limits())
.unwrap();
engine
.execute("CREATE TABLE b(v TEXT)", no_deadline(), unlimited_limits())
.unwrap();
for i in 0..10 {
engine
.execute(
&format!(
"INSERT INTO a VALUES ('row-{i:04}-padding-aaaaaaaaaaaaaaaaaaaaaaaaa')"
),
no_deadline(),
unlimited_limits(),
)
.unwrap();
engine
.execute(
&format!(
"INSERT INTO b VALUES ('row-{i:04}-padding-bbbbbbbbbbbbbbbbbbbbbbbbb')"
),
no_deadline(),
unlimited_limits(),
)
.unwrap();
}

// A tiny cap that the dump cannot fit into, ensuring the cap fires
// mid-construction rather than only after the full string is built.
let tiny_cap: usize = 100;
let mut o = opts();
let err = dispatch(
".dump",
&engine,
&mut o,
no_deadline(),
unlimited_limits(),
tiny_cap,
)
.unwrap_err();

assert!(
matches!(err, DotError::OutputCap(_, _)),
"expected OutputCap, got: {err}"
);
// Error message must not contain debug shapes — plain text only.
let msg = err.to_string();
assert!(
!msg.contains("{:?}"), // debug-ok: testing that debug shapes are absent from error output
"debug shape leaked into error: {msg}"
);
assert!(
msg.len() <= 1024,
"error message too long ({} bytes)",
msg.len()
);
}

/// `.dump` on an empty database still produces the PRAGMA/BEGIN/COMMIT
/// boilerplate even with a very tight cap that fits it.
#[test]
fn dump_empty_db_under_cap() {
let engine = mk_engine();
let mut o = opts();
// "PRAGMA foreign_keys=OFF;\nBEGIN TRANSACTION;\nCOMMIT;\n" is ~50 bytes.
let DotOutcome::Stdout(s) = dispatch(
".dump",
&engine,
&mut o,
no_deadline(),
unlimited_limits(),
4096,
)
.unwrap() else {
panic!("expected stdout");
};
assert!(s.starts_with("PRAGMA foreign_keys=OFF;"));
assert!(s.ends_with("COMMIT;\n"));
}

#[test]
fn read_returns_path() {
let engine = mk_engine();
Expand Down
13 changes: 11 additions & 2 deletions crates/bashkit/src/builtins/sqlite/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -789,8 +789,17 @@ async fn run_statements(
push_stdout_bounded(stdout, &rendered, limits.max_output_bytes)?;
}
Stmt::Dot(line) => {
let result =
dot_commands::dispatch(&line, engine, opts, deadline, query_limits(limits));
// Pass the *remaining* output budget so .dump can enforce it
// cumulatively during construction (DeepSec #1869).
let remaining = limits.max_output_bytes.saturating_sub(stdout.len());
let result = dot_commands::dispatch(
&line,
engine,
opts,
deadline,
query_limits(limits),
remaining,
);
match result {
Ok(DotOutcome::Stdout(s)) => {
push_stdout_bounded(stdout, &s, limits.max_output_bytes)?;
Expand Down
31 changes: 31 additions & 0 deletions crates/bashkit/src/builtins/sqlite/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,37 @@ async fn dump_data_respects_row_cap() {
);
}

/// `.dump` output cap is enforced cumulatively across all tables, not just
/// per-query (DeepSec #1869). With a tiny max_output_bytes, a dump that
/// would fit under max_result_bytes per table still hits the invocation cap.
#[tokio::test]
async fn dump_output_cap_enforced_across_multiple_tables() {
let limits = SqliteLimits::default()
.max_rows_per_query(1000)
.max_result_bytes(64 * 1024)
.max_output_bytes(120) // too small for schema + any data rows
.max_duration(std::time::Duration::ZERO);
let r = run_with_limits(
&[
":memory:",
"CREATE TABLE a(v TEXT); CREATE TABLE b(v TEXT);\
INSERT INTO a VALUES ('row-aaa'), ('row-bbb');\
INSERT INTO b VALUES ('row-ccc'), ('row-ddd');\n.dump",
],
None,
limits,
)
.await;
assert_eq!(r.exit_code, 1, "should fail due to output cap");
// Error must be plain text, not a debug-formatted struct.
let msg = &r.stderr;
assert!(
msg.contains("output exceeds byte cap") || msg.contains("sqlite:"),
"unexpected stderr: {msg}"
);
assert!(!msg.contains("{:?}"), "debug shape leaked into stderr"); // debug-ok: testing that debug shapes are absent
}

#[tokio::test]
async fn value_byte_cap_rejects_large_cell_before_rendering() {
let limits = SqliteLimits::default()
Expand Down
1 change: 1 addition & 0 deletions specs/sqlite-builtin.md
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@ leading SQL keyword via the parser's lightweight tokeniser
| TM-SQL-010 | DoS / fingerprinting via dangerous PRAGMAs | `SqliteLimits::pragma_deny` defaults block `cache_size`, `mmap_size`, `page_size`, `max_page_count`, `temp_store_directory`, `data_store_directory`, `compile_options`, `locking_mode`, `shared_cache`; parser handles comments plus quoted/schema-qualified names |
| TM-SQL-011 | Information leakage via host-side error strings | `sanitize()` strips ` at /…:N:M` annotations from turso errors |
| TM-SQL-012 | Sandbox escape via `VACUUM INTO` writing host files | Policy rejects `VACUUM` (with/without `INTO`) at the keyword sniffer; tested via `vacuum_into_blocked`/`vacuum_plain_blocked`/`vacuum_blocked_with_leading_comment` |
| TM-SQL-013 | DoS via `.dump` cumulative output bypass | `.dump` previously built the full string before `max_output_bytes` was applied; `bounded_append()` enforces the cap after each schema/row chunk with the remaining budget passed from `run_statements`; `THREAT[TM-DOS-091]`; tested via `dump_respects_output_cap` and `dump_output_cap_enforced_across_multiple_tables` |

## Test Plan

Expand Down
1 change: 1 addition & 0 deletions specs/threat-model.md
Original file line number Diff line number Diff line change
Expand Up @@ -1385,6 +1385,7 @@ This section maps former vulnerability IDs to the new threat ID scheme and track
| TM-DOS-088 | Command substitution OOM via state cloning | OOM at depth N (memory ≈ N × state_size) | Dedicated `max_subst_depth` limit (default 32), separate from `max_function_depth` — **FIXED** via #1088 |
| TM-DOS-089 | Command substitution stack overflow via inlined futures | SIGABRT at ~20-30 nested $() levels | Box::pin `expand_word` and `execute_cmd_subst` to cap per-level stack — **FIXED** via #1089 |
| ~~TM-DOS-090~~ | ~~`shuf` unbounded range/repeat materialization~~ | ~~OOM/CPU exhaustion via huge `--input-range` or `--head-count` before stdout truncation~~ | ~~Sample numeric ranges without full collection and reject output that exceeds `ExecutionLimits` before allocation~~ — `shuf_resource_tests` cover huge range `-n 1` and repeat output caps (**FIXED**) |
| TM-DOS-091 | SQLite `.dump` cumulative output bypass | `.dump` built the full schema+rows string before checking `max_output_bytes`; an attacker controlling many tables/rows could cause memory to grow far beyond the configured output cap | `bounded_append()` helper enforces the cap after each schema line and each INSERT row; `dispatch()` receives the *remaining* budget (`max_output_bytes - stdout.len()`) from `run_statements` — **FIXED** via #1869 |

### Accepted (Low Priority)

Expand Down
Loading