Dstack-TEE / Dstack-TEE/dstack

Audit remaining network tar extraction sites (vmm OCI layers, dstackup) after #881

Open
#991 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
544
Forks
96
Avg merge
17h 57m
Merged PRs (30d)
117

Description

## Context

#881 hardened image archive extraction in `dstack-verifier` (`download_image` → `extract_image_archive`): entry paths are restricted to normal relative components, only regular files and directories are accepted, and `tar::Entry::unpack_in` confinement failures are treated as errors.

The same class of input — a tar archive fetched over the network and unpacked into a local directory — exists at two other call sites that #881 intentionally left out of scope. This issue tracks auditing them.

## Call site 1: VMM OCI layer extraction (primary)

`dstack/vmm/src/app/registry.rs:260` (`extract_layer`, reached from `download_and_extract_layers` at line 253):

```rust
let decoder = GzDecoder::new(data);
let mut archive = tar::Archive::new(decoder);
archive.unpack(dest).context("failed to extract gzipped tar layer")?;
```

This unpacks guest-image layers pulled from a container registry over the OCI Distribution API, before any measurement or signature check binds the content. Compared to the verifier path after #881:

- **No entry-type allowlist.** Symlinks, hardlinks, character/block devices and FIFOs in a layer are materialised on the host. `tar-rs` only special-cases dir/symlink/hardlink; other node types fall through to the generic unpack path.
- **Escaping entries are silently skipped, not rejected.** `Archive::unpack` calls `Entry::unpack_in` per entry and discards the `bool`, so a member containing `..` is dropped without any error. The extraction reports success with a partial result. #881 explicitly turned this into a hard error for the verifier.
- **Single-member gzip only.** `flate2::read::GzDecoder` stops at the first gzip member and returns clean EOF; `tar::Archive` then ends iteration with no error, so a multi-member layer extracts partially and silently. (Verified locally: a 2048-byte tar split across two concatenated gzip members yields 1024 bytes and 1 entry, `err=None`.)
- **Post-extraction cleanup is best-effort.** The `for dir in &["dev", "etc", "proc", "sys"]` loop uses `fs_err::remove_dir` (non-recursive) and ignores the result, so a non-empty `etc/` from a layer survives.

Mitigations that *are* already present, for the record: `tar-rs` `unpack_in` drops `..` members, canonicalises the parent directory via `validate_inside_dst` before writing (so symlink-through-parent traversal is blocked), and masks setuid/setgid off unless `set_preserve_permissions(true)` is called. So this is a hardening/robustness gap and an unhelpful-failure-mode problem, not a known traversal vulnerability.

Note that this call site cannot simply reuse #881's rule set: container rootfs layers legitimately contain symlinks and whiteout entries, so it needs its own policy (e.g. an explicit type allowlist, erroring on skipped members, `MultiGzDecoder`, and a decompressed-size / entry-count cap) rather than a copy of the verifier logic.

## Call site 2: dstackup (secondary)

`dstack/crates/dstackup/src/image.rs:752` (`extract`) shells out to `tar -xzf ... --no-same-owner --no-same-permissions`. Ownership and permission carry-over are already handled and the intent is documented in a comment. Remaining gaps are symlink/hardlink members and the absence of a size cap. Lower priority than call site 1.

## Suggested scope

- [ ] Define and document the entry-type policy for OCI layer extraction in `vmm`.
- [ ] Turn silently-skipped (escaping) members into an error.
- [ ] Switch `GzDecoder` → `MultiGzDecoder` in `extract_layer`.
- [ ] Bound decompressed size and entry count for network-fetched archives (also missing in the verifier path after #881).
- [ ] Make the `dev`/`etc`/`proc`/`sys` cleanup explicit about what it does and does not remove, or drop it in favour of the type allowlist.

Refs: #881

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.