diff --git a/CHANGELOG.md b/CHANGELOG.md index bfdc4a0e4..3a09f621c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,10 +1,16 @@ # Changelog +## 0.15.2 (December 14, 2020) + +### Bug Fixes + +- [\#347](https://github.com/cosmos/iavl/pull/347) Fix another integer overflow in `decodeBytes()` that can cause panics for certain inputs. The `ValueOp` and `AbsenceOp` proof decoders are vulnerable to this via malicious inputs since 0.15.0. + ## 0.15.1 (December 13, 2020) ### Bug Fixes -- [\#340](https://github.com/cosmos/iavl/pull/340) Fix integer overflow in `decodeBytes()` that can cause out-of-memory errors on 32-bit machines for certain inputs. +[\#340](https://github.com/cosmos/iavl/pull/340) Fix integer overflow in `decodeBytes()` that can cause panics on 64-bit systems and out-of-memory issues on 32-bit systems. The `ValueOp` and `AbsenceOp` proof decoders are vulnerable to this via malicious inputs. The bug was introduced in 0.15.0. ## 0.15.0 (November 23, 2020) diff --git a/encoding.go b/encoding.go index 2e51d5a29..6068fe1eb 100644 --- a/encoding.go +++ b/encoding.go @@ -16,18 +16,25 @@ func decodeBytes(bz []byte) ([]byte, int, error) { if err != nil { return nil, n, err } - // ^uint(0) >> 1 will help determine the max int value variably on 32-bit and 64-bit machines. - if uint64(n)+s >= uint64(^uint(0)>>1) { - return nil, n, fmt.Errorf("invalid out of range length %v decoding []byte", uint64(n)+s) - } + // Make sure size doesn't overflow. ^uint(0) >> 1 will help determine the + // max int value variably on 32-bit and 64-bit machines. We also doublecheck + // that size is positive. size := int(s) - if len(bz) < n+size { + if s >= uint64(^uint(0)>>1) || size < 0 { + return nil, n, fmt.Errorf("invalid out of range length %v decoding []byte", s) + } + // Make sure end index doesn't overflow. We know n>0 from decodeUvarint(). + end := n + size + if end < n { + return nil, n, fmt.Errorf("invalid out of range length %v decoding []byte", size) + } + // Make sure the end index is within bounds. + if len(bz) < end { return nil, n, fmt.Errorf("insufficient bytes decoding []byte of length %v", size) } bz2 := make([]byte, size) - copy(bz2, bz[n:n+size]) - n += size - return bz2, n, nil + copy(bz2, bz[n:end]) + return bz2, end, nil } // decodeUvarint decodes a varint-encoded unsigned integer from a byte slice, returning it and the diff --git a/encoding_test.go b/encoding_test.go new file mode 100644 index 000000000..6fc32374f --- /dev/null +++ b/encoding_test.go @@ -0,0 +1,88 @@ +package iavl + +import ( + "encoding/binary" + "math" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestDecodeBytes(t *testing.T) { + bz := []byte{0, 1, 2, 3, 4, 5, 6, 7} + testcases := map[string]struct { + bz []byte + lengthPrefix uint64 + expect []byte + expectErr bool + }{ + "full": {bz, 8, bz, false}, + "empty": {bz, 0, []byte{}, false}, + "partial": {bz, 3, []byte{0, 1, 2}, false}, + "out of bounds": {bz, 9, nil, true}, + "empty input": {[]byte{}, 0, []byte{}, false}, + "empty input out of bounds": {[]byte{}, 1, nil, true}, + + // The following will always fail, since the byte slice is only 8 bytes, + // but we're making sure they don't panic due to overflow issues. See: + // https://github.com/cosmos/iavl/issues/339 + "max int32": {bz, uint64(math.MaxInt32), nil, true}, + "max int32 -1": {bz, uint64(math.MaxInt32) - 1, nil, true}, + "max int32 -10": {bz, uint64(math.MaxInt32) - 10, nil, true}, + "max int32 +1": {bz, uint64(math.MaxInt32) + 1, nil, true}, + "max int32 +10": {bz, uint64(math.MaxInt32) + 10, nil, true}, + + "max int32*2": {bz, uint64(math.MaxInt32) * 2, nil, true}, + "max int32*2 -1": {bz, uint64(math.MaxInt32)*2 - 1, nil, true}, + "max int32*2 -10": {bz, uint64(math.MaxInt32)*2 - 10, nil, true}, + "max int32*2 +1": {bz, uint64(math.MaxInt32)*2 + 1, nil, true}, + "max int32*2 +10": {bz, uint64(math.MaxInt32)*2 + 10, nil, true}, + + "max uint32": {bz, uint64(math.MaxUint32), nil, true}, + "max uint32 -1": {bz, uint64(math.MaxUint32) - 1, nil, true}, + "max uint32 -10": {bz, uint64(math.MaxUint32) - 10, nil, true}, + "max uint32 +1": {bz, uint64(math.MaxUint32) + 1, nil, true}, + "max uint32 +10": {bz, uint64(math.MaxUint32) + 10, nil, true}, + + "max uint32*2": {bz, uint64(math.MaxUint32) * 2, nil, true}, + "max uint32*2 -1": {bz, uint64(math.MaxUint32)*2 - 1, nil, true}, + "max uint32*2 -10": {bz, uint64(math.MaxUint32)*2 - 10, nil, true}, + "max uint32*2 +1": {bz, uint64(math.MaxUint32)*2 + 1, nil, true}, + "max uint32*2 +10": {bz, uint64(math.MaxUint32)*2 + 10, nil, true}, + + "max int64": {bz, uint64(math.MaxInt64), nil, true}, + "max int64 -1": {bz, uint64(math.MaxInt64) - 1, nil, true}, + "max int64 -10": {bz, uint64(math.MaxInt64) - 10, nil, true}, + "max int64 +1": {bz, uint64(math.MaxInt64) + 1, nil, true}, + "max int64 +10": {bz, uint64(math.MaxInt64) + 10, nil, true}, + + "max uint64": {bz, uint64(math.MaxUint64), nil, true}, + "max uint64 -1": {bz, uint64(math.MaxUint64) - 1, nil, true}, + "max uint64 -10": {bz, uint64(math.MaxUint64) - 10, nil, true}, + } + for name, tc := range testcases { + tc := tc + t.Run(name, func(t *testing.T) { + // Generate an input slice. + buf := make([]byte, binary.MaxVarintLen64) + varintBytes := binary.PutUvarint(buf, tc.lengthPrefix) + buf = append(buf[:varintBytes], tc.bz...) + + // Attempt to decode it. + b, n, err := decodeBytes(buf) + if tc.expectErr { + require.Error(t, err) + require.Equal(t, varintBytes, n) + } else { + require.NoError(t, err) + require.Equal(t, uint64(n), uint64(varintBytes)+tc.lengthPrefix) + require.Equal(t, tc.bz[:tc.lengthPrefix], b) + } + }) + } +} + +func TestDecodeBytes_invalidVarint(t *testing.T) { + _, _, err := decodeBytes([]byte{0xff}) + require.Error(t, err) +}