sherlock-audit / 2023-11-covalent-judging

3 stars 2 forks source link

Mafia - User cannot unstake when validator is frozen #44

Closed sherlock-admin closed 9 months ago

sherlock-admin commented 9 months ago

Mafia

medium

User cannot unstake when validator is frozen

Summary

The validator may be frozen by the project owner for some reason. At this time, the user cannot unstake his tokens. This is an infringement of the user's rights, because it is not the user's fault that the validator was frozen.

Vulnerability Detail

When the user calls unstake() function, it is checked whether the validator is frozen; if so, the transaction is revert

  function unstake(uint128 validatorId, uint128 amount) external whenNotPaused {
        require(amount > 0, "Amount is 0");
        _unstake(validatorId, amount);
    }

   function _unstake(uint128 validatorId, uint128 amount) internal {
        require(validatorId < validatorsN, "Invalid validator");

        Validator storage v = _validators[validatorId];
        Staking storage s = v.stakings[msg.sender];

        require(!v.frozen, "Validator is frozen");  // <-------------

Impact

It is not the user’s fault that the validator did something bad and suffers because of it - he cannot unstake

Code Snippet

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

Tool used

Manual Review

Recommendation

I suggest checking the status of the validator (whether it is frozen) only if the validator itself makes the unstake

   function _unstake(uint128 validatorId, uint128 amount) internal {
        require(validatorId < validatorsN, "Invalid validator");

        Validator storage v = _validators[validatorId];
        Staking storage s = v.stakings[msg.sender];
+       if(msg.sender == v._address) {
+          require(!v.frozen, "Validator is frozen"); 
+       }
sherlock-admin2 commented 9 months ago

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

takarez commented:

invalid: more like a centralization risk which is == invalid

noslav commented 9 months ago

fixed by allow unstake of delegator assets even if validator frozen - sa44

nevillehuang commented 8 months ago

Invalid, admins are trusted entities, so this is invalid based on sherlock rules, see point 5.3