Description

In sdk/utils.go, the function twosComplement is implemented as follows:

func twosComplement(bits []uint, n int) []uint {
	padded := padBitsRight(bits, n, 0)
	flipped := flipBits(padded)
	a := recompose(flipped, 1)
	a.Add(a, big.NewInt(1))
!	d := decomposeAndSlice(a, 1, 248)
	ret := make([]uint, len(d))
	for i, b := range d {
		ret[i] = uint(b.Uint64())
	}
	return ret
}

During the call to decomposeAndSlice, the constant 248 is used where it should be n.

Impact

Whenever n is different than 248, twosComplement will return a result of incorrect length and possibly lose information when n > 248.

There are only two places where twosComplement is called within brevis-sdk. Both are in sdk/api_int248.go, and in those calls n is actually 248, so this bug is not hit in the current codebase.

Recommendations

We recommend to use n instead of 248 in the decomposeAndSlice call:

func twosComplement(bits []uint, n int) []uint {
	padded := padBitsRight(bits, n, 0)
	flipped := flipBits(padded)
	a := recompose(flipped, 1)
	a.Add(a, big.NewInt(1))
- 	d := decomposeAndSlice(a, 1, 248)
+ 	d := decomposeAndSlice(a, 1, n)
	ret := make([]uint, len(d))
	for i, b := range d {
		ret[i] = uint(b.Uint64())
	}
	return ret
}

Remediation

This issue has been acknowledged by Brevis, and a fix was implemented in commit 4deade11↗.