Cap archive extraction; stream zip/7z to tempfile (H-5, H-6) #108

Merged

qwc merged 1 commit from fix/archive-extraction-limits into main

2026-05-18 20:54:52 +02:00

qwc commented

2026-05-18 20:46:29 +02:00

Owner

Summary

Workstream item #13. Audit findings H-5 (decompression bomb / disk DoS) and H-6 (memory amplification from buffering archives in RAM).

H-5 — extraction limits

Each archive entry was capped at maxFileSize (100 MB) but there was no aggregate cap and no entry-count cap. A 100 MB upload of bz2/xz could expand to multi-GB on disk; any uploader could fill the data volume.

New per-extraction quotas, threaded through all formats:

maxTotalSize = 1 GB cumulative extracted bytes
maxEntries = 10000 entries
extractStats counter shared across all entries
limitedWriter enforces the byte cap incrementally so a bomb file aborts as soon as the cumulative limit is crossed — not after GB are already on disk

H-6 — buffering

extractZip and extract7z both io.ReadAll-ed the whole archive into RAM (capped at maxFileSize*10 = 1 GB; in practice 100 MB due to http.MaxBytesReader). Concurrent uploads scaled memory linearly.

Replace with bufferArchiveToTempFile: copy the bounded input into a temp file via io.LimitReader, pass the *os.File (which is an io.ReaderAt) plus the size to zip.NewReader / sevenzip.NewReader. closeAndRemove defers cleanup at the call sites.

Changes

internal/docs/archive.go — new constants, extractStats, limitedWriter, bufferArchiveToTempFile, closeAndRemove. extractZip, extract7z, extractTar all use them. Net change: cleaner control flow, +stats threading.
internal/docs/archive_limits_test.go — three regression tests:
- too-many-entries: maxEntries+1 1-byte files → rejected with "too many entries"
- total-byte bomb: 11 × 100 MB compressible entries → rejected with "expands beyond total extraction limit"
- normal small zip still extracts cleanly

Test plan

go test ./... passes
Entry-count and total-size caps fire as expected; bomb test aborts in ~4s rather than writing the full 1.1 GB
Manual: upload a normal Sphinx-sized docs zip, confirm it extracts as before
Manual: load the system with several concurrent uploads (existing peak RAM ≈ N × archive_size for zip/7z; new behavior should be much flatter — RAM stays small, /tmp usage scales instead)

🤖 Generated with Claude Code

## Summary Workstream item #13. Audit findings **H-5** (decompression bomb / disk DoS) and **H-6** (memory amplification from buffering archives in RAM). ### H-5 — extraction limits Each archive entry was capped at `maxFileSize` (100 MB) but there was no aggregate cap and no entry-count cap. A 100 MB upload of bz2/xz could expand to multi-GB on disk; any uploader could fill the data volume. New per-extraction quotas, threaded through all formats: - `maxTotalSize = 1 GB` cumulative extracted bytes - `maxEntries = 10000` entries - `extractStats` counter shared across all entries - `limitedWriter` enforces the byte cap incrementally so a bomb file aborts as soon as the cumulative limit is crossed — not after GB are already on disk ### H-6 — buffering `extractZip` and `extract7z` both `io.ReadAll`-ed the whole archive into RAM (capped at `maxFileSize*10` = 1 GB; in practice 100 MB due to `http.MaxBytesReader`). Concurrent uploads scaled memory linearly. Replace with `bufferArchiveToTempFile`: copy the bounded input into a temp file via `io.LimitReader`, pass the `*os.File` (which is an `io.ReaderAt`) plus the size to `zip.NewReader` / `sevenzip.NewReader`. `closeAndRemove` defers cleanup at the call sites. ## Changes - `internal/docs/archive.go` — new constants, `extractStats`, `limitedWriter`, `bufferArchiveToTempFile`, `closeAndRemove`. `extractZip`, `extract7z`, `extractTar` all use them. Net change: cleaner control flow, +stats threading. - `internal/docs/archive_limits_test.go` — three regression tests: - too-many-entries: `maxEntries+1` 1-byte files → rejected with "too many entries" - total-byte bomb: 11 × 100 MB compressible entries → rejected with "expands beyond total extraction limit" - normal small zip still extracts cleanly ## Test plan - [x] `go test ./...` passes - [x] Entry-count and total-size caps fire as expected; bomb test aborts in ~4s rather than writing the full 1.1 GB - [ ] Manual: upload a normal Sphinx-sized docs zip, confirm it extracts as before - [ ] Manual: load the system with several concurrent uploads (existing peak RAM ≈ N × archive_size for zip/7z; new behavior should be much flatter — RAM stays small, /tmp usage scales instead) 🤖 Generated with [Claude Code](https://claude.com/claude-code)

qwc added 1 commit

2026-05-18 20:46:29 +02:00

Cap archive extraction; stream zip/7z to tempfile (H-5, H-6)

CI / test (pull_request) Successful in 1m22s

Details

CI / build (pull_request) Successful in 57s

Details

CI / docker (pull_request) Has been skipped

Details

04308ec898

Audit findings H-5 (decompression bomb / disk DoS) and H-6 (memory
amplification from buffering whole archives in RAM).

H-5 — extraction limits. Each archive entry was capped at maxFileSize
(100 MB) but there was no aggregate cap and no entry-count cap. A 100 MB
upload of bz2/xz could expand to multi-GB on disk, and any uploader
could fill the volume.

Add to each extraction pass:
  - maxTotalSize   = 1 GB cumulative extracted bytes
  - maxEntries     = 10000 entries
  - extractStats   counter shared across all entries
  - limitedWriter  enforces the byte cap incrementally so a bomb file
                   aborts as soon as the cumulative limit is crossed,
                   not after writing GB to disk.

Wired through extractZip, extract7z, and extractTar (which covers
.tar.gz/.tar.bz2/.tar.xz).

H-6 — buffering. extractZip and extract7z both io.ReadAll-ed the whole
archive into RAM (capped at maxFileSize*10 = 1 GB; in practice 100 MB
due to http.MaxBytesReader). Concurrent uploads scaled memory linearly.

Replace with bufferArchiveToTempFile: copy the bounded input into a
temp file via io.LimitReader, then pass the *os.File to zip.NewReader
or sevenzip.NewReader (both accept io.ReaderAt + size). closeAndRemove
defers cleanup on the call sites.

Three regression tests in archive_limits_test.go:
  - too-many-entries (maxEntries+1 tiny files → rejected)
  - total-byte bomb (11×100MB → rejected mid-stream, well before
    writing 1.1 GB to disk)
  - normal small zip still extracts cleanly

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

qwc merged commit 93abda41c6 into main

2026-05-18 20:54:52 +02:00

qwc deleted branch fix/archive-extraction-limits

2026-05-18 20:54:53 +02:00

qwc referenced this pull request from a commit

2026-05-18 20:54:55 +02:00

Merge pull request 'Cap archive extraction; stream zip/7z to tempfile (H-5, H-6)' (#108) from fix/archive-extraction-limits into main

qwc referenced this pull request from a commit

2026-07-13 13:19:10 +02:00

Merge pull request 'Cap archive extraction; stream zip/7z to tempfile (H-5, H-6)' (#108) from fix/archive-extraction-limits into main