From 2825ce0f49be55210d306ba89f4b8c59ec791ca9 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Thu, 7 Feb 2019 15:45:26 -0800 Subject: [PATCH 1/3] add overflow checks in sdk.Int deserialization --- types/int.go | 36 ++++++++++++++++++++++++++---------- types/int_test.go | 20 ++++++++++++++++++++ 2 files changed, 46 insertions(+), 10 deletions(-) diff --git a/types/int.go b/types/int.go index 5c4ff42450a3..f92de8052235 100644 --- a/types/int.go +++ b/types/int.go @@ -2,12 +2,15 @@ package types import ( "encoding/json" + "fmt" "testing" "math/big" "math/rand" ) +const maxBitLen = 255 + func newIntegerFromString(s string) (*big.Int, bool) { return new(big.Int).SetString(s, 0) } @@ -54,9 +57,21 @@ func marshalAmino(i *big.Int) (string, error) { return string(bz), err } +func unmarshalText(i *big.Int, text string) error { + if err := i.UnmarshalText([]byte(text)); err != nil { + return err + } + + if i.BitLen() > maxBitLen { + return fmt.Errorf("integer out of range: %s", text) + } + + return nil +} + // UnmarshalAmino for custom decoding scheme func unmarshalAmino(i *big.Int, text string) (err error) { - return i.UnmarshalText([]byte(text)) + return unmarshalText(i, text) } // MarshalJSON for custom encoding scheme @@ -77,12 +92,13 @@ func unmarshalJSON(i *big.Int, bz []byte) error { if err != nil { return err } - return i.UnmarshalText([]byte(text)) + + return unmarshalText(i, text) } // Int wraps integer with 256 bit range bound // Checks overflow, underflow and division by zero -// Exists in range from -(2^255-1) to 2^255-1 +// Exists in range from -(2^maxBitLen-1) to 2^maxBitLen-1 type Int struct { i *big.Int } @@ -99,7 +115,7 @@ func NewInt(n int64) Int { // NewIntFromBigInt constructs Int from big.Int func NewIntFromBigInt(i *big.Int) Int { - if i.BitLen() > 255 { + if i.BitLen() > maxBitLen { panic("NewIntFromBigInt() out of bound") } return Int{i} @@ -112,7 +128,7 @@ func NewIntFromString(s string) (res Int, ok bool) { return } // Check overflow - if i.BitLen() > 255 { + if i.BitLen() > maxBitLen { ok = false return } @@ -130,7 +146,7 @@ func NewIntWithDecimal(n int64, dec int) Int { i.Mul(big.NewInt(n), exp) // Check overflow - if i.BitLen() > 255 { + if i.BitLen() > maxBitLen { panic("NewIntWithDecimal() out of bound") } return Int{i} @@ -195,7 +211,7 @@ func (i Int) LT(i2 Int) bool { func (i Int) Add(i2 Int) (res Int) { res = Int{add(i.i, i2.i)} // Check overflow - if res.i.BitLen() > 255 { + if res.i.BitLen() > maxBitLen { panic("Int overflow") } return @@ -210,7 +226,7 @@ func (i Int) AddRaw(i2 int64) Int { func (i Int) Sub(i2 Int) (res Int) { res = Int{sub(i.i, i2.i)} // Check overflow - if res.i.BitLen() > 255 { + if res.i.BitLen() > maxBitLen { panic("Int overflow") } return @@ -224,12 +240,12 @@ func (i Int) SubRaw(i2 int64) Int { // Mul multiples two Ints func (i Int) Mul(i2 Int) (res Int) { // Check overflow - if i.i.BitLen()+i2.i.BitLen()-1 > 255 { + if i.i.BitLen()+i2.i.BitLen()-1 > maxBitLen { panic("Int overflow") } res = Int{mul(i.i, i2.i)} // Check overflow if sign of both are same - if res.i.BitLen() > 255 { + if res.i.BitLen() > maxBitLen { panic("Int overflow") } return diff --git a/types/int_test.go b/types/int_test.go index a6c3c900232c..20a2f3f61bf4 100644 --- a/types/int_test.go +++ b/types/int_test.go @@ -631,3 +631,23 @@ func TestSafeSub(t *testing.T) { ) } } + +func TestSerializationOverflow(t *testing.T) { + bx, _ := new(big.Int).SetString("91888242871839275229946405745257275988696311157297823662689937894645226298583", 10) + x := Int{bx} + y := new(Int) + + // require amino deserialization to fail due to overflow + xStr, err := x.MarshalAmino() + require.NoError(t, err) + + err = y.UnmarshalAmino(xStr) + require.Error(t, err) + + // require JSON deserialization to fail due to overflow + bz, err := x.MarshalJSON() + require.NoError(t, err) + + err = y.UnmarshalJSON(bz) + require.Error(t, err) +} From 0bf734cf149c00b91117836b1ad02f3971d36900 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Fri, 8 Feb 2019 14:19:29 -0800 Subject: [PATCH 2/3] add pending log --- PENDING.md | 1 + 1 file changed, 1 insertion(+) diff --git a/PENDING.md b/PENDING.md index e0b211ad3102..a5cf35e11719 100644 --- a/PENDING.md +++ b/PENDING.md @@ -92,6 +92,7 @@ IMPROVEMENTS * \#2509 Sanitize all usage of Dec.RoundInt64() * [\#556](https://github.com/cosmos/cosmos-sdk/issues/556) Increase `BaseApp` test coverage. + * Validate bit length when deserializing `Int` types. * Tendermint From 52de0ec5107e14742bff8a67ac567e32e7ab85a9 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Fri, 8 Feb 2019 14:21:16 -0800 Subject: [PATCH 3/3] add link to pending log entry --- PENDING.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/PENDING.md b/PENDING.md index a5cf35e11719..1d955126dd27 100644 --- a/PENDING.md +++ b/PENDING.md @@ -92,7 +92,8 @@ IMPROVEMENTS * \#2509 Sanitize all usage of Dec.RoundInt64() * [\#556](https://github.com/cosmos/cosmos-sdk/issues/556) Increase `BaseApp` test coverage. - * Validate bit length when deserializing `Int` types. + * [\#3552](https://github.com/cosmos/cosmos-sdk/pull/3552) Validate bit length when + deserializing `Int` types. * Tendermint