Atharv - No "0" value checking and duplicate `validatorId` in the function `BlockSpecimenProofChain.sol::addBSPOperator` for `validatorIDs` mapping

[sherlock-admin]

Atharv

medium

No "0" value checking and duplicate validatorId in the function BlockSpecimenProofChain.sol::addBSPOperator for validatorIDs mapping

Summary

No "0" value checking in the function BlockSpecimenProofChain.sol::addBSPOperator for validatorIDs mapping and we are assigning "0" value when removing in BlockSpecimenProofChain.sol::removeBSPOperator function.

Vulnerability Detail

While adding BSPOperator we should check the validatorId should not be equal to 0 otherwise, there is inconsistency in the smart contract and if the validatorId is already present we are overriding in the BlockSpecimenProofChain.sol::addBSPOperator function

Impact

Medium

Code Snippet

addBSPOperator

removeBSPOperator

Tool used

Manual Review

Recommendation

function addBSPOperator(address operator, uint128 validatorId) external onlyGovernor {
        require(operatorRoles[operator] == 0, "Operator already exists");
+       require(validatorId > 0);
        operatorRoles[operator] = BLOCK_SPECIMEN_PRODUCER_ROLE;
        validatorIDs[operator] = validatorId; 
        _validatorOperators[validatorId].add(operator);

        _blockSpecimenProducers.add(operator);
        _validatorActiveOperatorsCounters[validatorId]++;
        emit OperatorAdded(operator, validatorId, BLOCK_SPECIMEN_PRODUCER_ROLE);
    }

[sherlock-admin2]

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

takarez commented:

invalid

[nevillehuang]

Invalid, similar reasonings to #31, governor role is trusted to input appropriate validatorId