crypto/types: CompactUnmarshal naively assumes bytes sent in will always be Uvarint encoded and crashes if negative

[odeke-em]

Summary of Bug

I audited this code

cosmos-sdk/crypto/types/compact_bit_array.go

Lines 246 to 263 in bffcae5

	func CompactUnmarshal(bz []byte) (*CompactBitArray, error) {
	if len(bz) < 2 {
	return nil, errors.New("compact bit array: invalid compact unmarshal size")
	} else if bytes.Equal(bz, []byte("null")) {
	return NewCompactBitArray(0), nil
	}
	size, n := binary.Uvarint(bz)
	bz = bz[n:]
	if len(bz) != int(size+7)/8 {
	return nil, errors.New("compact bit array: invalid compact unmarshal size")
	}
	bA := &CompactBitArray{uint32(size % 8), bz}
	return bA, nil
	}

and look at this code

cosmos-sdk/crypto/types/compact_bit_array.go

Lines 253 to 254 in bffcae5

	size, n := binary.Uvarint(bz)
	bz = bz[n:]

all one needs to do is pass in bad bytes for the size and that'll be returned in n which is naively passed to a slice and hence a crash :-(

for example here is an excerpt that'll crash https://play.golang.org/p/UMRoxgM1sL7 or inlined

package main

import "encoding/binary"

func main() {
	bz := []byte{0xd7, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x01}
	size, n := binary.Uvarint(bz)
	bz = bz[n:]
	println(size, n)
}

which crashes with

panic: runtime error: slice bounds out of range [-11:]

goroutine 1 [running]:
main.main()
	/tmp/sandbox752607808/prog.go:8 +0xc6

In fact, I wrote a fuzz pass and it found 6 crashes in less than 4 seconds, it took me 5 seconds to read and figure it out as a human

$ go-fuzz
2021/04/21 18:22:11 workers: 8, corpus: 16 (1s ago), crashers: 6, restarts: 1/0, execs: 0 (0/sec), cover: 0, uptime: 3s
2021/04/21 18:22:14 workers: 8, corpus: 16 (4s ago), crashers: 7, restarts: 1/0, execs: 0 (0/sec), cover: 24, uptime: 6s

Remedy

We shouldn't be naively slicing here, we should return an error in these situations

Version

bffcae5

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

/cc @cuonglm