Skip to content

Commit 9f869fd

Browse files
ZhiXiao-LinRoy Lin
andauthored
fix(security): cap decompressed output to stop image/archive decompression bombs (#142)
A malicious image layer or build-context archive that compresses tiny but expands to hundreds of GB could fill the HOST disk during pull/build (DoS) — extraction streamed gzip/zstd/bzip2/xz into tar with no bound on decompressed output. The sibling ADD-URL path already caps downloads at MAX_ADD_URL_BYTES (512 MiB); the local-archive and layer paths had no equivalent. - New `oci::limited_reader::LimitedReader`: a Read adapter that errors once a byte budget is exceeded, plus `cap_from_env` for operator-tunable ceilings. - extract_layer (registry pull): wrap the decoder, cap total decompressed bytes (A3S_BOX_MAX_LAYER_BYTES, default 16 GiB). Refactored into a cap-parameterized inner fn so the bomb path is testable without env races. Also surface the underlying cause in the error (tar's Display alone hides the cap abort). - extract_tar_to_dst (build ADD/COPY auto-extract): wrap every decoder branch (tar/tar.gz/tar.bz2/tar.xz) with the cap (A3S_BOX_MAX_BUILD_EXTRACT_BYTES, default 4 GiB) — restoring parity with the already-capped ADD-URL path. Defaults are generous (no realistic legit single layer/archive hits them) and env-tunable; a bomb (100s of GB–PB) is bounded and aborts with a clear error. Tests: LimitedReader unit tests (errors past cap / passes under); extract_layer bomb test asserts a 64 KiB member under a 4 KiB cap ABORTS and bounds bytes written; generous-cap test confirms no regression. Neuter-verified on the KVM server (dropping the wrap makes the bomb test FAIL: got Ok). fmt + clippy clean. Severity: HIGH for build ADD (build farms / hosted build APIs run untrusted Dockerfiles), MEDIUM for layer pull (malicious-image precondition). Co-authored-by: Roy Lin <roylin@a3s.box>
1 parent 9caf6de commit 9f869fd

4 files changed

Lines changed: 155 additions & 6 deletions

File tree

src/runtime/src/oci/build/engine/utils.rs

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ pub(super) fn is_tar_archive(name: &str) -> bool {
2222

2323
/// Extract a tar archive to a destination directory.
2424
pub(super) fn extract_tar_to_dst(archive_path: &Path, dst: &Path) -> Result<()> {
25+
use crate::oci::limited_reader::LimitedReader;
2526
use flate2::read::GzDecoder;
2627
use std::io::BufReader;
2728

@@ -43,8 +44,17 @@ pub(super) fn extract_tar_to_dst(archive_path: &Path, dst: &Path) -> Result<()>
4344

4445
let name = archive_path.to_str().unwrap_or("").to_lowercase();
4546

47+
// Bound decompressed output so a compression-bomb archive (`ADD app.tar.gz`)
48+
// cannot fill the build host's disk. Mirrors the MAX_ADD_URL_BYTES cap on the
49+
// ADD-URL path (which the local-archive path previously lacked); tune with
50+
// A3S_BOX_MAX_BUILD_EXTRACT_BYTES.
51+
let max_bytes = crate::oci::limited_reader::cap_from_env(
52+
"A3S_BOX_MAX_BUILD_EXTRACT_BYTES",
53+
4 * 1024 * 1024 * 1024,
54+
);
55+
4656
if name.ends_with(".tar.gz") || name.ends_with(".tgz") {
47-
let decoder = GzDecoder::new(BufReader::new(file));
57+
let decoder = LimitedReader::new(GzDecoder::new(BufReader::new(file)), max_bytes);
4858
let mut archive = tar::Archive::new(decoder);
4959
archive.unpack(dst).map_err(|e| {
5060
BoxError::BuildError(format!(
@@ -64,7 +74,7 @@ pub(super) fn extract_tar_to_dst(archive_path: &Path, dst: &Path) -> Result<()>
6474
{
6575
use bzip2::read::BzDecoder;
6676

67-
let decoder = BzDecoder::new(BufReader::new(file));
77+
let decoder = LimitedReader::new(BzDecoder::new(BufReader::new(file)), max_bytes);
6878
let mut archive = tar::Archive::new(decoder);
6979
archive.unpack(dst).map_err(|e| {
7080
BoxError::BuildError(format!(
@@ -85,7 +95,7 @@ pub(super) fn extract_tar_to_dst(archive_path: &Path, dst: &Path) -> Result<()>
8595
{
8696
use xz2::read::XzDecoder;
8797

88-
let decoder = XzDecoder::new(BufReader::new(file));
98+
let decoder = LimitedReader::new(XzDecoder::new(BufReader::new(file)), max_bytes);
8999
let mut archive = tar::Archive::new(decoder);
90100
archive.unpack(dst).map_err(|e| {
91101
BoxError::BuildError(format!(
@@ -96,7 +106,7 @@ pub(super) fn extract_tar_to_dst(archive_path: &Path, dst: &Path) -> Result<()>
96106
})?;
97107
}
98108
} else if name.ends_with(".tar") {
99-
let mut archive = tar::Archive::new(BufReader::new(file));
109+
let mut archive = tar::Archive::new(LimitedReader::new(BufReader::new(file), max_bytes));
100110
archive.unpack(dst).map_err(|e| {
101111
BoxError::BuildError(format!(
102112
"Failed to extract tar {}: {}",

src/runtime/src/oci/layers.rs

Lines changed: 59 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,19 @@ use tar::Archive;
2424
/// - Extraction fails
2525
/// - Target directory cannot be created
2626
pub fn extract_layer(layer_path: &Path, target_dir: &Path) -> Result<()> {
27+
// Bound total decompressed output so a compression-bomb layer (a few MB that
28+
// expands to hundreds of GB of zeros) cannot fill the host disk during pull.
29+
// Generous default; tune with A3S_BOX_MAX_LAYER_BYTES.
30+
let max_layer_bytes =
31+
super::limited_reader::cap_from_env("A3S_BOX_MAX_LAYER_BYTES", 16 * 1024 * 1024 * 1024);
32+
extract_layer_with_cap(layer_path, target_dir, max_layer_bytes)
33+
}
34+
35+
fn extract_layer_with_cap(
36+
layer_path: &Path,
37+
target_dir: &Path,
38+
max_layer_bytes: u64,
39+
) -> Result<()> {
2740
// Validate layer exists
2841
if !layer_path.exists() {
2942
return Err(BoxError::OciImageError(format!(
@@ -82,6 +95,8 @@ pub fn extract_layer(layer_path: &Path, target_dir: &Path) -> Result<()> {
8295
Box::new(file)
8396
};
8497

98+
let decoder = super::limited_reader::LimitedReader::new(decoder, max_layer_bytes);
99+
85100
// Extract the tar archive, applying OCI whiteout semantics so files deleted
86101
// in an upper layer do not reappear from lower layers:
87102
// - `.wh.<name>` deletes the sibling `<name>` already materialized
@@ -164,10 +179,15 @@ pub fn extract_layer(layer_path: &Path, target_dir: &Path) -> Result<()> {
164179
}
165180

166181
entry.unpack_in(target_dir).map_err(|e| {
182+
// Surface the underlying cause (e.g. the LimitedReader's size-cap
183+
// error) — tar's wrapper Display alone would just say "failed to
184+
// unpack <path>" and hide a decompression-bomb abort from the operator.
185+
let cause = std::error::Error::source(&e)
186+
.map(|src| format!("{e}: {src}"))
187+
.unwrap_or_else(|| e.to_string());
167188
BoxError::OciImageError(format!(
168-
"Failed to extract layer to {}: {}",
189+
"Failed to extract layer to {}: {cause}",
169190
target_dir.display(),
170-
e
171191
))
172192
})?;
173193
}
@@ -371,6 +391,43 @@ mod tests {
371391
assert!(!target.join("d/.wh..wh..opq").exists());
372392
}
373393

394+
#[test]
395+
fn extract_layer_rejects_decompression_bomb_past_cap() {
396+
let temp_dir = TempDir::new().unwrap();
397+
let layer = temp_dir.path().join("bomb.tar.gz");
398+
let target = temp_dir.path().join("out");
399+
// 64 KiB of zeros — compresses to almost nothing but exceeds a small cap,
400+
// standing in for a real layer that expands to hundreds of GB.
401+
let big = vec![0u8; 64 * 1024];
402+
create_test_layer(&layer, &[("big", &big)]);
403+
404+
// A 4 KiB cap must abort the extraction...
405+
let result = extract_layer_with_cap(&layer, &target, 4 * 1024);
406+
assert!(
407+
result.is_err(),
408+
"the cap must abort an oversized (bomb) layer, got: {result:?}"
409+
);
410+
// ...BEFORE the full 64 KiB member is written to disk.
411+
let written = std::fs::metadata(target.join("big"))
412+
.map(|m| m.len())
413+
.unwrap_or(0);
414+
assert!(
415+
written < 64 * 1024,
416+
"cap must bound bytes written before aborting; wrote {written}"
417+
);
418+
}
419+
420+
#[test]
421+
fn extract_layer_with_generous_cap_extracts_normally() {
422+
let temp_dir = TempDir::new().unwrap();
423+
let layer = temp_dir.path().join("ok.tar.gz");
424+
let target = temp_dir.path().join("out");
425+
create_test_layer(&layer, &[("file.txt", b"hello")]);
426+
// A generous cap must not regress a normal small layer.
427+
extract_layer_with_cap(&layer, &target, 16 * 1024 * 1024).unwrap();
428+
assert!(target.join("file.txt").exists());
429+
}
430+
374431
// Helper function to create a test tar.gz layer
375432
fn create_test_layer(path: &Path, files: &[(&str, &[u8])]) {
376433
use flate2::write::GzEncoder;
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
//! A `Read` adapter that fails once a byte budget is exceeded.
2+
//!
3+
//! Layer/archive extraction streams a decompressor (gzip/zstd/bzip2/xz) into a
4+
//! tar reader with no bound on the *decompressed* output. A compression-bomb
5+
//! layer (a few MB that expands to hundreds of GB) would therefore fill the
6+
//! host disk during `pull`/build and DoS the host. Wrapping the decompressor in
7+
//! [`LimitedReader`] caps total decompressed bytes and aborts with a clear error
8+
//! instead — mirroring the existing `MAX_ADD_URL_BYTES` cap on the ADD-URL path.
9+
10+
use std::io::{self, Read};
11+
12+
/// Wraps a reader and returns an error once more than `limit` bytes have been
13+
/// read in total. Use it around a decompressor so the tar layer above it cannot
14+
/// pull an unbounded amount of decompressed data to disk.
15+
pub(crate) struct LimitedReader<R> {
16+
inner: R,
17+
remaining: u64,
18+
limit: u64,
19+
}
20+
21+
impl<R: Read> LimitedReader<R> {
22+
pub(crate) fn new(inner: R, limit: u64) -> Self {
23+
Self {
24+
inner,
25+
remaining: limit,
26+
limit,
27+
}
28+
}
29+
}
30+
31+
impl<R: Read> Read for LimitedReader<R> {
32+
fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
33+
let n = self.inner.read(buf)?;
34+
if n as u64 > self.remaining {
35+
return Err(io::Error::other(format!(
36+
"decompressed output exceeded the {}-byte limit (possible decompression bomb); \
37+
raise the limit via the relevant A3S_BOX_MAX_*_BYTES env var if this is a \
38+
legitimately large image/archive",
39+
self.limit
40+
)));
41+
}
42+
self.remaining -= n as u64;
43+
Ok(n)
44+
}
45+
}
46+
47+
/// Read a byte-size cap from `var`, falling back to `default_bytes` when unset or
48+
/// unparseable. Lets operators tune the decompression-bomb ceilings.
49+
pub(crate) fn cap_from_env(var: &str, default_bytes: u64) -> u64 {
50+
std::env::var(var)
51+
.ok()
52+
.and_then(|v| v.trim().parse::<u64>().ok())
53+
.filter(|&v| v > 0)
54+
.unwrap_or(default_bytes)
55+
}
56+
57+
#[cfg(test)]
58+
mod tests {
59+
use super::*;
60+
61+
#[test]
62+
fn errors_once_total_read_exceeds_limit() {
63+
// 10 KiB of data, 4 KiB cap → must error before delivering it all.
64+
let data = vec![0u8; 10 * 1024];
65+
let mut r = LimitedReader::new(&data[..], 4 * 1024);
66+
let mut sink = Vec::new();
67+
let err = std::io::copy(&mut r, &mut sink).unwrap_err();
68+
assert_eq!(err.kind(), io::ErrorKind::Other);
69+
assert!(sink.len() as u64 <= 4 * 1024, "must not exceed the cap");
70+
assert!(err.to_string().contains("decompression bomb"));
71+
}
72+
73+
#[test]
74+
fn passes_data_under_the_limit() {
75+
let data = vec![7u8; 4096];
76+
let mut r = LimitedReader::new(&data[..], 1024 * 1024);
77+
let mut sink = Vec::new();
78+
std::io::copy(&mut r, &mut sink).unwrap();
79+
assert_eq!(sink, data);
80+
}
81+
}

src/runtime/src/oci/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ pub mod build;
2929
pub mod credentials;
3030
mod image;
3131
mod layers;
32+
mod limited_reader;
3233
mod pull;
3334
pub mod reference;
3435
pub mod registry;

0 commit comments

Comments
 (0)