Skip to content

fix(tar): enforce limits for stdout extraction - #1946

Merged
chaliy merged 1 commit into
mainfrom
2026-06-05-propose-fix-for-tar-o-vulnerability
Jun 5, 2026
Merged

chaliy merged 1 commit into
mainfrom
2026-06-05-propose-fix-for-tar-o-vulnerability

Conversation

@chaliy

@chaliy chaliy commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Motivation

  • tar -O appended extracted file bytes directly to an in-memory stdout string and skipped VFS write checks, allowing per-file and cumulative output to bypass max_file_size and max_total_bytes quotas.
  • This could be triggered by untrusted archives to exhaust memory/output and evade existing filesystem protections.

Description

  • Enforce per-file size by calling limits.check_file_size(size as u64) before converting/adding a file to stdout.
  • Enforce cumulative stdout quota by calling limits.check_total_bytes(current_stdout_len, output_len) using the post-conversion byte length before appending.
  • Add build_tar_with_entries helper and two unit tests: test_tar_extract_stdout_enforces_file_size_limit and test_tar_extract_stdout_enforces_total_output_limit to cover per-file and total-output rejection cases.

Testing

  • Ran cargo fmt --check and applied formatting; check passed.
  • Ran the archive unit tests: cargo test -p bashkit builtins::archive::tests, which passed (all archive tests including the two new tests succeeded).
  • Ran the focused test filter cargo test -p bashkit test_tar_extract_stdout_enforces*, which passed.

Codex Task

Copilot AI review requested due to automatic review settings June 5, 2026 19:48

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens tar -xO extraction in bashkit by enforcing the same filesystem quota limits used for VFS writes, preventing untrusted archives from bypassing max_file_size / max_total_bytes via stdout extraction.

Changes:

  • Apply FsLimits::check_file_size and FsLimits::check_total_bytes before appending extracted file content to in-memory stdout for tar -O.
  • Add a build_tar_with_entries test helper for composing multi-entry tar fixtures.
  • Add two unit tests covering per-file and cumulative stdout quota enforcement for tar -xO.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +599 to +611
if let Err(err) = limits.check_file_size(size as u64) {
return Ok(ExecResult::err(format!("tar: {name}: {err}\n"), 2));
}

let output = String::from_utf8_lossy(content);
if let Err(err) =
limits.check_total_bytes(stdout_output.len() as u64, output.len() as u64)
{
return Ok(ExecResult::err(format!("tar: {name}: {err}\n"), 2));
}

// -O: write content to stdout after applying VFS-sized quotas.
stdout_output.push_str(&output);
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jun 5, 2026 •

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
bashkit 0aff55e Commit Preview URL Jun 05 2026, 08:04 PM

@chaliy
chaliy merged commit ed5cd11 into main Jun 5, 2026
36 checks passed
@chaliy
chaliy deleted the 2026-06-05-propose-fix-for-tar-o-vulnerability branch June 5, 2026 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants