Repository: gocql/gocql
Pull request #1694 (2023-05-04 14:32:32)

Fix potential crashes on deserealization
Fixes https://github.com/gocql/gocql/issues/1693

List of issues:
- `unmarshalUDT`, can cause panic of data is not aligned with udt structure
- `unmarshalMap`, potential wrong result if `readCollectionSize` return negative size
- `unmarshalList`, otential wrong result if `readCollectionSize` return negative size
- `decVint`, potential panic if data is not aligned
- `decVints`, potential panic if data is not aligned
- `unmarshalDecimal`, potential panic if data is not aligned

martin-sucha: Please update AUTHORS and CHANGELOG as per [CONTRIBUTING.md](https://github.com/gocql/gocql/blob/master/CONTRIBUTING.md#minimum-requirement-checklist)
dkropachev: > Please update AUTHORS and CHANGELOG as per [CONTRIBUTING.md](https://github.com/gocql/gocql/blob/master/CONTRIBUTING.md#minimum-requirement-checklist)

Done

Patch:
diff --git a/AUTHORS b/AUTHORS
index 28b008a..9527b03 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -137,3 +137,4 @@ Steven Seidman <steven.seidman@datadoghq.com>
 Wojciech Przytuła <wojciech.przytula@scylladb.com>
 João Reis <joao.reis@datastax.com>
 Lauro Ramos Venancio <lauro.venancio@incognia.com>
+Dmitry Kropachev <dmitry.kropachev@gmail.com>
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 10a430c..448f9ca 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Changed
 
 ### Fixed
+- Potential panic on deserialization (#1695)
 
 ## [1.4.0] - 2023-04-26
 
diff --git a/marshal.go b/marshal.go
index d476449..0ae6806 100644
--- a/marshal.go
+++ b/marshal.go
@@ -1169,6 +1169,9 @@ func unmarshalDecimal(info TypeInfo, data []byte, value interface{}) error {
 	case Unmarshaler:
 		return v.UnmarshalCQL(info, data)
 	case *inf.Dec:
+		if len(data) < 4 {
+			return unmarshalErrorf("inf.Dec needs at least 4 bytes, while value has only %d", len(data))
+		}
 		scale := decInt(data[0:4])
 		unscaled := decBigInt2C(data[4:], nil)
 		*v = *inf.NewDecBig(unscaled, inf.Scale(scale))
@@ -1443,7 +1446,10 @@ func unmarshalDuration(info TypeInfo, data []byte, value interface{}) error {
 			}
 			return nil
 		}
-		months, days, nanos := decVints(data)
+		months, days, nanos, err := decVints(data)
+		if err != nil {
+			return unmarshalErrorf("failed to unmarshal %s into %T: %s", info, value, err.Error())
+		}
 		*v = Duration{
 			Months:      months,
 			Days:        days,
@@ -1454,25 +1460,40 @@ func unmarshalDuration(info TypeInfo, data []byte, value interface{}) error {
 	return unmarshalErrorf("can not unmarshal %s into %T", info, value)
 }
 
-func decVints(data []byte) (int32, int32, int64) {
-	month, i := decVint(data)
-	days, j := decVint(data[i:])
-	nanos, _ := decVint(data[i+j:])
-	return int32(month), int32(days), nanos
+func decVints(data []byte) (int32, int32, int64, error) {
+	month, i, err := decVint(data, 0)
+	if err != nil {
+		return 0, 0, 0, fmt.Errorf("failed to extract month: %s", err.Error())
+	}
+	days, i, err := decVint(data, i)
+	if err != nil {
+		return 0, 0, 0, fmt.Errorf("failed to extract days: %s", err.Error())
+	}
+	nanos, _, err := decVint(data, i)
+	if err != nil {
+		return 0, 0, 0, fmt.Errorf("failed to extract nanoseconds: %s", err.Error())
+	}
+	return int32(month), int32(days), nanos, err
 }
 
-func decVint(data []byte) (int64, int) {
-	firstByte := data[0]
+func decVint(data []byte, start int) (int64, int, error) {
+	if len(data) <= start {
+		return 0, 0, errors.New("unexpected eof")
+	}
+	firstByte := data[start]
 	if firstByte&0x80 == 0 {
-		return decIntZigZag(uint64(firstByte)), 1
+		return decIntZigZag(uint64(firstByte)), start + 1, nil
 	}
 	numBytes := bits.LeadingZeros32(uint32(^firstByte)) - 24
 	ret := uint64(firstByte & (0xff >> uint(numBytes)))
-	for i := 0; i < numBytes; i++ {
+	if len(data) < start+numBytes+1 {
+		return 0, 0, fmt.Errorf("data expect to have %d bytes, but it has only %d", start+numBytes+1, len(data))
+	}
+	for i := start; i < start+numBytes; i++ {
 		ret <<= 8
 		ret |= uint64(data[i+1] & 0xff)
 	}
-	return decIntZigZag(ret), numBytes + 1
+	return decIntZigZag(ret), start + numBytes + 1, nil
 }
 
 func decIntZigZag(n uint64) int64 {
@@ -1648,13 +1669,12 @@ func unmarshalList(info TypeInfo, data []byte, value interface{}) error {
 				return err
 			}
 			data = data[p:]
-			if len(data) < m {
-				return unmarshalErrorf("unmarshal list: unexpected eof")
-			}
-
 			// In case m < 0, the value is null, and unmarshalData should be nil.
 			var unmarshalData []byte
 			if m >= 0 {
+				if len(data) < m {
+					return unmarshalErrorf("unmarshal list: unexpected eof")
+				}
 				unmarshalData = data[:m]
 				data = data[m:]
 			}
@@ -1764,14 +1784,13 @@ func unmarshalMap(info TypeInfo, data []byte, value interface{}) error {
 			return err
 		}
 		data = data[p:]
-		if len(data) < m {
-			return unmarshalErrorf("unmarshal map: unexpected eof")
-		}
 		key := reflect.New(t.Key())
-
 		// In case m < 0, the key is null, and unmarshalData should be nil.
 		var unmarshalData []byte
 		if m >= 0 {
+			if len(data) < m {
+				return unmarshalErrorf("unmarshal map: unexpected eof")
+			}
 			unmarshalData = data[:m]
 			data = data[m:]
 		}
@@ -1784,14 +1803,14 @@ func unmarshalMap(info TypeInfo, data []byte, value interface{}) error {
 			return err
 		}
 		data = data[p:]
-		if len(data) < m {
-			return unmarshalErrorf("unmarshal map: unexpected eof")
-		}
 		val := reflect.New(t.Elem())
 
 		// In case m < 0, the value is null, and unmarshalData should be nil.
 		unmarshalData = nil
 		if m >= 0 {
+			if len(data) < m {
+				return unmarshalErrorf("unmarshal map: unexpected eof")
+			}
 			unmarshalData = data[:m]
 			data = data[m:]
 		}
@@ -2281,14 +2300,16 @@ func unmarshalUDT(info TypeInfo, data []byte, value interface{}) error {
 	case UDTUnmarshaler:
 		udt := info.(UDTTypeInfo)
 
-		for _, e := range udt.Elements {
+		for id, e := range udt.Elements {
 			if len(data) == 0 {
 				return nil
 			}
+			if len(data) < 4 {
+				return unmarshalErrorf("can not unmarshal %s: field [%d]%s: unexpected eof", info, id, e.Name)
+			}
 
 			var p []byte
 			p, data = readBytes(data)
-
 			if err := v.UnmarshalUDT(e.Name, e.Type, p); err != nil {
 				return err
 			}
@@ -2315,10 +2336,13 @@ func unmarshalUDT(info TypeInfo, data []byte, value interface{}) error {
 		rv.Set(reflect.MakeMap(t))
 		m := *v
 
-		for _, e := range udt.Elements {
+		for id, e := range udt.Elements {
 			if len(data) == 0 {
 				return nil
 			}
+			if len(data) < 4 {
+				return unmarshalErrorf("can not unmarshal %s: field [%d]%s: unexpected eof", info, id, e.Name)
+			}
 
 			valType, err := goType(e.Type)
 			if err != nil {
@@ -2368,10 +2392,13 @@ func unmarshalUDT(info TypeInfo, data []byte, value interface{}) error {
 	}
 
 	udt := info.(UDTTypeInfo)
-	for _, e := range udt.Elements {
+	for id, e := range udt.Elements {
+		if len(data) == 0 {
+			return nil
+		}
 		if len(data) < 4 {
 			// UDT def does not match the column value
-			return nil
+			return unmarshalErrorf("can not unmarshal %s: field [%d]%s: unexpected eof", info, id, e.Name)
 		}
 
 		var p []byte


Test patch:
diff --git a/marshal_test.go b/marshal_test.go
index 5df7d4b..f007ab9 100644
--- a/marshal_test.go
+++ b/marshal_test.go
@@ -1264,6 +1264,42 @@ var unmarshalTests = []struct {
 		map[string]int{"foo": 1},
 		UnmarshalError("unmarshal map: unexpected eof"),
 	},
+	{
+		NativeType{proto: 2, typ: TypeDecimal},
+		[]byte("\xff\xff\xff"),
+		inf.NewDec(0, 0), // From the datastax/python-driver test suite
+		UnmarshalError("inf.Dec needs at least 4 bytes, while value has only 3"),
+	},
+	{
+		NativeType{proto: 5, typ: TypeDuration},
+		[]byte("\x89\xa2\xc3\xc2\x9a\xe0F\x91"),
+		Duration{},
+		UnmarshalError("failed to unmarshal duration into *gocql.Duration: failed to extract nanoseconds: data expect to have 9 bytes, but it has only 8"),
+	},
+	{
+		NativeType{proto: 5, typ: TypeDuration},
+		[]byte("\x89\xa2\xc3\xc2\x9a"),
+		Duration{},
+		UnmarshalError("failed to unmarshal duration into *gocql.Duration: failed to extract nanoseconds: unexpected eof"),
+	},
+	{
+		NativeType{proto: 5, typ: TypeDuration},
+		[]byte("\x89\xa2\xc3\xc2"),
+		Duration{},
+		UnmarshalError("failed to unmarshal duration into *gocql.Duration: failed to extract days: data expect to have 5 bytes, but it has only 4"),
+	},
+	{
+		NativeType{proto: 5, typ: TypeDuration},
+		[]byte("\x89\xa2"),
+		Duration{},
+		UnmarshalError("failed to unmarshal duration into *gocql.Duration: failed to extract days: unexpected eof"),
+	},
+	{
+		NativeType{proto: 5, typ: TypeDuration},
+		[]byte("\x89"),
+		Duration{},
+		UnmarshalError("failed to unmarshal duration into *gocql.Duration: failed to extract month: data expect to have 2 bytes, but it has only 1"),
+	},
 }
 
 func decimalize(s string) *inf.Dec {