Anubis - Potential Underflow in recoverUnstaking and transferUnstakedOut Functions

[sherlock-admin2]

Anubis

medium

Potential Underflow in recoverUnstaking and transferUnstakedOut Functions

Summary

The recoverUnstaking and transferUnstakedOut functions in the contract might be susceptible to underflow issues, leading to unintended consequences like locking funds indefinitely or creating inaccuracies in tracking staked amounts.

Vulnerability Detail

Both the recoverUnstaking and transferUnstakedOut functions deduct an amount from us.amount without ensuring that us.amount is greater than or equal to amount. Solidity 0.8.x does have in-built overflow/underflow protection, but relying solely on this can lead to unexpected contract behavior, like transaction reverts, especially if the logic assumes certain state changes post deduction.

Impact

If an underflow occurs (although protected by Solidity's in-built mechanism), it may cause transactions to fail or get reverted, leading to a denial of service. It might also cause issues with the correct accounting of staked tokens, potentially leading to locked funds or inaccurate tracking of staked amounts.

Code Snippet

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

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

Tool used

Manual Review

Recommendation

Ensure that the amount being deducted is never greater than the available amount to prevent potential underflow, even though Solidity 0.8.x offers inherent protection. Implement checks to validate that us.amount is greater than or equal to amount before performing the deduction. Here's a modified code snippet to illustrate this:

function recoverUnstaking(uint128 amount, uint128 validatorId, uint128 unstakingId) external whenNotPaused {
    //...
    require(us.amount >= amount, "Unstaking amount is insufficient for the requested recovery");
    us.amount -= amount;
    //...
}

function transferUnstakedOut(uint128 amount, uint128 validatorId, uint128 unstakingId) external whenNotPaused {
    //...
    require(us.amount >= amount, "Unstaking amount is insufficient for the requested transfer");
    us.amount -= amount;
    //...
}

[sudeepdino008]

these checks are already there.

[sherlock-admin2]

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

takarez commented:

invalid

[nevillehuang]

Invalid, underflow cannot happen due to this check