From e0b83e008785b617e5fe32782bf9e73dad2f756e Mon Sep 17 00:00:00 2001 From: Tim Ramlot <42113979+inteon@users.noreply.github.com> Date: Fri, 12 Apr 2024 16:32:28 +0200 Subject: [PATCH] BUGFIX: memory was allocated based on encoded length instead of the length of the io.Reader, causing potential DOS due to unexpected high memory usage (max MaxPacketLengthBytes) Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com> --- ber.go | 16 +++++++++++++--- fuzz_test.go | 5 ----- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/ber.go b/ber.go index bafa786..53fe04e 100644 --- a/ber.go +++ b/ber.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "io" + "io/ioutil" "math" "os" "reflect" @@ -352,13 +353,22 @@ func readPacket(reader io.Reader) (*Packet, int, error) { if MaxPacketLengthBytes > 0 && int64(length) > MaxPacketLengthBytes { return nil, read, fmt.Errorf("length %d greater than maximum %d", length, MaxPacketLengthBytes) } - content := make([]byte, length) + + var content []byte if length > 0 { - _, err := io.ReadFull(reader, content) + // Read the content and limit it to the parsed length. + // If the content is less than the length, we return an EOF error. + content, err = ioutil.ReadAll(io.LimitReader(reader, int64(length))) + if err == nil && len(content) < int(length) { + err = io.EOF + } if err != nil { return nil, read, unexpectedEOF(err) } - read += length + read += len(content) + } else { + // If length == 0, we set the ByteValue to an empty slice + content = make([]byte, 0) } if p.ClassType == ClassUniversal { diff --git a/fuzz_test.go b/fuzz_test.go index 0a6d0dd..f914312 100644 --- a/fuzz_test.go +++ b/fuzz_test.go @@ -30,11 +30,6 @@ func FuzzDecodePacket(f *testing.F) { f.Add([]byte{0x09, 0x02, 0x85, 0x30}) f.Add([]byte{0x09, 0x01, 0xcf}) - // Set a limit on the length decoded in readPacket() since the call to - // make([]byte, length) can allocate up to MaxPacketLengthBytes which is - // currently 2 GB. This can cause memory related crashes when fuzzing in - // parallel or on memory constrained devices. - MaxPacketLengthBytes = 65536 f.Fuzz(func(t *testing.T, data []byte) { stime := time.Now() p, err := DecodePacketErr(data)