crypto/types: check for overflow and unreasonably large element count (#9163)

Ensure that we don't pass overflowed values into make, because
a clever attacker could see that to cause:

    (bits+7)/8

to become negative, they just have to make (bits+7) become negative
simply by >=maxint-6

but also reject unreasonably large element count like >2**32, which
while arbitrary is super duper large for a bit array.

Fixes #9162
This commit is contained in:
Emmanuel T Odeke 2021-04-21 20:09:41 -07:00 committed by GitHub
parent bffcae54a1
commit 417832fa9a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 47 additions and 2 deletions

View File

@ -5,6 +5,7 @@ import (
"encoding/binary"
"errors"
"fmt"
"math"
"regexp"
"strings"
)
@ -15,14 +16,24 @@ import (
// This is not thread safe, and is not intended for concurrent usage.
// NewCompactBitArray returns a new compact bit array.
// It returns nil if the number of bits is zero.
// It returns nil if the number of bits is zero, or if there is any overflow
// in the arithmetic to encounter for the number of its elements: (bits+7)/8,
// or if the number of elements will be an unreasonably large number like
// > maxint32 aka >2**31.
func NewCompactBitArray(bits int) *CompactBitArray {
if bits <= 0 {
return nil
}
nElems := (bits + 7) / 8
if nElems <= 0 || nElems > math.MaxInt32 {
// We encountered an overflow here, and shouldn't pass negatives
// to make, nor should we allow unreasonable limits > maxint32.
// See https://github.com/cosmos/cosmos-sdk/issues/9162
return nil
}
return &CompactBitArray{
ExtraBitsStored: uint32(bits % 8),
Elems: make([]byte, (bits+7)/8),
Elems: make([]byte, nElems),
}
}

View File

@ -2,6 +2,8 @@ package types
import (
"encoding/json"
"fmt"
"math"
"math/rand"
"testing"
@ -239,3 +241,35 @@ func BenchmarkNumTrueBitsBefore(b *testing.B) {
}
})
}
func TestNewCompactBitArrayCrashWithLimits(t *testing.T) {
if testing.Short() {
t.Skip("This test can be expensive in memory")
}
tests := []struct {
in int
mustPass bool
}{
{int(^uint(0) >> 30), false},
{int(^uint(0) >> 1), false},
{int(^uint(0) >> 2), false},
{int(math.MaxInt32), true},
{int(math.MaxInt32) + 1, true},
{int(math.MaxInt32) + 2, true},
{int(math.MaxInt32) - 7, true},
{int(math.MaxInt32) + 24, true},
{int(math.MaxInt32) * 9, false}, // results in >=maxint after (bits+7)/8
{1, true},
{0, false},
}
for _, tt := range tests {
tt := tt
t.Run(fmt.Sprintf("%d", tt.in), func(t *testing.T) {
got := NewCompactBitArray(tt.in)
if g := got != nil; g != tt.mustPass {
t.Fatalf("got!=nil=%t, want=%t", g, tt.mustPass)
}
})
}
}