Liquid Staking
Stakelink
DeFiHardhatOracle
50,000 USDC
View results
Previous Next
Submission Details
Severity: high
Invalid

Withdrawing More Tokens Than Deposited

lin0x9a7a

Summary

Users may withdraw more tokens than they originally deposited by setting the _amount parameter to type(uint256).max. This vulnerability stems from the incorrect assumption that setting _amount to type(uint256).max means the user wants to withdraw their entire balance, without considering potential balance increases since the original deposit.

Vulnerability Details

When _amount is type(uint256).max, the toWithdraw variable is set to the user's entire balance (balanceOf(_account)) instead of the original deposited amount. This leads to the entire balance being burned and transferred to the user, even if they had only deposited a smaller amount.

The reason for this vulnerability is that the code assumes setting _amount to type(uint256).max means the user wants to withdraw their entire balance. However, it does not consider that the user's balance may have increased since their original deposit due to rewards or other deposits.

StakingPool.sol# https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/StakingPool.sol#L142-L165

function withdraw(
address _account,
address _receiver,
uint256 _amount,
bytes[] calldata _data
) external onlyPriorityPool {
uint256 toWithdraw = _amount;
if (_amount == type(uint256).max) {
toWithdraw = balanceOf(_account); // <-- Vuln: Sets toWithdraw to full balance
}
​
uint256 balance = token.balanceOf(address(this));
if (toWithdraw > balance) {
_withdrawLiquidity(toWithdraw - balance, _data);
}
require(
token.balanceOf(address(this)) >= toWithdraw,
"Not enough liquidity available to withdraw"
);
​
_burn(_account, toWithdraw); // <-- Vuln: Burns the full balance
totalStaked -= toWithdraw; // <-- Vuln: Decreases totalStaked by the full balance
token.safeTransfer(_receiver, toWithdraw); // <-- Vuln: Transfers the full balance
}

The bug is connected to the logic in the withdraw function that handles the case when _amount is type(uint256).max. Instead of limiting the withdrawal to the original deposited amount, it allows withdrawing the full balance.

Imagine a User:

  1. User deposits a non-zero amount of tokens into the pool using the deposit function.

  2. User calls the withdraw function with _amount set to type(uint256).max.

  3. The withdraw function burns and transfers the user's entire balance, even if it exceeds the original deposited amount.

Impact

Users could withdraw more tokens than they originally deposited, effectively stealing tokens from the pool. It can lead to the depletion of the pool's funds and loss of assets for other users and the protocol.

Tools Used

Vs Code

Recommendations

By adding min(balanceOf(_account), _amount), the code ensures that even if type(uint256).max is passed, the withdrawal amount is capped at the minimum of the user's balance and the original deposit amount. This prevents users from withdrawing more tokens than they deposited.

if (_amount == type(uint256).max) {
- toWithdraw = balanceOf(_account);
+ toWithdraw = min(balanceOf(_account), _amount);
}
​
// ...
​
_burn(_account, toWithdraw);
totalStaked -= toWithdraw;
token.safeTransfer(_receiver, toWithdraw);
}
​
Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.