fix(implant): bound pivot frame allocation from length prefix - #2292
Merged
Conversation
Both pivot readers (NetConnPivot and NetConnPivotClient) allocated make([]byte, dataLength) directly from a peer's 4-byte length prefix with only a lower-bound (<= 0) check. A corrupted or desynced prefix -- e.g. from a framing desync during a transfer over a pivot -- can therefore be read as a value up to ~4GiB and trigger an allocation large enough to crash the implant (reported in BishopFox#1452 with a 300KiB download over an http -> named-pipe pivot). Add an upper bound (MaxFrameLength, 512MiB) and reject oversized frames with ErrFrameTooLarge before allocating. The constant and error live in the pivots package and are reused by pivotclients, which already imports it. Adds tests covering rejection of an oversized prefix (with a timeout guard so a regression fails fast instead of hanging) and a valid-frame round-trip.
5 tasks
moloch--
pushed a commit
that referenced
this pull request
Aug 5, 2026
…h prefix Same vulnerability fixed for pivots in #2292: a corrupted or desynced 4-byte length prefix could trigger an allocation up to ~4 GiB, crashing the implant. Cap at 512 MiB (matching the pivot transport limit) and return a descriptive error instead of allocating. Also fixes stale "[pivot]" log tags in both transports' zero-length error paths.
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.
Problem
Both pivot readers —
NetConnPivot.read()(implant/sliver/pivots/pivots.go) andNetConnPivotClient.read()(implant/sliver/transports/pivotclients/pivotclient.go) — read a 4-byte little-endian length prefix and thenmake([]byte, dataLength)with only a lower-bound (<= 0) check. A corrupted or desynced prefix (e.g. a framing desync during a transfer over a pivot) is therefore read as a value up to ~4 GiB and triggers an allocation large enough to crash the implant.This matches the failure mode reported in #1452 ("Downloading 300KiB file over http->named pipe pivot breaks implant and fails") — the tiny payload rules out a legitimately-large frame, pointing at an oversized/garbage declared length.
Change
MaxFrameLength(512 MiB) and reject frames whose declared length exceeds it with a newErrFrameTooLarge, before allocating.pivotspackage and reused bypivotclients(which already imports it).Testing
ErrFrameTooLarge(with a timeout guard so a regression fails fast instead of hanging), and a valid frame still round-trips unchanged.go test ./implant/sliver/pivots/... ./implant/sliver/transports/pivotclients/...pass;go vetandgofmtclean.Note / possible follow-up
The identical unbounded
make([]byte, dataLength)pattern also exists in the implant's mTLS (mtls.go) and WireGuard (wireguard.go) readers. I kept this PR scoped to the pivot path (the one #1452 is about) to keep it small, but I'm happy to extend the same bound to those readers here or in a follow-up — whichever you prefer.