Skip to content

Commit 979f9a1

Browse files
committed
Detect truncated packets when handling extended option lengths
When parsing extending option lengths, verify that enough bytes are left in the packet. This fixes a panic that occured when a specially crafted truncated packet was processed. The TestInvalidMessageParsing was refactored to allow easy additions of such cases to the test suite. The bug was found with the help of go-fuzz (https://github.com/dvyukov/go-fuzz).
1 parent f6a4ce0 commit 979f9a1

File tree

2 files changed

+33
-23
lines changed

2 files changed

+33
-23
lines changed

message.go

+16-4
Original file line numberDiff line numberDiff line change
@@ -580,16 +580,22 @@ func (m *Message) UnmarshalBinary(data []byte) error {
580580
b := data[4+tokenLen:]
581581
prev := 0
582582

583-
parseExtOpt := func(opt int) int {
583+
parseExtOpt := func(opt int) (int, error) {
584584
switch opt {
585585
case extoptByteCode:
586+
if len(b) < 1 {
587+
return -1, errors.New("truncated")
588+
}
586589
opt = int(b[0]) + extoptByteAddend
587590
b = b[1:]
588591
case extoptWordCode:
592+
if len(b) < 2 {
593+
return -1, errors.New("truncated")
594+
}
589595
opt = int(binary.BigEndian.Uint16(b[:2])) + extoptWordAddend
590596
b = b[2:]
591597
}
592-
return opt
598+
return opt, nil
593599
}
594600

595601
for len(b) > 0 {
@@ -607,8 +613,14 @@ func (m *Message) UnmarshalBinary(data []byte) error {
607613

608614
b = b[1:]
609615

610-
delta = parseExtOpt(delta)
611-
length = parseExtOpt(length)
616+
delta, err := parseExtOpt(delta)
617+
if err != nil {
618+
return err
619+
}
620+
length, err = parseExtOpt(length)
621+
if err != nil {
622+
return err
623+
}
612624

613625
if len(b) < length {
614626
return errors.New("truncated")

message_test.go

+17-19
Original file line numberDiff line numberDiff line change
@@ -190,25 +190,23 @@ func TestEncodeMessageSmallWithPayload(t *testing.T) {
190190
}
191191

192192
func TestInvalidMessageParsing(t *testing.T) {
193-
msg, err := parseMessage(nil)
194-
if err == nil {
195-
t.Errorf("Unexpected success parsing short message: %v", msg)
196-
}
197-
198-
msg, err = parseMessage([]byte{0xff, 0, 0, 0, 0, 0})
199-
if err == nil {
200-
t.Errorf("Unexpected success parsing invalid message: %v", msg)
201-
}
202-
203-
msg, err = parseMessage([]byte{0x4f, 0, 0, 0, 0, 0})
204-
if err == nil {
205-
t.Errorf("Unexpected success parsing invalid message: %v", msg)
206-
}
207-
208-
// TKL=5 but packet is truncated
209-
msg, err = parseMessage([]byte{0x45, 0, 0, 0, 0, 0})
210-
if err == nil {
211-
t.Errorf("Unexpected success parsing invalid message: %v", msg)
193+
var invalidPackets = [][]byte{
194+
nil,
195+
{0x40},
196+
{0x40, 0},
197+
{0x40, 0, 0},
198+
{0xff, 0, 0, 0, 0, 0},
199+
{0x4f, 0, 0, 0, 0, 0},
200+
{0x45, 0, 0, 0, 0, 0}, // TKL=5 but packet is truncated
201+
{0x40, 0x01, 0x30, 0x39, 0x4d}, // Extended word length but no extra length byte
202+
{0x40, 0x01, 0x30, 0x39, 0x4e, 0x01}, // Extended word length but no full extra length word
203+
}
204+
205+
for _, data := range invalidPackets {
206+
msg, err := parseMessage(data)
207+
if err == nil {
208+
t.Errorf("Unexpected success parsing short message (%#v): %v", data, msg)
209+
}
212210
}
213211
}
214212

0 commit comments

Comments
 (0)