cawfree - Panic: Validators with IDs over `255` will prevent quorum blocks from finalizing.

[sherlock-admin2]

cawfree

high

Panic: Validators with IDs over 255 will prevent quorum blocks from finalizing.

Summary

When finalizing a block specimen proof that has reached quorum, a participant with a validatorID greater than 255 will cause finalization to revert with underflow.

Vulnerability Detail

Once a block specimen hash has met quorum, the internal function _finalizeWithParticipants(BlockSpecimenSession,uint64,uint64,bytes32,bytes32) is called to export the finalized results of the quorum.

When computing the validatorBitMap, a validatorID is used to determine the bit offset corresponding to the validator:

SessionParticipantData storage pd = participantsData[participant];
validatorBitMap |= (1 << (255 - validatorIDs[participant])); /// @audit

However, validatorIDs are not uint8s, they are uint128s (without restriction), meaning that for any identifier for a validator over 255 participating in majority-consensus validation will lead to numeric underflow during block finalization.

Impact

I consider this issue to be high severity as it prevents the proof chain from progressing specifically for the scenario where sufficient quorum has been met, preventing majority consensus votes from being finalized.

It should be noted that a Validator with a validatorID in excess of 255 can participate in normally in all other parts of the protocol, including being able to lock up collateral for staking.

Further, as there is no significant economic constraint against programmatically generating multiple Validator instances, therefore it would be possible to quickly reach the aforementioned threshold value if the intention were to explicitly grief the protocol - even unrelated new Validators that enter the protocol will be unintentionally complicit to this attack.

Code Snippet

function _finalizeWithParticipants(BlockSpecimenSession storage session, uint64 chainId, uint64 blockHeight, bytes32 agreedBlockHash, bytes32 agreedSpecimenHash) internal {
    address participant;
    address[] storage participants = session.blockProperties[agreedBlockHash].participants[agreedSpecimenHash];
    uint256 len = participants.length;
    uint256 validatorBitMap; // sets the ith bit to 1 if the ith validator submits the agreed specimen hash

    mapping(address => SessionParticipantData) storage participantsData = session.participantsData;

    for (uint256 i = 0; i < len; i++) {
        participant = participants[i];
        SessionParticipantData storage pd = participantsData[participant];
        validatorBitMap |= (1 << (255 - validatorIDs[participant]));
        // release gas if possible
        if (pd.submissionCounter == 1) {
            pd.submissionCounter = 0;
            pd.stake = 0;
        }
    }

    emit BlockSpecimenQuorum(chainId, blockHeight, validatorBitMap, agreedBlockHash, agreedSpecimenHash);

    delete session.blockProperties[agreedBlockHash]; // release gas
}

Tool used

Foundry

Recommendation

Consider constructing the bitmap using the offset of participants in the order they engaged within consensus for the block being finalized, opposed to their global identifier.

Duplicate of #81

[sherlock-admin2]

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

takarez commented:

invalid: The lenght of the validatorIDs is controlled by a governance which is considered trusted according to sherlock rules VIII number 4 under "exceptions"