cli/context/store: cap tls file size on zip import#6934
Open
texasich wants to merge 1 commit intodocker:masterfrom
Open
cli/context/store: cap tls file size on zip import#6934texasich wants to merge 1 commit intodocker:masterfrom
texasich wants to merge 1 commit intodocker:masterfrom
Conversation
`io.ReadAll(f)` on the decompressed tls/* entries was unbounded, so a zip whose compressed archive is within the 10 MiB outer cap could still decompress to multi-gigabyte TLS files and OOM the CLI. The meta.json branch right above already wraps its reader in limitedReader, this just mirrors it for the tls branch. Also fast-fails on the advertised UncompressedSize64 before calling zf.Open, so a well-formed zip bomb is rejected without any decompression at all. limitedReader still guards the stream in case the header lies. Fixes docker#6917 Signed-off-by: texasich <texasich@users.noreply.github.com>
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.
Wrap the
io.ReadAllcall fortls/*entries inimportZipwith the existinglimitedReader, and add a fast-fail check againstzf.UncompressedSize64before opening the entry. Fixes #6917.The
meta.jsonbranch a few lines above already uses&limitedReader{R: f, N: maxAllowedFileSizeToImport}; thetls/branch was still callingio.ReadAll(f)directly, which is unbounded after flate has decompressed. A compressed archive that fits under the outer 10 MiB cap can trivially decompress to multi-GB TLS entries and take the CLI out with an OOM.Primary guard is the header check: if
UncompressedSize64exceeds the cap we bail out before callingzf.Open(), so no decompression happens at all for a well-formed zip bomb. ThelimitedReaderwrapper stays as defense-in-depth for the case where the header lies about the declared size.Added
TestImportZipTLSTooLargeincli/context/store/store_test.go: it builds an in-memory zip whosetls/docker/ca.pementry is2 * maxAllowedFileSizeToImportzero bytes (compresses to ~10 KiB, so the outer archive limit is not what's catching it), and asserts thatImportrejects it with the expected error. Also rango vetandgo buildon linux and windows.Local
go test ./cli/context/store/passes aside from an unrelated pre-existingTestImportZipTempDir cleanup flake on Windows (open handle ontest.zip), which reproduces on master without these changes.Reject oversized TLS entries in
docker context importof zip archives to prevent OOM from zip bombs.