Bound header and armor parsing against pre-auth memory exhaustion#19
Merged
Conversation
A hostile stream with an unterminated multi-gigabyte line was buffered in full before any length or authenticity check could reject it. Cap header lines at 64 KiB and total header size at 16 MiB, and bound armor lines via a pass-through stream so StreamReader.ReadLine keeps its fast path. The armor bound costs one vectorized scan per buffered read; armored decrypt benchmarks stay within noise of the unbounded code.
Both branches of the newline walk threw the same exception; computing the line length once lets a single check cover them.
The header and armor bounds were private constants split across two parsers. Expose them as AgeLimits.MaxHeaderLineBytes / MaxHeaderBytes / MaxArmorLineBytes so callers can see the limits and reference them, and so a configurable override has an obvious home later. Document them in the README and add direct coverage for the total-header cap.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #19 +/- ##
==========================================
- Coverage 92.05% 91.28% -0.77%
==========================================
Files 41 42 +1
Lines 2390 2443 +53
Branches 313 322 +9
==========================================
+ Hits 2200 2230 +30
- Misses 132 154 +22
- Partials 58 59 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
The third and final security finding ported from PR #1, plus a visibility change requested during review.
Problem
An age header must be buffered whole to verify its MAC, so the reader accumulates every header byte before authentication. With no ceiling, a hostile or truncated stream — an unterminated multi-gigabyte line, or an endless run of tiny stanzas — is buffered until the process runs out of memory. Confirmed against a live infinite-stream probe: unpatched, the parser consumed 268 MB+ and was still growing; neither Go
agenor Rustragebounds this either (both buffer unboundedly, ~316 MB / ~110 MB on a 100 MB header line, then reject only for structural reasons). The age spec sets no header-size, argument-length, or recipient-count limit, so this is AgeSharp's own defense.Fix
HeaderReader); cap a single armor line at 64 KiB via a pass-throughNewlineBoundedStreamthat keepsStreamReader.ReadLineon its fast path. Patched, the same probe rejects after ~65 KB in ~20 ms.AgeLimitsconstants (MaxHeaderLineBytes,MaxHeaderBytes,MaxArmorLineBytes), documented in the README, so they're visible and a configurable override has an obvious home later.Bounds are unreachable by real files
Body lines are grammar-capped at 64 chars; the largest built-in stanza line (ML-KEM-768
enc) is ~1.5 KiB; 16 MiB still allows 100,000+ recipients. Only a stanza argument or version line over 64 KiB, or ~130k+ recipients, could trip a limit — inputs no producer creates. Full CCTV vector corpus passes.Verification
375 unit + 143 CCTV vectors pass, zero warnings. New tests cover both per-line caps and the total-header cap (driven directly against
HeaderReader). Ported fromsecurity-hardeningcommits 9591375 + fe617e4; benchmark A/B from that branch (armored decrypt within noise) still applies — the Format files were unchanged since.