sherlock-audit / 2023-11-covalent-judging

3 stars 2 forks source link

caventa - Validator unstaking exploit extends delegator cooldown period #95

Closed sherlock-admin2 closed 7 months ago

sherlock-admin2 commented 7 months ago

caventa

medium

Validator unstaking exploit extends delegator cooldown period

Summary

Validator could misuse unstaking process to delay or indefinitely delay delegators from unstake to receive their funds

Vulnerability Detail

See _unstake function

function _unstake(uint128 validatorId, uint128 amount) internal {
    ...
    if (isValidator && validatorEnableMinStake > 0 && v.disabledAtBlock == 0 && s.staked < validatorEnableMinStake) {
        uint256 disabledAtBlock = block.number;
        v.disabledAtBlock = disabledAtBlock;
        emit ValidatorDisabled(validatorId, disabledAtBlock);
    }

    // create unstaking instance
    uint128 coolDownEnd = uint128(v.disabledAtBlock != 0 ? v.disabledAtBlock : block.number);
    unchecked {
        coolDownEnd += (isValidator ? validatorCoolDown : delegatorCoolDown);
    }
    uint128 unstakeId = uint128(v.unstakings[msg.sender].length);
    v.unstakings[msg.sender].push(Unstaking(coolDownEnd, effectiveAmount));
    emit Unstaked(validatorId, msg.sender, effectiveAmount, unstakeId);
    ...
}

Steps for potential exploitation:

A validator begins with a stake slightly above the required minimum. The validator frequently unstakes small amounts, causing their total stake to fall below the minimum threshold. When the validator's stake drops below the minimum, the contract automatically disables them and records the current block number in v.disabledAtBlock. After each disabling, the validator sets up a new unstaking instance. The cooldown end for these instances is determined based on v.disabledAtBlock. Through repeated actions, the validator can consistently prolong the cooldown period.

Impact

The "rewardValidators" function rewards validators based on an amount, and this amount should have a correlation with the share. See the following partial code snippet from the rewardValidators function

           commissionPaid = uint128((uint256(amount) * uint256(v.commissionRate)) / DIVIDER);

            // distribute the tokens by increasing the exchange rate
            // div by zero impossible due to check above
            // (and in fact, presuming minValidatorEnableStake >= DIVIDER, v.totalShares will
            //  always be >= DIVIDER while validator is enabled)
            v.exchangeRate += uint128((uint256(amount - commissionPaid) * uint256(DIVIDER)) / v.totalShares);

            // commission is not compounded
            // commisison is distributed under the validator instance
            v.commissionAvailableToRedeem += commissionPaid;

If a validator can maintain their delegator stake for a longer period, they have the potential to receive more rewards because the total share (or "share" amount) will be larger.

Code Snippet

https://github.com/sherlock-audit/2023-11-covalent/blob/main/cqt-staking/contracts/OperationalStaking.sol#L523-L533

Tool used

Manual Review

Recommendation

Modify the contract to prevent validators from influencing the timing of delegator unstaking and fund access.

sherlock-admin2 commented 7 months ago

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

takarez commented:

valid

nevillehuang commented 6 months ago

Invalid, validators disabling themselves will only harm them given they will not be eligible for rewards anymore.