Repository: fxamacker/cbor (go)

Issue:
bug: Validate CBOR tag content type if tag number is 0 or 1, even when TimeTag = DecTagIgnored
### What version of fxamacker/cbor are you using?
v2.2.0

### Does this issue reproduce with the latest release?
Yes.

### What OS and CPU architecture are you using (`go env`)?

<details><summary><code>go env</code> Output</summary><br><pre>
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/user/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/user/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build282002194=/tmp/go-build -gno-record-gcc-switches"
</pre></details>

### What did you do?

Decode CBOR time tag (tag number 0 or 1) with inappropriate content type to time.Time.  

Decoding option for TimeTag is DecTagIgnored by default.

```
func main() {		
	// CBOR tag 0 should be followed by string content; otherwise, it is invalid.
	cborData, _ := hex.DecodeString("c01a514b67b0") // 0(1363896240)
	var tm time.Time
	if err := cbor.Unmarshal(cborData, &tm); err == nil {
		fmt.Printf("Unmarshal(0x%x) didn't return error\n", cborData)
	}
}
```

### What did you expect to see?
Decoding CBOR time tag (tag number 0 or 1) with inappropriate content type should return error.

### What did you see instead?

When TimeTag = DecTagIgnored, data type validation isn't performed for CBOR time content when tag number is 0 or 1.  It should only skip validation of CBOR time content for other tag numbers.

Decoding CBOR time tag (tag number 0 or 1) with inappropriate content type doesn't return error.


Pull request:
Validate tagged time even if TimeTag = DecTagIgnored
Validate CBOR data type of tagged content if tag number is 0 or 1, even when TimeTag = DecTagIgnored.

Closes #221

Patch:
diff --git a/decode.go b/decode.go
index 22b5c7e..36549d9 100644
--- a/decode.go
+++ b/decode.go
@@ -442,15 +442,6 @@ func (d *decodeState) value(v interface{}) error {
 
 	rv = rv.Elem()
 
-	if rv.Kind() == reflect.Interface && rv.NumMethod() == 0 {
-		// Fast path to decode to empty interface without retrieving typeInfo.
-		iv, err := d.parse()
-		if iv != nil {
-			rv.Set(reflect.ValueOf(iv))
-		}
-		return err
-	}
-
 	return d.parseToValue(rv, getTypeInfo(rv.Type()))
 }
 
@@ -507,6 +498,17 @@ func (d *decodeState) parseToValue(v reflect.Value, tInfo *typeInfo) error { //n
 		}
 	}
 
+	// Check validity of supported built-in tags.
+	if d.nextCBORType() == cborTypeTag {
+		off := d.off
+		_, _, tagNum := d.getHead()
+		if err := validBuiltinTag(tagNum, d.data[d.off]); err != nil {
+			d.skip()
+			return err
+		}
+		d.off = off
+	}
+
 	if tInfo.spclType != specialTypeNone {
 		switch tInfo.spclType {
 		case specialTypeEmptyIface:
@@ -546,13 +548,9 @@ func (d *decodeState) parseToValue(v reflect.Value, tInfo *typeInfo) error { //n
 	t := d.nextCBORType()
 
 	// Skip tag number(s) here to avoid recursion
-	if t == cborTypeTag {
+	for t == cborTypeTag {
 		d.getHead()
 		t = d.nextCBORType()
-		for t == cborTypeTag {
-			d.getHead()
-			t = d.nextCBORType()
-		}
 	}
 
 	switch t {
@@ -655,25 +653,7 @@ func (d *decodeState) parseToTime() (tm time.Time, err error) {
 		} else {
 			// Read tag number
 			_, _, tagNum := d.getHead()
-
-			// Verify tag number (0 or 1) is followed by appropriate tag content type.
-			t = d.nextCBORType()
-			switch tagNum {
-			case 0:
-				// Tag content (date/time text string in RFC 3339 format) must be string type.
-				if t != cborTypeTextString {
-					d.skip()
-					err = errors.New("cbor: tag number 0 must be followed by text string, got " + t.String())
-					return
-				}
-			case 1:
-				// Tag content (epoch date/time) must be uint, int, or float type.
-				if t != cborTypePositiveInt && t != cborTypeNegativeInt && (d.data[d.off] < 0xf9 || d.data[d.off] > 0xfb) {
-					d.skip()
-					err = errors.New("cbor: tag number 1 must be followed by integer or floating-point number, got " + t.String())
-					return
-				}
-			default:
+			if tagNum != 0 && tagNum != 1 {
 				d.skip()
 				err = errors.New("cbor: wrong tag number for time.Time, got " + strconv.Itoa(int(tagNum)) + ", expect 0 or 1")
 				return
@@ -1641,3 +1621,23 @@ func fieldByIndex(v reflect.Value, index []int) (reflect.Value, error) {
 	}
 	return v, nil
 }
+
+// validBuiltinTag checks that supported built-in tag numbers are followed by expected content types.
+func validBuiltinTag(tagNum uint64, contentHead byte) error {
+	t := cborType(contentHead & 0xe0)
+	switch tagNum {
+	case 0:
+		// Tag content (date/time text string in RFC 3339 format) must be string type.
+		if t != cborTypeTextString {
+			return errors.New("cbor: tag number 0 must be followed by text string, got " + t.String())
+		}
+		return nil
+	case 1:
+		// Tag content (epoch date/time) must be uint, int, or float type.
+		if t != cborTypePositiveInt && t != cborTypeNegativeInt && (contentHead < 0xf9 || contentHead > 0xfb) {
+			return errors.New("cbor: tag number 1 must be followed by integer or floating-point number, got " + t.String())
+		}
+		return nil
+	}
+	return nil
+}


Test patch:
diff --git a/decode_test.go b/decode_test.go
index 4f831a0..bb83da9 100644
--- a/decode_test.go
+++ b/decode_test.go
@@ -1924,6 +1924,98 @@ func TestDecodeTimeWithTagError(t *testing.T) {
 	}
 }
 
+func TestDecodeTag0Error(t *testing.T) {
+	cborData := hexDecode("c01a514b67b0") // 0(1363896240)
+	wantErrorMsg := "cbor: tag number 0 must be followed by text string, got positive integer"
+
+	timeTagIgnoredDM, _ := DecOptions{TimeTag: DecTagIgnored}.DecMode()
+	timeTagOptionalDM, _ := DecOptions{TimeTag: DecTagOptional}.DecMode()
+	timeTagRequiredDM, _ := DecOptions{TimeTag: DecTagRequired}.DecMode()
+
+	testCases := []struct {
+		name string
+		dm   DecMode
+	}{
+		{name: "DecTagIgnored", dm: timeTagIgnoredDM},
+		{name: "DecTagOptional", dm: timeTagOptionalDM},
+		{name: "DecTagRequired", dm: timeTagRequiredDM},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			// Decode to interface{}
+			var v interface{}
+			if err := tc.dm.Unmarshal(cborData, &v); err == nil {
+				t.Errorf("Unmarshal(0x%x) didn't return error, want error msg %q", cborData, wantErrorMsg)
+			} else if !strings.Contains(err.Error(), wantErrorMsg) {
+				t.Errorf("Unmarshal(0x%x) returned error %q, want %q", cborData, err, wantErrorMsg)
+			}
+
+			// Decode to time.Time
+			var tm time.Time
+			if err := tc.dm.Unmarshal(cborData, &tm); err == nil {
+				t.Errorf("Unmarshal(0x%x) didn't return error, want error msg %q", cborData, wantErrorMsg)
+			} else if !strings.Contains(err.Error(), wantErrorMsg) {
+				t.Errorf("Unmarshal(0x%x) returned error %q, want %q", cborData, err, wantErrorMsg)
+			}
+
+			// Decode to uint64
+			var ui uint64
+			if err := tc.dm.Unmarshal(cborData, &ui); err == nil {
+				t.Errorf("Unmarshal(0x%x) didn't return error, want error msg %q", cborData, wantErrorMsg)
+			} else if !strings.Contains(err.Error(), wantErrorMsg) {
+				t.Errorf("Unmarshal(0x%x) returned error %q, want %q", cborData, err, wantErrorMsg)
+			}
+		})
+	}
+}
+
+func TestDecodeTag1Error(t *testing.T) {
+	cborData := hexDecode("c174323031332d30332d32315432303a30343a30305a") // 1("2013-03-21T20:04:00Z")
+	wantErrorMsg := "cbor: tag number 1 must be followed by integer or floating-point number, got UTF-8 text string"
+
+	timeTagIgnoredDM, _ := DecOptions{TimeTag: DecTagIgnored}.DecMode()
+	timeTagOptionalDM, _ := DecOptions{TimeTag: DecTagOptional}.DecMode()
+	timeTagRequiredDM, _ := DecOptions{TimeTag: DecTagRequired}.DecMode()
+
+	testCases := []struct {
+		name string
+		dm   DecMode
+	}{
+		{name: "DecTagIgnored", dm: timeTagIgnoredDM},
+		{name: "DecTagOptional", dm: timeTagOptionalDM},
+		{name: "DecTagRequired", dm: timeTagRequiredDM},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			// Decode to interface{}
+			var v interface{}
+			if err := tc.dm.Unmarshal(cborData, &v); err == nil {
+				t.Errorf("Unmarshal(0x%x) didn't return error, want error msg %q", cborData, wantErrorMsg)
+			} else if !strings.Contains(err.Error(), wantErrorMsg) {
+				t.Errorf("Unmarshal(0x%x) returned error %q, want %q", cborData, err, wantErrorMsg)
+			}
+
+			// Decode to time.Time
+			var tm time.Time
+			if err := tc.dm.Unmarshal(cborData, &tm); err == nil {
+				t.Errorf("Unmarshal(0x%x) didn't return error, want error msg %q", cborData, wantErrorMsg)
+			} else if !strings.Contains(err.Error(), wantErrorMsg) {
+				t.Errorf("Unmarshal(0x%x) returned error %q, want %q", cborData, err, wantErrorMsg)
+			}
+
+			// Decode to string
+			var s string
+			if err := tc.dm.Unmarshal(cborData, &s); err == nil {
+				t.Errorf("Unmarshal(0x%x) didn't return error, want error msg %q", cborData, wantErrorMsg)
+			} else if !strings.Contains(err.Error(), wantErrorMsg) {
+				t.Errorf("Unmarshal(0x%x) returned error %q, want %q", cborData, err, wantErrorMsg)
+			}
+		})
+	}
+}
+
 func TestDecodeTimeStreaming(t *testing.T) {
 	// Decoder decodes from mixed invalid and valid time.
 	testCases := []struct {