fix(image): cap metadata, layer, and daemon-spool reads against resource exhaustion#80
Open
deveshctl wants to merge 3 commits into
Open
fix(image): cap metadata, layer, and daemon-spool reads against resource exhaustion#80deveshctl wants to merge 3 commits into
deveshctl wants to merge 3 commits into
Conversation
Three resource-exhaustion paths in the archive/spool pipeline sat outside the existing MaxLayerBlobSize cap, each reachable from 'layerx <archive.tar>' with no daemon: - A gzip-bomb layer expanded without bound because the analysis path passed the decompressed reader straight to ParseLayerTar. It now mirrors the extraction path with io.LimitReader(dec, MaxLayerBlobSize+1). - manifest.json / image config / legacy root *.json were read with an unbounded io.ReadAll that trusted the tar header's declared size. A new MaxMetadataSize (64 MiB) cap plus a shared readMetadataEntry helper fails fast on an inflated header and bounds the stream when the header understates. - The daemon ImageSave response was spooled to a temp file with no ceiling, letting a rogue DOCKER_HOST fill the disk. A new MaxArchiveSize (64 GiB) cap via spoolFromDaemon bounds both daemon spool sites. Also add TestEfficiency_Golden pinning the full EfficiencyResult for a fixed multi-layer fixture, so a refactor touching the waste denominator fails loudly.
The golden fixture's /etc/config appears at the same 300-byte size in L1 and L2. The test assumed Stack would classify the L2 re-emit as Unchanged carryover and omit it from the waste run, expecting WastedBytes=2200 and a single WastedFile. Stack has no identical-size shortcut for files: a path present in both the cumulative tree and the incoming layer is always Modified, so /etc/config opens a second Added/Modified occurrence that pathRuns records. The real result is a run [(L1,300),(L2,300)] charging 300 wasted bytes, giving WastedBytes=2500, Score=1-2500/4800, and two WastedFiles (/bin/app 2200, /etc/config 300). Correct the expectations and the derivation comment to match the actual waste-accounting semantics. Production code is unchanged; only the test's predicted numbers were wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes three resource-exhaustion paths in the archive/spool pipeline that sat outside the existing per-blob cap. Each is reachable from
layerx <archive.tar>with no daemon, and each could exhaust memory or disk before any meaningful parsing began.Changes
io.LimitReaderat the same 16 GiB ceiling the extraction path already enforced, so both paths are equally bomb-resistant.manifest.json, the image config, and legacy root-level*.jsonfiles were read with an unboundedio.ReadAllthat trusted the tar header's declared size. A new 64 MiB cap (MaxMetadataSize) plus a sharedreadMetadataEntryhelper fails fast when a header overstates the size and bounds the stream when the header understates it — a hostile archive can lie in either direction.ImageSaveresponse was spooled to a temp file with no ceiling, letting a rogue or compromised container-engine endpoint fill the local disk with an endless response. A new 64 GiB cap (MaxArchiveSize) viaspoolFromDaemonbounds both daemon spool sites.Cap sizes are ordered so legitimate images are unaffected: 64 MiB metadata < 16 GiB per layer < 64 GiB whole archive.
Also adds a golden test pinning the full efficiency result for a fixed multi-layer fixture, so a refactor that silently changes the waste denominator fails loudly, and pins the nightly workflow's Go toolchain to a patch release.
Testing
go build ./...andgo vet ./...clean.