Incorrect highestVotingPower Calculation in argmaxBlockByCount Function Resulting in Incorrect Block Selection

Summary

The argmaxBlockByCount function is designed to identify the block height with the highest number of reputers voting for it. However, the calculation step for highestVotingPower is missing, leading to incorrect block selection. Specifically, the function updates blockOfMaxPower when a new block with higher voting power is found but does not update highestVotingPower. This oversight results in a failure to properly track the highest voting power and select the correct block.

Vulnerability Detail

The following code snippet from argmaxBlockByCount illustrates the issue. Lines 9-11 properly calculate blockVotingPower as the count of reputers voting for each block. However, when finding a new highest voting power, only blockOfMaxPower is updated, while highestVotingPower is not, leading to incorrect outcomes.

func (ap *AppChain) argmaxBlockByCount(
    blockToReputer *map[int64][]string,
) int64 {
    firstIter := true
    highestVotingPower := cosmossdk_io_math.ZeroInt()
    blockOfMaxPower := int64(-1)
    for block, reputersWhoVotedForBlock := range *blockToReputer {
        blockVotingPower := cosmossdk_io_math.NewInt(int64(len(reputersWhoVotedForBlock)))

        if firstIter || blockVotingPower.GT(highestVotingPower) {
@>          blockOfMaxPower = block // Correctly updates the block
@>          // Missing highestVotingPower = blockVotingPower
        }

        firstIter = false
    }

    return blockOfMaxPower
}

Impact

The function does not accurately identify the block with the highest count of reputers. This misidentification can lead to incorrect block selection, potentially affecting subsequent processing or decision-making processes that rely on these results.

Code Snippet

Code Link

Tool used

Manual Review

Recommendation

To resolve this issue, update highestVotingPower whenever blockOfMaxPower is updated:

func (ap *AppChain) argmaxBlockByCount(
    blockToReputer *map[int64][]string,
) int64 {
    firstIter := true
    highestVotingPower := cosmossdk_io_math.ZeroInt()
    blockOfMaxPower := int64(-1)
    for block, reputersWhoVotedForBlock := range *blockToReputer {
        blockVotingPower := cosmossdk_io_math.NewInt(int64(len(reputersWhoVotedForBlock)))

        if firstIter || blockVotingPower.GT(highestVotingPower) {
+           highestVotingPower = blockVotingPower  // Update the highest voting power
            blockOfMaxPower = block  // Correctly updates the block
        }

        firstIter = false
    }

    return blockOfMaxPower
}

Duplicate of #44

sherlock-admin4 commented 4 months ago

1 comment(s) were left on this issue during the judging contest.

0xmystery commented:

Missing highestVotingPower update

sherlock-admin2 commented 4 months ago

The protocol team fixed this issue in the following PRs/commits: https://github.com/allora-network/allora-inference-base/pull/141

sherlock-audit / 2024-06-allora-judging

Yuriisereda - Incorrect highestVotingPower Calculation in argmaxBlockByCount Function Resulting in Incorrect Block Selection #45