Repository: zeromicro/go-zero
Pull request #3582 (2023-09-21 13:58:27)

fix: avoid integer overflow in mapping.Unmarshal

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/zeromicro/go-zero/pull/3582?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) Report
> Merging [#3582](https://app.codecov.io/gh/zeromicro/go-zero/pull/3582?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (b3cdaa5) into [master](https://app.codecov.io/gh/zeromicro/go-zero/commit/18d66a795d5fee00960850fb9aec05db380c2566?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (18d66a7) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.


| [Files Changed](https://app.codecov.io/gh/zeromicro/go-zero/pull/3582?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | |
|---|---|---|
| [core/mapping/unmarshaler.go](https://app.codecov.io/gh/zeromicro/go-zero/pull/3582?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-Y29yZS9tYXBwaW5nL3VubWFyc2hhbGVyLmdv) | `96.39% <100.00%> (-0.06%)` | :arrow_down: |
| [core/mapping/utils.go](https://app.codecov.io/gh/zeromicro/go-zero/pull/3582?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-Y29yZS9tYXBwaW5nL3V0aWxzLmdv) | `100.00% <100.00%> (ø)` | |

zcong1993: LGTM

Patch:
diff --git a/core/mapping/unmarshaler.go b/core/mapping/unmarshaler.go
index c0912e9d..54ca804a 100644
--- a/core/mapping/unmarshaler.go
+++ b/core/mapping/unmarshaler.go
@@ -609,24 +609,11 @@ func (u *Unmarshaler) processFieldPrimitiveWithJSONNumber(fieldType reflect.Type
 	target := reflect.New(Deref(fieldType)).Elem()
 
 	switch typeKind {
-	case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
-		iValue, err := v.Int64()
-		if err != nil {
-			return err
-		}
-
-		target.SetInt(iValue)
-	case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64:
-		iValue, err := v.Int64()
-		if err != nil {
+	case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64,
+		reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64:
+		if err := setValueFromString(typeKind, target, v.String()); err != nil {
 			return err
 		}
-
-		if iValue < 0 {
-			return fmt.Errorf("unmarshal %q with bad value %q", fullName, v.String())
-		}
-
-		target.SetUint(uint64(iValue))
 	case reflect.Float32, reflect.Float64:
 		fValue, err := v.Float64()
 		if err != nil {
diff --git a/core/mapping/utils.go b/core/mapping/utils.go
index 3597c97e..b041d8d0 100644
--- a/core/mapping/utils.go
+++ b/core/mapping/utils.go
@@ -34,6 +34,7 @@ const (
 
 var (
 	errUnsupportedType  = errors.New("unsupported type on setting field value")
+	errNumberOverflow   = errors.New("integer overflow")
 	errNumberRange      = errors.New("wrong number range setting")
 	optionsCache        = make(map[string]optionsCacheValue)
 	cacheLock           sync.RWMutex
@@ -482,22 +483,61 @@ func parseSegments(val string) []string {
 	return segments
 }
 
+func setIntValue(value reflect.Value, v any, min, max int64) error {
+	iv := v.(int64)
+	if iv < min || iv > max {
+		return errNumberOverflow
+	}
+
+	value.SetInt(iv)
+	return nil
+}
+
 func setMatchedPrimitiveValue(kind reflect.Kind, value reflect.Value, v any) error {
 	switch kind {
 	case reflect.Bool:
 		value.SetBool(v.(bool))
-	case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
+		return nil
+	case reflect.Int: // int depends on int size, 32 or 64
+		return setIntValue(value, v, math.MinInt, math.MaxInt)
+	case reflect.Int8:
+		return setIntValue(value, v, math.MinInt8, math.MaxInt8)
+	case reflect.Int16:
+		return setIntValue(value, v, math.MinInt16, math.MaxInt16)
+	case reflect.Int32:
+		return setIntValue(value, v, math.MinInt32, math.MaxInt32)
+	case reflect.Int64:
 		value.SetInt(v.(int64))
-	case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64:
+		return nil
+	case reflect.Uint: // uint depends on int size, 32 or 64
+		return setUintValue(value, v, math.MaxUint)
+	case reflect.Uint8:
+		return setUintValue(value, v, math.MaxUint8)
+	case reflect.Uint16:
+		return setUintValue(value, v, math.MaxUint16)
+	case reflect.Uint32:
+		return setUintValue(value, v, math.MaxUint32)
+	case reflect.Uint64:
 		value.SetUint(v.(uint64))
+		return nil
 	case reflect.Float32, reflect.Float64:
 		value.SetFloat(v.(float64))
+		return nil
 	case reflect.String:
 		value.SetString(v.(string))
+		return nil
 	default:
 		return errUnsupportedType
 	}
+}
+
+func setUintValue(value reflect.Value, v any, boundary uint64) error {
+	iv := v.(uint64)
+	if iv > boundary {
+		return errNumberOverflow
+	}
 
+	value.SetUint(iv)
 	return nil
 }
 


Test patch:
diff --git a/core/mapping/unmarshaler_test.go b/core/mapping/unmarshaler_test.go
index a2ea5cff..b39c6ab8 100644
--- a/core/mapping/unmarshaler_test.go
+++ b/core/mapping/unmarshaler_test.go
@@ -569,6 +569,330 @@ func TestUnmarshalIntWithString(t *testing.T) {
 	})
 }
 
+func TestUnmarshalInt8WithOverflow(t *testing.T) {
+	t.Run("int8 from string", func(t *testing.T) {
+		type inner struct {
+			Value int8 `key:"int,string"`
+		}
+
+		m := map[string]any{
+			"int": "8589934592", // overflow
+		}
+
+		var in inner
+		assert.Error(t, UnmarshalKey(m, &in))
+	})
+
+	t.Run("int8 from json.Number", func(t *testing.T) {
+		type inner struct {
+			Value int8 `key:"int"`
+		}
+
+		m := map[string]any{
+			"int": json.Number("8589934592"), // overflow
+		}
+
+		var in inner
+		assert.Error(t, UnmarshalKey(m, &in))
+	})
+
+	t.Run("int8 from json.Number", func(t *testing.T) {
+		type inner struct {
+			Value int8 `key:"int"`
+		}
+
+		m := map[string]any{
+			"int": json.Number("-8589934592"), // overflow
+		}
+
+		var in inner
+		assert.Error(t, UnmarshalKey(m, &in))
+	})
+
+	t.Run("int8 from int64", func(t *testing.T) {
+		type inner struct {
+			Value int8 `key:"int"`
+		}
+
+		m := map[string]any{
+			"int": int64(1) << 36, // overflow
+		}
+
+		var in inner
+		assert.Error(t, UnmarshalKey(m, &in))
+	})
+}
+
+func TestUnmarshalInt16WithOverflow(t *testing.T) {
+	t.Run("int16 from string", func(t *testing.T) {
+		type inner struct {
+			Value int16 `key:"int,string"`
+		}
+
+		m := map[string]any{
+			"int": "8589934592", // overflow
+		}
+
+		var in inner
+		assert.Error(t, UnmarshalKey(m, &in))
+	})
+
+	t.Run("int16 from json.Number", func(t *testing.T) {
+		type inner struct {
+			Value int16 `key:"int"`
+		}
+
+		m := map[string]any{
+			"int": json.Number("8589934592"), // overflow
+		}
+
+		var in inner
+		assert.Error(t, UnmarshalKey(m, &in))
+	})
+
+	t.Run("int16 from json.Number", func(t *testing.T) {
+		type inner struct {
+			Value int16 `key:"int"`
+		}
+
+		m := map[string]any{
+			"int": json.Number("-8589934592"), // overflow
+		}
+
+		var in inner
+		assert.Error(t, UnmarshalKey(m, &in))
+	})
+
+	t.Run("int16 from int64", func(t *testing.T) {
+		type inner struct {
+			Value int16 `key:"int"`
+		}
+
+		m := map[string]any{
+			"int": int64(1) << 36, // overflow
+		}
+
+		var in inner
+		assert.Error(t, UnmarshalKey(m, &in))
+	})
+}
+
+func TestUnmarshalInt32WithOverflow(t *testing.T) {
+	t.Run("int32 from string", func(t *testing.T) {
+		type inner struct {
+			Value int32 `key:"int,string"`
+		}
+
+		m := map[string]any{
+			"int": "8589934592", // overflow
+		}
+
+		var in inner
+		assert.Error(t, UnmarshalKey(m, &in))
+	})
+
+	t.Run("int32 from json.Number", func(t *testing.T) {
+		type inner struct {
+			Value int32 `key:"int"`
+		}
+
+		m := map[string]any{
+			"int": json.Number("8589934592"), // overflow
+		}
+
+		var in inner
+		assert.Error(t, UnmarshalKey(m, &in))
+	})
+
+	t.Run("int32 from json.Number", func(t *testing.T) {
+		type inner struct {
+			Value int32 `key:"int"`
+		}
+
+		m := map[string]any{
+			"int": json.Number("-8589934592"), // overflow
+		}
+
+		var in inner
+		assert.Error(t, UnmarshalKey(m, &in))
+	})
+
+	t.Run("int32 from int64", func(t *testing.T) {
+		type inner struct {
+			Value int32 `key:"int"`
+		}
+
+		m := map[string]any{
+			"int": int64(1) << 36, // overflow
+		}
+
+		var in inner
+		assert.Error(t, UnmarshalKey(m, &in))
+	})
+}
+
+func TestUnmarshalUint8WithOverflow(t *testing.T) {
+	t.Run("uint8 from string", func(t *testing.T) {
+		type inner struct {
+			Value uint8 `key:"int,string"`
+		}
+
+		m := map[string]any{
+			"int": "8589934592", // overflow
+		}
+
+		var in inner
+		assert.Error(t, UnmarshalKey(m, &in))
+	})
+
+	t.Run("uint8 from json.Number", func(t *testing.T) {
+		type inner struct {
+			Value uint8 `key:"int"`
+		}
+
+		m := map[string]any{
+			"int": json.Number("8589934592"), // overflow
+		}
+
+		var in inner
+		assert.Error(t, UnmarshalKey(m, &in))
+	})
+
+	t.Run("uint8 from json.Number", func(t *testing.T) {
+		type inner struct {
+			Value uint8 `key:"int"`
+		}
+
+		m := map[string]any{
+			"int": json.Number("-1"), // overflow
+		}
+
+		var in inner
+		assert.Error(t, UnmarshalKey(m, &in))
+	})
+
+	t.Run("uint8 from int64", func(t *testing.T) {
+		type inner struct {
+			Value uint8 `key:"int"`
+		}
+
+		m := map[string]any{
+			"int": int64(1) << 36, // overflow
+		}
+
+		var in inner
+		assert.Error(t, UnmarshalKey(m, &in))
+	})
+}
+
+func TestUnmarshalUint16WithOverflow(t *testing.T) {
+	t.Run("uint16 from string", func(t *testing.T) {
+		type inner struct {
+			Value uint16 `key:"int,string"`
+		}
+
+		m := map[string]any{
+			"int": "8589934592", // overflow
+		}
+
+		var in inner
+		assert.Error(t, UnmarshalKey(m, &in))
+	})
+
+	t.Run("uint16 from json.Number", func(t *testing.T) {
+		type inner struct {
+			Value uint16 `key:"int"`
+		}
+
+		m := map[string]any{
+			"int": json.Number("8589934592"), // overflow
+		}
+
+		var in inner
+		assert.Error(t, UnmarshalKey(m, &in))
+	})
+
+	t.Run("uint16 from json.Number", func(t *testing.T) {
+		type inner struct {
+			Value uint16 `key:"int"`
+		}
+
+		m := map[string]any{
+			"int": json.Number("-1"), // overflow
+		}
+
+		var in inner
+		assert.Error(t, UnmarshalKey(m, &in))
+	})
+
+	t.Run("uint16 from int64", func(t *testing.T) {
+		type inner struct {
+			Value uint16 `key:"int"`
+		}
+
+		m := map[string]any{
+			"int": int64(1) << 36, // overflow
+		}
+
+		var in inner
+		assert.Error(t, UnmarshalKey(m, &in))
+	})
+}
+
+func TestUnmarshalUint32WithOverflow(t *testing.T) {
+	t.Run("uint32 from string", func(t *testing.T) {
+		type inner struct {
+			Value uint32 `key:"int,string"`
+		}
+
+		m := map[string]any{
+			"int": "8589934592", // overflow
+		}
+
+		var in inner
+		assert.Error(t, UnmarshalKey(m, &in))
+	})
+
+	t.Run("uint32 from json.Number", func(t *testing.T) {
+		type inner struct {
+			Value uint32 `key:"int"`
+		}
+
+		m := map[string]any{
+			"int": json.Number("8589934592"), // overflow
+		}
+
+		var in inner
+		assert.Error(t, UnmarshalKey(m, &in))
+	})
+
+	t.Run("uint32 from json.Number", func(t *testing.T) {
+		type inner struct {
+			Value uint32 `key:"int"`
+		}
+
+		m := map[string]any{
+			"int": json.Number("-1"), // overflow
+		}
+
+		var in inner
+		assert.Error(t, UnmarshalKey(m, &in))
+	})
+
+	t.Run("uint32 from int64", func(t *testing.T) {
+		type inner struct {
+			Value uint32 `key:"int"`
+		}
+
+		m := map[string]any{
+			"int": int64(1) << 36, // overflow
+		}
+
+		var in inner
+		assert.Error(t, UnmarshalKey(m, &in))
+	})
+}
+
 func TestUnmarshalBoolSliceRequired(t *testing.T) {
 	type inner struct {
 		Bools []bool `key:"bools"`