sherlock-audit / 2023-11-covalent-judging

3 stars 2 forks source link

ydlee - Specimen session cannot be finalized if the validator submits the agreed specimen hash has an ID greater than 255. #97

Closed sherlock-admin2 closed 7 months ago

sherlock-admin2 commented 7 months ago

ydlee

medium

Specimen session cannot be finalized if the validator submits the agreed specimen hash has an ID greater than 255.

Summary

When finalize a specimen session, the validators that submit the agreed specimen hash will be tracked in the validatorBitMap. But the bitmap is 256 bits at max, if the validatorID > 255, the tracking will revert and the specimen session will not be finalized.

Vulnerability Detail

There is no limit to the value of validatorID when adding a BSP operator, so it is possible that validatorID exceeds 255.

176:    function addBSPOperator(address operator, uint128 validatorId) external onlyGovernor {
177:        require(operatorRoles[operator] == 0, "Operator already exists");
178:        operatorRoles[operator] = BLOCK_SPECIMEN_PRODUCER_ROLE;
179:->      validatorIDs[operator] = validatorId;
180:        _validatorOperators[validatorId].add(operator);
181:
182:        _blockSpecimenProducers.add(operator);
183:        _validatorActiveOperatorsCounters[validatorId]++;
184:        emit OperatorAdded(operator, validatorId, BLOCK_SPECIMEN_PRODUCER_ROLE);
185:    }

https://github.com/sherlock-audit/2023-11-covalent/blob/main/cqt-staking/contracts/BlockSpecimenProofChain.sol#L176-L185

When it comes to finalize a quorum achieved specimen session, a validatorBitMap is used to track the validators that submit the agreed specimen. The bitmap can track 256 validators at most, each bit for a validatorID. So if validatorID > 255, the tracking will revert, and the finalization will fail.

Function: finalizeSpecimenSession
430:        // check if the number of submissions is sufficient and if the quorum is achieved
431:        if (_minSubmissionsRequired <= max && (max * _DIVIDER) / contributorsN > 43:_blockSpecimenQuorum) {
432:            // TODO: doesn't free session space. Though it should.
433:->          _finalizeWithParticipants(session, chainId, blockHeight, agreedBlockHash, 43:agreedSpecimenHash);
434:        } else emit QuorumNotReached(chainId, blockHeight);

https://github.com/sherlock-audit/2023-11-covalent/blob/main/cqt-staking/contracts/BlockSpecimenProofChain.sol#L430-L434

441:    function _finalizeWithParticipants(BlockSpecimenSession storage session, uint64 chainId, uint64 blockHeight, bytes32 agreedBlockHash, bytes32 agreedSpecimenHash) internal {
442:        address participant;
443:        address[] storage participants = session.blockProperties[agreedBlockHash].participants[agreedSpecimenHash];
444:        uint256 len = participants.length;
445:        uint256 validatorBitMap; // sets the ith bit to 1 if the ith validator submits the agreed specimen hash
446:
447:        mapping(address => SessionParticipantData) storage participantsData = session.participantsData;
448:
449:        for (uint256 i = 0; i < len; i++) {
450:            participant = participants[i];
451:            SessionParticipantData storage pd = participantsData[participant];
452:->          validatorBitMap |= (1 << (255 - validatorIDs[participant])); // @audit revert if `ID > 255`
453:            // release gas if possible
454:            if (pd.submissionCounter == 1) {
455:                pd.submissionCounter = 0;
456:                pd.stake = 0;
457:            }
458:        }
459:
460:        emit BlockSpecimenQuorum(chainId, blockHeight, validatorBitMap, agreedBlockHash, agreedSpecimenHash);
461:
462:        delete session.blockProperties[agreedBlockHash]; // release gas
463:    }

https://github.com/sherlock-audit/2023-11-covalent/blob/main/cqt-staking/contracts/BlockSpecimenProofChain.sol#L441-L463

Impact

Agreed specimen cannot be finalized.

Code Snippet

https://github.com/sherlock-audit/2023-11-covalent/blob/main/cqt-staking/contracts/BlockSpecimenProofChain.sol#L176-L185

https://github.com/sherlock-audit/2023-11-covalent/blob/main/cqt-staking/contracts/BlockSpecimenProofChain.sol#L430-L434

https://github.com/sherlock-audit/2023-11-covalent/blob/main/cqt-staking/contracts/BlockSpecimenProofChain.sol#L441-L463

Tool used

Manual Review

Recommendation

Require the validatorID < 256 in addBSPOperator.

Duplicate of #81

sherlock-admin2 commented 7 months ago

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"