From e6fdfbb1cb8d01e83649057dbb626c4d8f9d7f28 Mon Sep 17 00:00:00 2001 From: Conner Fromknecht Date: Wed, 7 Aug 2019 18:51:52 -0700 Subject: [PATCH] tlv/truncated: fix decoding bug in DTUint16 and DTUint32 This commit fixes a bug in DTUint16 and DTUint32, which would cause them to read too many bytes from the reader. This is due to the fact that ReadFull was being called on a slice that could be greater than the underlying type. This is not an issue for DTUint64, since the 8-byte buffer corresponds to the maximum possible size of a uint64. The solution is to clamp the buffer to 2 and 4 bytes respectively. A series of tests are also added to exercise these cases. --- tlv/truncated.go | 4 +- tlv/truncated_test.go | 164 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 166 insertions(+), 2 deletions(-) diff --git a/tlv/truncated.go b/tlv/truncated.go index b35fc166a..8ed9cb0d4 100644 --- a/tlv/truncated.go +++ b/tlv/truncated.go @@ -44,7 +44,7 @@ func ETUint16(w io.Writer, val interface{}, buf *[8]byte) error { // be resurrected. An error is returned if val is not a *uint16. func DTUint16(r io.Reader, val interface{}, buf *[8]byte, l uint64) error { if t, ok := val.(*uint16); ok && l <= 2 { - _, err := io.ReadFull(r, buf[2-l:]) + _, err := io.ReadFull(r, buf[2-l:2]) if err != nil { return err } @@ -96,7 +96,7 @@ func ETUint32(w io.Writer, val interface{}, buf *[8]byte) error { // be resurrected. An error is returned if val is not a *uint32. func DTUint32(r io.Reader, val interface{}, buf *[8]byte, l uint64) error { if t, ok := val.(*uint32); ok && l <= 4 { - _, err := io.ReadFull(r, buf[4-l:]) + _, err := io.ReadFull(r, buf[4-l:4]) if err != nil { return err } diff --git a/tlv/truncated_test.go b/tlv/truncated_test.go index 6bc4f5082..d2a34562e 100644 --- a/tlv/truncated_test.go +++ b/tlv/truncated_test.go @@ -1,6 +1,7 @@ package tlv_test import ( + "bytes" "fmt" "testing" @@ -10,26 +11,32 @@ import ( var tuint16Tests = []struct { value uint16 size uint64 + bytes []byte }{ { value: 0x0000, size: 0, + bytes: []byte{}, }, { value: 0x0001, size: 1, + bytes: []byte{0x01}, }, { value: 0x00ff, size: 1, + bytes: []byte{0xff}, }, { value: 0x0100, size: 2, + bytes: []byte{0x01, 0x00}, }, { value: 0xffff, size: 2, + bytes: []byte{0xff, 0xff}, }, } @@ -48,45 +55,96 @@ func TestSizeTUint16(t *testing.T) { } } +// TestTUint16 asserts that ETUint16 outputs the proper encoding of a truncated +// uint16, and that DTUint16 is able to parse the output. +func TestTUint16(t *testing.T) { + var buf [8]byte + for _, test := range tuint16Tests { + if len(test.bytes) != int(test.size) { + t.Fatalf("invalid test case, "+ + "len(bytes)[%d] != size[%d]", + len(test.bytes), test.size) + } + + name := fmt.Sprintf("0x%x", test.value) + t.Run(name, func(t *testing.T) { + var b bytes.Buffer + err := tlv.ETUint16(&b, &test.value, &buf) + if err != nil { + t.Fatalf("unable to encode tuint16: %v", err) + } + + if bytes.Compare(b.Bytes(), test.bytes) != 0 { + t.Fatalf("encoding mismatch, "+ + "expected: %x, got: %x", + test.bytes, b.Bytes()) + } + + var value uint16 + r := bytes.NewReader(b.Bytes()) + err = tlv.DTUint16(r, &value, &buf, test.size) + if err != nil { + t.Fatalf("unable to decode tuint16: %v", err) + } + + if value != test.value { + t.Fatalf("decoded value mismatch, "+ + "expected: %d, got: %d", + test.value, value) + } + }) + } +} + var tuint32Tests = []struct { value uint32 size uint64 + bytes []byte }{ { value: 0x00000000, size: 0, + bytes: []byte{}, }, { value: 0x00000001, size: 1, + bytes: []byte{0x01}, }, { value: 0x000000ff, size: 1, + bytes: []byte{0xff}, }, { value: 0x00000100, size: 2, + bytes: []byte{0x01, 0x00}, }, { value: 0x0000ffff, size: 2, + bytes: []byte{0xff, 0xff}, }, { value: 0x00010000, size: 3, + bytes: []byte{0x01, 0x00, 0x00}, }, { value: 0x00ffffff, size: 3, + bytes: []byte{0xff, 0xff, 0xff}, }, { value: 0x01000000, size: 4, + bytes: []byte{0x01, 0x00, 0x00, 0x00}, }, { value: 0xffffffff, size: 4, + bytes: []byte{0xff, 0xff, 0xff, 0xff}, }, } @@ -105,77 +163,136 @@ func TestSizeTUint32(t *testing.T) { } } +// TestTUint32 asserts that ETUint32 outputs the proper encoding of a truncated +// uint32, and that DTUint32 is able to parse the output. +func TestTUint32(t *testing.T) { + var buf [8]byte + for _, test := range tuint32Tests { + if len(test.bytes) != int(test.size) { + t.Fatalf("invalid test case, "+ + "len(bytes)[%d] != size[%d]", + len(test.bytes), test.size) + } + + name := fmt.Sprintf("0x%x", test.value) + t.Run(name, func(t *testing.T) { + var b bytes.Buffer + err := tlv.ETUint32(&b, &test.value, &buf) + if err != nil { + t.Fatalf("unable to encode tuint32: %v", err) + } + + if bytes.Compare(b.Bytes(), test.bytes) != 0 { + t.Fatalf("encoding mismatch, "+ + "expected: %x, got: %x", + test.bytes, b.Bytes()) + } + + var value uint32 + r := bytes.NewReader(b.Bytes()) + err = tlv.DTUint32(r, &value, &buf, test.size) + if err != nil { + t.Fatalf("unable to decode tuint32: %v", err) + } + + if value != test.value { + t.Fatalf("decoded value mismatch, "+ + "expected: %d, got: %d", + test.value, value) + } + }) + } +} + var tuint64Tests = []struct { value uint64 size uint64 + bytes []byte }{ { value: 0x0000000000000000, size: 0, + bytes: []byte{}, }, { value: 0x0000000000000001, size: 1, + bytes: []byte{0x01}, }, { value: 0x00000000000000ff, size: 1, + bytes: []byte{0xff}, }, { value: 0x0000000000000100, size: 2, + bytes: []byte{0x01, 0x00}, }, { value: 0x000000000000ffff, size: 2, + bytes: []byte{0xff, 0xff}, }, { value: 0x0000000000010000, size: 3, + bytes: []byte{0x01, 0x00, 0x00}, }, { value: 0x0000000000ffffff, size: 3, + bytes: []byte{0xff, 0xff, 0xff}, }, { value: 0x0000000001000000, size: 4, + bytes: []byte{0x01, 0x00, 0x00, 0x00}, }, { value: 0x00000000ffffffff, size: 4, + bytes: []byte{0xff, 0xff, 0xff, 0xff}, }, { value: 0x0000000100000000, size: 5, + bytes: []byte{0x01, 0x00, 0x00, 0x00, 0x00}, }, { value: 0x000000ffffffffff, size: 5, + bytes: []byte{0xff, 0xff, 0xff, 0xff, 0xff}, }, { value: 0x0000010000000000, size: 6, + bytes: []byte{0x01, 0x00, 0x00, 0x00, 0x00, 0x00}, }, { value: 0x0000ffffffffffff, size: 6, + bytes: []byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, }, { value: 0x0001000000000000, size: 7, + bytes: []byte{0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}, }, { value: 0x00ffffffffffffff, size: 7, + bytes: []byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, }, { value: 0x0100000000000000, size: 8, + bytes: []byte{0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}, }, { value: 0xffffffffffffffff, size: 8, + bytes: []byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, }, } @@ -183,6 +300,12 @@ var tuint64Tests = []struct { // along boundary conditions of the input space. func TestSizeTUint64(t *testing.T) { for _, test := range tuint64Tests { + if len(test.bytes) != int(test.size) { + t.Fatalf("invalid test case, "+ + "len(bytes)[%d] != size[%d]", + len(test.bytes), test.size) + } + name := fmt.Sprintf("0x%x", test.value) t.Run(name, func(t *testing.T) { size := tlv.SizeTUint64(test.value) @@ -193,3 +316,44 @@ func TestSizeTUint64(t *testing.T) { }) } } + +// TestTUint64 asserts that ETUint64 outputs the proper encoding of a truncated +// uint64, and that DTUint64 is able to parse the output. +func TestTUint64(t *testing.T) { + var buf [8]byte + for _, test := range tuint64Tests { + if len(test.bytes) != int(test.size) { + t.Fatalf("invalid test case, "+ + "len(bytes)[%d] != size[%d]", + len(test.bytes), test.size) + } + + name := fmt.Sprintf("0x%x", test.value) + t.Run(name, func(t *testing.T) { + var b bytes.Buffer + err := tlv.ETUint64(&b, &test.value, &buf) + if err != nil { + t.Fatalf("unable to encode tuint64: %v", err) + } + + if bytes.Compare(b.Bytes(), test.bytes) != 0 { + t.Fatalf("encoding mismatch, "+ + "expected: %x, got: %x", + test.bytes, b.Bytes()) + } + + var value uint64 + r := bytes.NewReader(b.Bytes()) + err = tlv.DTUint64(r, &value, &buf, test.size) + if err != nil { + t.Fatalf("unable to decode tuint64: %v", err) + } + + if value != test.value { + t.Fatalf("decoded value mismatch, "+ + "expected: %d, got: %d", + test.value, value) + } + }) + } +}