types: return an error if voting power overflows

in VerifyCommitTrusting

Closes #4755

CHANGELOG_PENDING.md

@@ -48,3 +48,4 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi
 - [blockchain/v2] [\#4761](https://github.com/tendermint/tendermint/pull/4761) Fix excessive CPU usage caused by spinning on closed channels (@erikgrinaker)
 - [blockchain/v2] Respect `fast_sync` option (@erikgrinaker)
 - [light] [\#4741](https://github.com/tendermint/tendermint/pull/4741) Correctly return  `ErrSignedHeaderNotFound` and `ErrValidatorSetNotFound` on corresponding RPC errors (@erikgrinaker)
+- [types] [\#4764](https://github.com/tendermint/tendermint/pull/4764) Return an error if voting power overflows in `VerifyCommitTrusting` (@melekes)

types/validator_set.go

@@ -749,11 +749,15 @@ func (vals *ValidatorSet) VerifyFutureCommit(newSet *ValidatorSet, chainID strin
 
 // VerifyCommitTrusting verifies that trustLevel ([1/3, 1]) of the validator
 // set signed this commit.
+//
 // NOTE the given validators do not necessarily correspond to the validator set
 // for this commit, but there may be some intersection.
+//
+// Panics if trustLevel is invalid.
 func (vals *ValidatorSet) VerifyCommitTrusting(chainID string, blockID BlockID,
 	height int64, commit *Commit, trustLevel tmmath.Fraction) error {
 
+	// sanity check
 	if trustLevel.Numerator*3 < trustLevel.Denominator || // < 1/3
 		trustLevel.Numerator > trustLevel.Denominator { // > 1
 		panic(fmt.Sprintf("trustLevel must be within [1/3, 1], given %v", trustLevel))
@@ -766,9 +770,15 @@ func (vals *ValidatorSet) VerifyCommitTrusting(chainID string, blockID BlockID,
 	var (
 		talliedVotingPower int64
 		seenVals           = make(map[int]int, len(commit.Signatures)) // validator index -> commit index
-		votingPowerNeeded  = (vals.TotalVotingPower() * trustLevel.Numerator) / trustLevel.Denominator
 	)
 
+	// Safely calculate voting power needed.
+	totalVotingPowerMulByNumerator, overflow := safeMul(vals.TotalVotingPower(), trustLevel.Numerator)
+	if overflow {
+		return errors.New("int64 overflow while calculating voting power needed. please provide smaller trustLevel numerator")
+	}
+	votingPowerNeeded := totalVotingPowerMulByNumerator / trustLevel.Denominator
+
 	for idx, commitSig := range commit.Signatures {
 		if commitSig.Absent() {
 			continue // OK, some signatures can be absent.
@@ -912,7 +922,7 @@ func RandValidatorSet(numValidators int, votingPower int64) (*ValidatorSet, []Pr
 }
 
 ///////////////////////////////////////////////////////////////////////////////
-// safe addition/subtraction
+// safe addition/subtraction/multiplication
 
 func safeAdd(a, b int64) (int64, bool) {
 	if b > 0 && a > math.MaxInt64-b {
@@ -953,3 +963,33 @@ func safeSubClip(a, b int64) int64 {
 	}
 	return c
 }
+
+func safeMul(a, b int64) (int64, bool) {
+	if a == 0 || b == 0 {
+		return 0, false
+	}
+
+	absOfB := b
+	if b < 0 {
+		absOfB = -b
+	}
+
+	var (
+		c        = a
+		overflow bool
+	)
+
+	for absOfB > 1 {
+		c, overflow = safeAdd(c, a)
+		if overflow {
+			return c, true
+		}
+		absOfB--
+	}
+
+	if (b < 0 && a > 0) || (b < 0 && a < 0) {
+		return -c, false
+	}
+
+	return c, false
+}

types/validator_set_test.go

@@ -1365,7 +1365,47 @@ func TestVerifyCommitTrusting(t *testing.T) {
 			assert.NoError(t, err)
 		}
 	}
+}
+
+func TestVerifyCommitTrustingErrorsOnOverflow(t *testing.T) {
+	var (
+		blockID               = makeBlockIDRandom()
+		voteSet, valSet, vals = randVoteSet(1, 1, PrecommitType, 1, MaxTotalVotingPower)
+		commit, err           = MakeCommit(blockID, 1, 1, voteSet, vals, time.Now())
+	)
+	require.NoError(t, err)
 
+	err = valSet.VerifyCommitTrusting("test_chain_id", blockID, commit.Height, commit,
+		tmmath.Fraction{Numerator: 25, Denominator: 55})
+	if assert.Error(t, err) {
+		assert.Contains(t, err.Error(), "int64 overflow")
+	}
+}
+
+func TestSafeMul(t *testing.T) {
+	testCases := []struct {
+		a        int64
+		b        int64
+		c        int64
+		overflow bool
+	}{
+		0: {0, 0, 0, false},
+		1: {1, 0, 0, false},
+		2: {2, 3, 6, false},
+		3: {2, -3, -6, false},
+		4: {-2, -3, 6, false},
+		5: {-2, 3, -6, false},
+		6: {math.MaxInt64, 1, math.MaxInt64, false},
+		7: {math.MaxInt64 / 2, 2, math.MaxInt64 - 1, false},
+		8: {math.MaxInt64 / 2, 3, -1, true},
+		9: {math.MaxInt64, 2, -1, true},
+	}
+
+	for i, tc := range testCases {
+		c, overflow := safeMul(tc.a, tc.b)
+		assert.Equal(t, tc.c, c, "#%d", i)
+		assert.Equal(t, tc.overflow, overflow, "#%d", i)
+	}
 }
 
 //---------------------