Security hardening: plugin name validation, CSPRNG passphrases, parser bounds#1
Open
pscheid92 wants to merge 8 commits into
Open
Security hardening: plugin name validation, CSPRNG passphrases, parser bounds#1pscheid92 wants to merge 8 commits into
pscheid92 wants to merge 8 commits into
Conversation
A crafted recipient/identity string could smuggle path separators
through the bech32 HRP into the age-plugin-{name} executable path,
redirecting Process.Start to an attacker-chosen binary. Restrict
plugin names to [a-z0-9-] both at parse time and at the spawn site.
System.Random is predictable; the generated passphrase is the sole secret protecting the encrypted file.
The RejectScryptIdentity sentinel was only inserted when an identity file yielded a ScryptRecipient, which never happens — so the friendly "use -p instead" error was dead code and users got a generic "no identity matched". Always try the sentinel first; it also runs before any plugin identity would be spawned.
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.
The encrypt path accepted any work factor: values >= 31 overflowed 1 << workFactor, and anything above 20 produced files this library's own decrypt cap refuses to read back. Enforce [1, 20] eagerly, matching the existing decrypt-side maximum.
The whole-stream Encrypt/Decrypt helpers buffered the entire input in a MemoryStream and truncated lengths above 2 GiB via an int cast. They now delegate to EncryptStream/DecryptStream, which are byte-for-byte identical and memory-bounded. Also drop the unused InspectCommand.Error.
Re-measured microbenchmarks and the CLI comparison on Apple M2. Armored decrypt stays within noise of the pre-hardening baseline (verified via stashed A/B runs with matched conditions). Also corrects allocation figures that no longer matched the unmodified baseline on the current .NET 10 runtime.
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
Both branches of the newline walk threw the same exception; computing the line length once lets a single check cover them.
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
2 participants
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
Fixes from a full security/bug audit of the library, CLI, and parsers. Six findings, each in its own commit, plus a benchmark refresh.
Security
2ae73aa). A crafted recipient/identity string could smuggle path separators through the bech32 HRP into theage-plugin-{name}executable path, letting a hostile recipients file redirectProcess.Startto an attacker-chosen binary. Names are now restricted to[a-z0-9-]at parse time and again at the spawn site. Hyphenated plugins likeage-plugin-fido2-hmacstill work.cacd850).age -pwith an empty passphrase generated the file's sole secret withSystem.Random; nowRandomNumberGenerator.9591375). A hostile stream with an unterminated multi-gigabyte line was buffered in full before any check could reject it. Header lines are capped at 64 KiB (16 MiB total header); armor lines are bounded by a pass-through stream soStreamReader.ReadLinekeeps its fast path.Bugs
age -d -ion a passphrase-encrypted file now says "use -p instead" (2f6c30f) — the sentinel that produced this error was dead code and users got a generic "no identity matched".33a3040) — values ≥ 31 overflowed1 << workFactor, and anything above 20 produced files the library's own decrypt cap refuses to read back.StreamEncryptionhelpers no longer buffer the whole input (8e84123) — they delegated-buffered via a 2 GiB-capped int cast; now they route through the memory-boundedEncryptStream/DecryptStream.Verification
docs/BENCHMARKS.mdregenerated (67a2386), including the CLI comparison against age v1.3.1 / rage v0.11.1 with a fresh AOT build. Headline results unchanged. Also corrects stale allocation figures that no longer matched the unmodified baseline on the current .NET 10 runtime.🤖 Generated with Claude Code