Repository: fxamacker/cbor
Pull request #645 (2025-03-26 21:40:51)

Port updated error handling in RawTag.UnmarshalCBOR(), etc. to match cbor.Unmarshal()
This ports PR #636 (in release-2.7 branch) to main branch so it can be included in v2.8.0.

`RawTag.UnmarshalCBOR()` is intended to be called by the codec internally and the codec checks for malformed data before calling it.  However, it is possible for user apps to directly call it, so user apps might provide malformed data which can cause panic.

This PR updates these 3 functions to use same error handling as `cbor.Unmarshal()`:
- `ByteString.UnmarshalCBOR(data)`
- `RawTag.UnmarshalCBOR(data)`
- `SimpleValue.UnmarshalCBOR(data)`

Basically, this adds the same well-formedness checks on input data already done by `cbor.Unmarshal()`, so `UnmarshalCBOR()` will return same error if input data is malformed (not panic).

### Caveats

Unfortunately, this approach means the same data is checked twice for the intended case of the codec calling `UnmarshalCBOR()` internally.  This can be revisited and maybe optimized in the future.

PR #636 passed very brief fuzzing on Sunday, March 16, 2025.  However, the fuzzing needs to be run for longer duration before tagging new release.


Patch:
diff --git a/simplevalue.go b/simplevalue.go
index db1e03a..5c6c668 100644
--- a/simplevalue.go
+++ b/simplevalue.go
@@ -55,6 +55,21 @@ func (sv *SimpleValue) UnmarshalCBOR(data []byte) error {
 
 	d := decoder{data: data, dm: defaultDecMode}
 
+	// Check well-formedness of CBOR data item.
+	// NOTE: well-formedness check here is redundant when
+	// Unmarshal() invokes SimpleValue.UnmarshalCBOR().
+	// However, SimpleValue.UnmarshalCBOR() is exported, so
+	// the codec needs to support same behavior for:
+	// - Unmarshal(data, *SimpleValue)
+	// - SimpleValue.UnmarshalCBOR(data)
+	err := d.wellformed(false, false)
+	if err != nil {
+		return err
+	}
+
+	// Restore decoder offset after well-formedness check.
+	d.off = 0
+
 	typ, ai, val := d.getHead()
 
 	if typ != cborTypePrimitives {
diff --git a/tag.go b/tag.go
index 7e62245..def4c3f 100644
--- a/tag.go
+++ b/tag.go
@@ -38,6 +38,21 @@ func (t *RawTag) UnmarshalCBOR(data []byte) error {
 
 	d := decoder{data: data, dm: defaultDecMode}
 
+	// Check if data is a well-formed CBOR data item.
+	// NOTE: well-formedness check here is redundant when
+	// Unmarshal() invokes RawTag.UnmarshalCBOR().
+	// However, RawTag.UnmarshalCBOR() is exported, so
+	// the codec needs to support same behavior for:
+	// - Unmarshal(data, *RawTag)
+	// - RawTag.UnmarshalCBOR(data)
+	err := d.wellformed(false, false)
+	if err != nil {
+		return err
+	}
+
+	// Restore decoder offset after well-formedness check.
+	d.off = 0
+
 	// Unmarshal tag number.
 	typ, _, num := d.getHead()
 	if typ != cborTypeTag {


Test patch:
diff --git a/bytestring.go b/bytestring.go
index 823bff1..6bc081f 100644
--- a/bytestring.go
+++ b/bytestring.go
@@ -52,6 +52,21 @@ func (bs *ByteString) UnmarshalCBOR(data []byte) error {
 
 	d := decoder{data: data, dm: defaultDecMode}
 
+	// Check well-formedness of CBOR data item.
+	// NOTE: well-formedness check here is redundant when
+	// Unmarshal() invokes ByteString.UnmarshalCBOR().
+	// However, ByteString.UnmarshalCBOR() is exported, so
+	// the codec needs to support same behavior for:
+	// - Unmarshal(data, *ByteString)
+	// - ByteString.UnmarshalCBOR(data)
+	err := d.wellformed(false, false)
+	if err != nil {
+		return err
+	}
+
+	// Restore decoder offset after well-formedness check.
+	d.off = 0
+
 	// Check if CBOR data type is byte string
 	if typ := d.nextCBORType(); typ != cborTypeByteString {
 		return &UnmarshalTypeError{CBORType: typ.String(), GoType: typeByteString.String()}
diff --git a/bytestring_test.go b/bytestring_test.go
index 2f4920e..ae208ae 100644
--- a/bytestring_test.go
+++ b/bytestring_test.go
@@ -3,7 +3,11 @@
 
 package cbor
 
-import "testing"
+import (
+	"io"
+	"strings"
+	"testing"
+)
 
 func TestByteString(t *testing.T) {
 	type s1 struct {
@@ -99,3 +103,110 @@ func TestByteString(t *testing.T) {
 	dm, _ := DecOptions{}.DecMode()
 	testRoundTrip(t, testCases, em, dm)
 }
+
+func TestUnmarshalByteStringOnBadData(t *testing.T) {
+	testCases := []struct {
+		name   string
+		data   []byte
+		errMsg string
+	}{
+		// Empty data
+		{
+			name:   "nil data",
+			data:   nil,
+			errMsg: io.EOF.Error(),
+		},
+		{
+			name:   "empty data",
+			data:   []byte{},
+			errMsg: io.EOF.Error(),
+		},
+
+		// Wrong CBOR types
+		{
+			name:   "uint type",
+			data:   hexDecode("01"),
+			errMsg: "cbor: cannot unmarshal positive integer into Go value of type cbor.ByteString",
+		},
+		{
+			name:   "int type",
+			data:   hexDecode("20"),
+			errMsg: "cbor: cannot unmarshal negative integer into Go value of type cbor.ByteString",
+		},
+		{
+			name:   "string type",
+			data:   hexDecode("60"),
+			errMsg: "cbor: cannot unmarshal UTF-8 text string into Go value of type cbor.ByteString",
+		},
+		{
+			name:   "array type",
+			data:   hexDecode("80"),
+			errMsg: "cbor: cannot unmarshal array into Go value of type cbor.ByteString",
+		},
+		{
+			name:   "map type",
+			data:   hexDecode("a0"),
+			errMsg: "cbor: cannot unmarshal map into Go value of type cbor.ByteString",
+		},
+		{
+			name:   "tag type",
+			data:   hexDecode("c074323031332d30332d32315432303a30343a30305a"),
+			errMsg: "cbor: cannot unmarshal tag into Go value of type cbor.ByteString",
+		},
+		{
+			name:   "float type",
+			data:   hexDecode("f90000"),
+			errMsg: "cbor: cannot unmarshal primitives into Go value of type cbor.ByteString",
+		},
+
+		// Truncated CBOR data
+		{
+			name:   "truncated head",
+			data:   hexDecode("18"),
+			errMsg: io.ErrUnexpectedEOF.Error(),
+		},
+
+		// Truncated CBOR byte string
+		{
+			name:   "truncated byte string",
+			data:   hexDecode("44010203"),
+			errMsg: io.ErrUnexpectedEOF.Error(),
+		},
+
+		// Extraneous CBOR data
+		{
+			name:   "extraneous data",
+			data:   hexDecode("c074323031332d30332d32315432303a30343a30305a00"),
+			errMsg: "cbor: 1 bytes of extraneous data starting at index 22",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			// Test ByteString.UnmarshalCBOR(data)
+			{
+				var v ByteString
+
+				err := v.UnmarshalCBOR(tc.data)
+				if err == nil {
+					t.Errorf("UnmarshalCBOR(%x) didn't return error", tc.data)
+				}
+				if !strings.HasPrefix(err.Error(), tc.errMsg) {
+					t.Errorf("UnmarshalCBOR(%x) returned error %q, want %q", tc.data, err.Error(), tc.errMsg)
+				}
+			}
+			// Test Unmarshal(data, *ByteString), which calls ByteString.UnmarshalCBOR() under the hood
+			{
+				var v ByteString
+
+				err := Unmarshal(tc.data, &v)
+				if err == nil {
+					t.Errorf("UnmarshalCBOR(%x) didn't return error", tc.data)
+				}
+				if !strings.HasPrefix(err.Error(), tc.errMsg) {
+					t.Errorf("UnmarshalCBOR(%x) returned error %q, want %q", tc.data, err.Error(), tc.errMsg)
+				}
+			}
+		})
+	}
+}
diff --git a/simplevalue_test.go b/simplevalue_test.go
index 28d6f1d..1022c59 100644
--- a/simplevalue_test.go
+++ b/simplevalue_test.go
@@ -5,7 +5,9 @@ package cbor
 
 import (
 	"bytes"
+	"io"
 	"reflect"
+	"strings"
 	"testing"
 )
 
@@ -51,6 +53,125 @@ func TestUnmarshalSimpleValue(t *testing.T) {
 	})
 }
 
+func TestUnmarshalSimpleValueOnBadData(t *testing.T) {
+	testCases := []struct {
+		name   string
+		data   []byte
+		errMsg string
+	}{
+		// Empty data
+		{
+			name:   "nil data",
+			data:   nil,
+			errMsg: io.EOF.Error(),
+		},
+		{
+			name:   "empty data",
+			data:   []byte{},
+			errMsg: io.EOF.Error(),
+		},
+
+		// Wrong CBOR types
+		{
+			name:   "uint type",
+			data:   hexDecode("01"),
+			errMsg: "cbor: cannot unmarshal positive integer into Go value of type SimpleValue",
+		},
+		{
+			name:   "int type",
+			data:   hexDecode("20"),
+			errMsg: "cbor: cannot unmarshal negative integer into Go value of type SimpleValue",
+		},
+		{
+			name:   "byte string type",
+			data:   hexDecode("40"),
+			errMsg: "cbor: cannot unmarshal byte string into Go value of type SimpleValue",
+		},
+		{
+			name:   "string type",
+			data:   hexDecode("60"),
+			errMsg: "cbor: cannot unmarshal UTF-8 text string into Go value of type SimpleValue",
+		},
+		{
+			name:   "array type",
+			data:   hexDecode("80"),
+			errMsg: "cbor: cannot unmarshal array into Go value of type SimpleValue",
+		},
+		{
+			name:   "map type",
+			data:   hexDecode("a0"),
+			errMsg: "cbor: cannot unmarshal map into Go value of type SimpleValue",
+		},
+		{
+			name:   "tag type",
+			data:   hexDecode("c074323031332d30332d32315432303a30343a30305a"),
+			errMsg: "cbor: cannot unmarshal tag into Go value of type SimpleValue",
+		},
+		{
+			name:   "float type",
+			data:   hexDecode("f90000"),
+			errMsg: "cbor: cannot unmarshal primitives into Go value of type SimpleValue",
+		},
+
+		// Truncated CBOR data
+		{
+			name:   "truncated head",
+			data:   hexDecode("18"),
+			errMsg: io.ErrUnexpectedEOF.Error(),
+		},
+
+		// Truncated CBOR simple value
+		{
+			name:   "truncated simple value",
+			data:   hexDecode("f8"),
+			errMsg: io.ErrUnexpectedEOF.Error(),
+		},
+
+		// Invalid simple value
+		{
+			name:   "invalid simple value",
+			data:   hexDecode("f800"),
+			errMsg: "cbor: invalid simple value 0 for type primitives",
+		},
+
+		// Extraneous CBOR data
+		{
+			name:   "extraneous data",
+			data:   hexDecode("f4f5"),
+			errMsg: "cbor: 1 bytes of extraneous data starting at index 1",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			// Test SimpleValue.UnmarshalCBOR(data)
+			{
+				var v SimpleValue
+
+				err := v.UnmarshalCBOR(tc.data)
+				if err == nil {
+					t.Errorf("UnmarshalCBOR(%x) didn't return error", tc.data)
+				}
+				if !strings.HasPrefix(err.Error(), tc.errMsg) {
+					t.Errorf("UnmarshalCBOR(%x) returned error %q, want %q", tc.data, err.Error(), tc.errMsg)
+				}
+			}
+			// Test Unmarshal(data, *SimpleValue), which calls SimpleValue.UnmarshalCBOR() under the hood
+			{
+				var v SimpleValue
+
+				err := Unmarshal(tc.data, &v)
+				if err == nil {
+					t.Errorf("UnmarshalCBOR(%x) didn't return error", tc.data)
+				}
+				if !strings.HasPrefix(err.Error(), tc.errMsg) {
+					t.Errorf("UnmarshalCBOR(%x) returned error %q, want %q", tc.data, err.Error(), tc.errMsg)
+				}
+			}
+		})
+	}
+}
+
 func testUnmarshalInvalidSimpleValueToEmptyInterface(t *testing.T, data []byte) {
 	var v any
 	if err := Unmarshal(data, v); err == nil {
diff --git a/tag_test.go b/tag_test.go
index 74d46eb..8d4e9c5 100644
--- a/tag_test.go
+++ b/tag_test.go
@@ -1539,3 +1539,125 @@ func TestEncodeBuiltinTag(t *testing.T) {
 		})
 	}
 }
+
+func TestUnmarshalRawTagOnBadData(t *testing.T) {
+	testCases := []struct {
+		name   string
+		data   []byte
+		errMsg string
+	}{
+		// Empty data
+		{
+			name:   "nil data",
+			data:   nil,
+			errMsg: io.EOF.Error(),
+		},
+		{
+			name:   "empty data",
+			data:   []byte{},
+			errMsg: io.EOF.Error(),
+		},
+
+		// Wrong CBOR types
+		{
+			name:   "uint type",
+			data:   hexDecode("01"),
+			errMsg: "cbor: cannot unmarshal positive integer into Go value of type cbor.RawTag",
+		},
+		{
+			name:   "int type",
+			data:   hexDecode("20"),
+			errMsg: "cbor: cannot unmarshal negative integer into Go value of type cbor.RawTag",
+		},
+		{
+			name:   "byte string type",
+			data:   hexDecode("40"),
+			errMsg: "cbor: cannot unmarshal byte string into Go value of type cbor.RawTag",
+		},
+		{
+			name:   "string type",
+			data:   hexDecode("60"),
+			errMsg: "cbor: cannot unmarshal UTF-8 text string into Go value of type cbor.RawTag",
+		},
+		{
+			name:   "array type",
+			data:   hexDecode("80"),
+			errMsg: "cbor: cannot unmarshal array into Go value of type cbor.RawTag",
+		},
+		{
+			name:   "map type",
+			data:   hexDecode("a0"),
+			errMsg: "cbor: cannot unmarshal map into Go value of type cbor.RawTag",
+		},
+		{
+			name:   "primitive type",
+			data:   hexDecode("f4"),
+			errMsg: "cbor: cannot unmarshal primitives into Go value of type cbor.RawTag",
+		},
+		{
+			name:   "float type",
+			data:   hexDecode("f90000"),
+			errMsg: "cbor: cannot unmarshal primitives into Go value of type cbor.RawTag",
+		},
+
+		// Truncated CBOR data
+		{
+			name:   "truncated head",
+			data:   hexDecode("18"),
+			errMsg: io.ErrUnexpectedEOF.Error(),
+		},
+
+		// Truncated CBOR tag data
+		{
+			name:   "truncated tag number",
+			data:   hexDecode("d8"),
+			errMsg: io.ErrUnexpectedEOF.Error(),
+		},
+		{
+			name:   "tag number not followed by tag content",
+			data:   hexDecode("da"),
+			errMsg: io.ErrUnexpectedEOF.Error(),
+		},
+		{
+			name:   "truncated tag content",
+			data:   hexDecode("c074323031332d30332d32315432303a30343a3030"),
+			errMsg: io.ErrUnexpectedEOF.Error(),
+		},
+
+		// Extraneous CBOR data
+		{
+			name:   "extraneous data",
+			data:   hexDecode("c074323031332d30332d32315432303a30343a30305a00"),
+			errMsg: "cbor: 1 bytes of extraneous data starting at index 22",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			// Test RawTag.UnmarshalCBOR(data)
+			{
+				var v RawTag
+
+				err := v.UnmarshalCBOR(tc.data)
+				if err == nil {
+					t.Errorf("UnmarshalCBOR(%x) didn't return error", tc.data)
+				}
+				if !strings.HasPrefix(err.Error(), tc.errMsg) {
+					t.Errorf("UnmarshalCBOR(%x) returned error %q, want %q", tc.data, err.Error(), tc.errMsg)
+				}
+			}
+			// Test Unmarshal(data, *RawTag), which calls RawTag.UnmarshalCBOR() under the hood
+			{
+				var v RawTag
+
+				err := Unmarshal(tc.data, &v)
+				if err == nil {
+					t.Errorf("UnmarshalCBOR(%x) didn't return error", tc.data)
+				}
+				if !strings.HasPrefix(err.Error(), tc.errMsg) {
+					t.Errorf("UnmarshalCBOR(%x) returned error %q, want %q", tc.data, err.Error(), tc.errMsg)
+				}
+			}
+		})
+	}
+}