pcapgo: reject EPB/PB with CaptureLength exceeding block length#1235
Open
adilburaksen wants to merge 1 commit intogoogle:masterfrom
Open
pcapgo: reject EPB/PB with CaptureLength exceeding block length#1235adilburaksen wants to merge 1 commit intogoogle:masterfrom
adilburaksen wants to merge 1 commit intogoogle:masterfrom
Conversation
NgReader.readPacketHeader() applied a SnapLength bounds-check for Simple Packet Blocks (ngBlockTypeSimplePacket) but not for Enhanced Packet Blocks (ngBlockTypeEnhancedPacket) or legacy Packet Blocks (ngBlockTypePacket). This allowed a crafted pcapng file to cause an unbounded allocation in ReadPacketData() and ZeroCopyReadPacketData() via the unchecked make([]byte, r.ci.CaptureLength) call. Add two guards for EPB and legacy PB: 1. Reject any CaptureLength that exceeds the remaining block length (the block cannot contain more data than its declared size). 2. Clamp CaptureLength against the interface SnapLength when set, consistent with the existing Simple Packet Block handling. Add regression tests TestNgEPBOversizeCaptureLength and TestNgEPBOversizeCaptureLengthZeroCopy that construct a 32-byte pcapng with a 512 MB CaptureLength and verify both read paths return an error instead of attempting the allocation.
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.
Summary
NgReader.readPacketHeader()inpcapgo/ngread.goapplied aSnapLengthbounds-check only forngBlockTypeSimplePacket, leavingngBlockTypeEnhancedPacket(EPB) andngBlockTypePacket(legacy PB) unguarded. A crafted pcapng file could trigger an unbounded allocation inReadPacketData()andZeroCopyReadPacketData()via the subsequentmake([]byte, r.ci.CaptureLength)call.Reproducer: an 80-byte pcapng containing a valid SHB + IDB + EPB with
CaptureLength = 0xFFFFFFFFcauses a 4 GB allocation attempt before returning EOF.Changes
pcapgo/ngread.go: for EPB and legacy PB, validate thatCaptureLengthdoes not exceed the remaining block length before allocating; then clamp against interfaceSnapLengthwhen set (matching existing SPB behaviour).pcapgo/ngread_test.go: addTestNgEPBOversizeCaptureLengthandTestNgEPBOversizeCaptureLengthZeroCopy— each constructs a 32-byte pcapng with a 512 MBCaptureLengthand asserts that both read paths return an error rather than attempting the allocation.Test plan
go test ./pcapgo/...passes (includes new regression tests)TestNgEPBOversizeCaptureLengthandTestNgEPBOversizeCaptureLengthZeroCopypass