fix(filedata): cap aggregate offline archive extraction (SEC-9491) - #849
Open
kparkinson-ld wants to merge 2 commits into
Open
kparkinson-ld wants to merge 2 commits into
kparkinson-ld wants to merge 2 commits into
Conversation
Contributor
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Contributor
|
@keelerm84 Are the proposed ceilings—1 GiB aggregate decompressed data and 10,000 tar entries—safe for the largest legitimate customer offline archive? If not, what two values should this PR use? |
…91-archive-limits
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
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.
Caps offline archive extraction before unbounded aggregate data or entry counts can be written.
Closes SEC-9491.
Requirements
Related issues
SEC-9491
Implementation details
Describe the solution you've provided
readTartracks the cumulative declared size and number of tar entries, rejecting the archive before extracting an entry that would exceed either ceiling. Boundary and rejection behavior are covered by unit tests.Describe alternatives you've considered
A new customer-facing configuration surface was avoided because these are safety ceilings rather than operational tuning controls. The provisional constants can be adjusted during review without changing the implementation shape.
Additional context
Validated with
make lint,make test, and the race-enabledinternal/filedatatests.Link to Devin session: https://app.devin.ai/sessions/52a2fa364bbe4af4ae3571fba69b2172
Open in Devin Desktop: https://app.devin.ai/desktop/session/52a2fa364bbe4af4ae3571fba69b2172?variant=devin
Requested by: @kparkinson-ld
Note
Overview
Adds aggregate safety limits on offline
.tar/.tar.gzextraction so malicious or malformed archives cannot declare unbounded total uncompressed size or entry count before files are written.readTarnow delegates toreadTarWithLimits, which counts every tar header and sums declared regular-file sizes against new ceilings (1 GiB total, 10,000 entries) while keeping the existing 200 MiB per-fileio.CopyNcap. Over-limit archives fail early with new “malformed or malicious archive” errors.Unit tests exercise rejection for aggregate size and entry count, plus acceptance at the exact limits via injectable limits in
readTarWithLimits.Reviewed by Cursor Bugbot for commit d852251. Bugbot is set up for automated code reviews on this repo. Configure here.