Skip to content

Commit

Permalink
BUGFIX: memory was allocated based on encoded length instead of the l…
Browse files Browse the repository at this point in the history
…ength of the io.Reader, causing potential DOS due to unexpected high memory usage (max MaxPacketLengthBytes) (#42)

Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com>
  • Loading branch information
inteon committed Apr 22, 2024
1 parent 04301b4 commit 5679dfd
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 8 deletions.
16 changes: 13 additions & 3 deletions ber.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"errors"
"fmt"
"io"
"io/ioutil"
"math"
"os"
"reflect"
Expand Down Expand Up @@ -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 {
Expand Down
5 changes: 0 additions & 5 deletions fuzz_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 5679dfd

Please sign in to comment.