Skip to content

Commit

Permalink
Detect truncated packets when handling extended option lengths
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
dubek committed Oct 12, 2015
1 parent f6a4ce0 commit 979f9a1
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 23 deletions.
20 changes: 16 additions & 4 deletions message.go
Original file line number Diff line number Diff line change
Expand Up @@ -580,16 +580,22 @@ func (m *Message) UnmarshalBinary(data []byte) error {
b := data[4+tokenLen:]
prev := 0

parseExtOpt := func(opt int) int {
parseExtOpt := func(opt int) (int, error) {
switch opt {
case extoptByteCode:
if len(b) < 1 {
return -1, errors.New("truncated")
}
opt = int(b[0]) + extoptByteAddend
b = b[1:]
case extoptWordCode:
if len(b) < 2 {
return -1, errors.New("truncated")
}
opt = int(binary.BigEndian.Uint16(b[:2])) + extoptWordAddend
b = b[2:]
}
return opt
return opt, nil
}

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

b = b[1:]

delta = parseExtOpt(delta)
length = parseExtOpt(length)
delta, err := parseExtOpt(delta)
if err != nil {
return err
}
length, err = parseExtOpt(length)
if err != nil {
return err
}

if len(b) < length {
return errors.New("truncated")
Expand Down
36 changes: 17 additions & 19 deletions message_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,25 +190,23 @@ func TestEncodeMessageSmallWithPayload(t *testing.T) {
}

func TestInvalidMessageParsing(t *testing.T) {
msg, err := parseMessage(nil)
if err == nil {
t.Errorf("Unexpected success parsing short message: %v", msg)
}

msg, err = parseMessage([]byte{0xff, 0, 0, 0, 0, 0})
if err == nil {
t.Errorf("Unexpected success parsing invalid message: %v", msg)
}

msg, err = parseMessage([]byte{0x4f, 0, 0, 0, 0, 0})
if err == nil {
t.Errorf("Unexpected success parsing invalid message: %v", msg)
}

// TKL=5 but packet is truncated
msg, err = parseMessage([]byte{0x45, 0, 0, 0, 0, 0})
if err == nil {
t.Errorf("Unexpected success parsing invalid message: %v", msg)
var invalidPackets = [][]byte{
nil,
{0x40},
{0x40, 0},
{0x40, 0, 0},
{0xff, 0, 0, 0, 0, 0},
{0x4f, 0, 0, 0, 0, 0},
{0x45, 0, 0, 0, 0, 0}, // TKL=5 but packet is truncated
{0x40, 0x01, 0x30, 0x39, 0x4d}, // Extended word length but no extra length byte
{0x40, 0x01, 0x30, 0x39, 0x4e, 0x01}, // Extended word length but no full extra length word
}

for _, data := range invalidPackets {
msg, err := parseMessage(data)
if err == nil {
t.Errorf("Unexpected success parsing short message (%#v): %v", data, msg)
}
}
}

Expand Down

0 comments on commit 979f9a1

Please sign in to comment.