cergyk - OperationalStaking::setValidatorAddress unstaked validator can grief delegator by setting his address as new validator

[sherlock-admin]

cergyk

medium

OperationalStaking::setValidatorAddress unstaked validator can grief delegator by setting his address as new validator

Summary

Validators and delegators both stake CQT to earn rewards associated with a validator Id, however they have vastly different withdrawal times, respectively 6 months and 28 days. This means that a malicious validator can grief a delegator by setting his address as new validator and imposing on him to keep the funds in the contract for 6 months instead of 28 days.

Vulnerability Detail

We can see that a delegator can set any address except current as new validator address: https://github.com/sherlock-audit/2023-11-covalent/blob/main/cqt-staking/contracts/OperationalStaking.sol#L689-L711

Also since this function does not check that a validator is disabled, the validator can have unstaked most of his funds already.

Scenario

Setup: Alice has a validator of id 1, Bob delegated to Alice's validator.

1/ Alice disables her validator by unstaking most of her funds:

Alice can unstake the amount such as maxDelegationCap ratio is respected. Since the value currently used in production for this ratio is x20, we can estimate that Alice has to leave only 5% of the value of funds delegated to her in the contract.

2/ Alice waits the cooldown period and transfers all of her funds out.

3/ Alice sets Bob as the new validator, which would lock Bob's stake for 6 months instead of 28 days, and Alice loses 5% of Bob funds.

Impact

Alice as a validator can cause griefing to Bob which has delegated, by sacrificing 5% of the value of the delegation, to make Bob lock his funds for 6 months instead of 28 days.

Code Snippet

Tool used

Manual Review

Recommendation

Please consider ensuring that validator does not set the new validator address to a current delegator, or enable the delegator to opt-out of being set as the new validator.

[sherlock-admin2]

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

takarez commented:

invalid

[noslav]

This is a bigger ask since first we have to extract all addresses involved in the stakings map in the validator struct

    struct Validator {
        uint128 commissionAvailableToRedeem;
        uint128 exchangeRate; // validator exchange rate
        address _address; // wallet address of the operator which is mapped to the validator instance
        uint128 delegated; // track amount of tokens delegated
        uint128 totalShares; // total number of validator shares
        uint128 commissionRate;
        uint256 disabledAtBlock;
        mapping(address => Staking) stakings;
        mapping(address => Unstaking[]) unstakings;
        bool frozen;
    }

    function getTotalValidators() public view returns (uint256) {
        return validatorsN;
    }

    function getAllDelegators() public view returns (bytes32[] memory delegatorKeys) {
        for (uint128 i = 0; i < validatorsN; i++) {
            Validator storage v = _validators[i];
            for (uint128 j = 0; j < v.stakings.length; j++) {
                address delegator = v.stakings[msg.sender][j].address; // Extract the delegate's address from the staking mapping
                bytes32 delegatorKey = keccak256(abi.encodePacked(delegator)); // Generate a unique identifier for the delegate's key
                if (!delegatorKey.equals(bytes32[](new bytes32[]))) {
                    delegatorKeys.push(delegatorKey); // Add the delegate's key to the array of delegator keys
                }
            }
        }

        return delegatorKeys;
    }

[noslav]

partially fixed by implement unique delegator extraction for setValidatoAddress - sa88

[sudeepdino008]

the implication of an malicious validator doing this is that he loses control of his funds (since delegator address is not owned by the validator). So sure malicious validator can do this, but at the cost of losing all his funds. So this attack vector doesn't make sense.

[noslav]

@sudeepdino008 good point however since the max cap allow a higher delegation amount than validator min stake, the validator can still unstake till they are at the min level and yet grieve a delegator with a higher delegated amount by setting them as a validator address (say for example: a delegator pulls out from a good validator and restakes to another validator with lower commissions etc), plus the delegator will also now be responsible for other delegators for that particular validator losing funds/rewards since infrastructure has to be set up to meet the needs. This can be attack vector we should prevent. This has parallels to the nothing at stake problem (except here its a "comparably low at stake")

[rogarcia]

commit with fix https://github.com/covalenthq/cqt-staking/pull/125/commits/d89d3e2108179658ae0d96f1b2af1ba364f7a772

[midori-fuse]

Escalate

This issue should be a dupe of #66. Both issues' root cause is that setValidatorAddress() stacks the old validator address' info on top of the new validator address' info, then making the new address a validator. According to Sherlock rules, one error in the code leads to two attack paths should be duped. This issue simply describes a different - and much more costly - attack that arises from the same root cause.

Furthermore, I'd also like to argue that this attack is far too unrealistic to even be considered a medium.

• The cost of the attack is too large compared to its return. At the current time, 35000 CQT is equal to $7085, you forfeit that much to grief one delegator (and it's your delegator no less), furthermore you actually forfeit your funds to your supposed "victim".
  • If Alice attacks Bob right away, then Bob gets locked for 6 months, but he takes all of the stakes of Alice, which is at least 35000 CQT. Note that Alice loses ALL of her stakes, not just minimum amount.
  • If Alice unstakes all but leaving 35000 CQT prior to attacking, then her unstake is also subject to a 6-month cooldown.
  • If she attacks Bob right away without waiting for 6 months, Bob gets all her unstakings. Alice loses funds in the same way as with the former scenario.
  • If she waits 6 months for her funds to be released first before attacking Bob, then she has to have sniped Bob for 6 months in advance.

Therefore the impact is 5-month cooldown extension of one user, but the cost is either all of the validator's stake, or the malicious validator must have sniped that specific target for 6 months, which is how long step (1) requires, and the validator still loses 35000 CQT anyway.

• Furthermore you cannot grief more than one user with this, as you lose your validator status as a result. Validator listing is permissioned, by applying with the Covalent team.

Quoting Sherlock rule on DOS-griefing:

Could Denial-of-Service (DOS), griefing, or locking of contracts count as a Medium (or High) issue? DoS has two separate scores on which it can become an issue:

1. The issue causes locking of funds for users for more than a week.
2. The issue impacts the availability of time-sensitive functions (cutoff functions are not considered time-sensitive). If at least one of these are describing the case, the issue can be a Medium. If both apply, the issue can be considered of High severity. Additional constraints related to the issue may decrease its severity accordingly.

While this issue technically does fit into criteria 1, I believe the constraints related to this attack is far too much to consider this a medium.

[sherlock-admin]

Escalate

This issue should be a dupe of #66. Both issues' root cause is that setValidatorAddress() stacks the old validator address' info on top of the new validator address' info, then making the new address a validator. According to Sherlock rules, one error in the code leads to two attack paths should be duped. This issue simply describes a different - and much more costly - attack that arises from the same root cause.

Furthermore, I'd also like to argue that this attack is far too unrealistic to even be considered a medium.

• The cost of the attack is too large compared to its return. At the current time, 35000 CQT is equal to $7085, you forfeit that much to grief one delegator (and it's your delegator no less), furthermore you actually forfeit your funds to your supposed "victim".
  • If Alice attacks Bob right away, then Bob gets locked for 6 months, but he takes all of the stakes of Alice, which is at least 35000 CQT. Note that Alice loses ALL of her stakes, not just minimum amount.
  • If Alice unstakes all but leaving 35000 CQT prior to attacking, then her unstake is also subject to a 6-month cooldown.
  • If she attacks Bob right away without waiting for 6 months, Bob gets all her unstakings. Alice loses funds in the same way as with the former scenario.
  • If she waits 6 months for her funds to be released first before attacking Bob, then she has to have sniped Bob for 6 months in advance.

Therefore the impact is 5-month cooldown extension of one user, but the cost is either all of the validator's stake, or the malicious validator must have sniped that specific target for 6 months, which is how long step (1) requires, and the validator still loses 35000 CQT anyway.

• Furthermore you cannot grief more than one user with this, as you lose your validator status as a result. Validator listing is permissioned, by applying with the Covalent team.

Quoting Sherlock rule on DOS-griefing:

Could Denial-of-Service (DOS), griefing, or locking of contracts count as a Medium (or High) issue? DoS has two separate scores on which it can become an issue:

1. The issue causes locking of funds for users for more than a week.
2. The issue impacts the availability of time-sensitive functions (cutoff functions are not considered time-sensitive). If at least one of these are describing the case, the issue can be a Medium. If both apply, the issue can be considered of High severity. Additional constraints related to the issue may decrease its severity accordingly.

While this issue technically does fit into criteria 1, I believe the constraints related to this attack is far too much to consider this a medium.

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[ArnieSec]

I agree with the escalation above

[Oot2k]

Agree with the escalation, duplicate of 66 or low based on attack cost.

[sudeepdino008]

commit with fix covalenthq/cqt-staking@d89d3e2

Fix provided

[nevillehuang]

Agree with @midori-fuse, this should be low severity based on impact described

[Czar102]

Is it reasonable/possible to be a delegator with 100k+ CQT stake?

[midori-fuse]

@Czar102 According to Covalent staking dashboard, looks like the average stake (delegator or validator) is 250k (106M total stake across 436 unique addresses).

The minimum amount of validator self-stake at the current time, however, is at 175k. The minimum attacker loss would be U of Calgary with a 155k stake loss (still given the 6 months snipe). 155k because a validator must keep their delegated stake not exceeding 27x their validator stake.

155k CQT is more than $33k as of current.

[Czar102]

I'm wondering if it's possible for a delegator to have a stake so large that additional lockup time, even with this "bonus" from the attacker, would inflict a net loss (assuming some reasonable interest rates on the token). @midori-fuse any idea?

[midori-fuse]

@Czar102 According to the dashboard, the current average staking APR is 10.09%. The reward they would've accrued in 5 months is 4.17% of their stake amount.

The max delegator stake multiplier is 27x. Since 1/27 = 3.7%, it's true that there is a net loss of 0.47% of their original stake amount (in would-have-accrued rewards), or 12.7% of the attacker's loss.

• The equilibrium point is 24x, i.e. if the delegator stakes 24x the validator's amount, then there wouldn't be a loss.

I still don't think such an attack is reasonable because:

• Currently there are 436 unique addresses staked across 15 delegators. Even without this info, it's unlikely anyone is delegating 24x the validators', except for self-delegates (e.g. for the validator to earn more rewards).
• You'd have to sacrifice your entire stake for your delegator to incur a net loss of 12.7% of your sacrificed amount.
  • The lower bound of the attack's cost is also very large: sacrificing 175k CQT to grief 22k. It's also unlikely someone will even delegate you that amount, so you still lose 175k CQT, but potentially grief even less.
  • You also lose your validator status and reputation at that, which adds to the attack risk.
• If you actually have 24x of 175k CQT (= 4.2M CQT), then it's more optimal to run your own validator instead of delegating to someone else. You get more rewards since you are not subject to others' commission rate, and the hardware requirements are easily affordable if you have such funds.

175k is because this is the value currently used in production.

[Czar102]

Given that the attacker can grief for about 1/8 of the attack cost, simultaneously losing reputation, and given that such huge delegators should become validators, I think the escalation made a valid point.

Planning to accept the escalation and consider this issue a low severity one.

[Czar102]

Result: Low Unique

[sherlock-admin2]

Escalations have been resolved successfully!

Escalation status:

• midori-fuse: accepted

[sherlock-admin]

The protocol team fixed this issue in PR/commit https://github.com/covalenthq/cqt-staking/pull/125/commits/d89d3e2108179658ae0d96f1b2af1ba364f7a772.

[sherlock-admin]

The Lead Senior Watson signed off on the fix.