Liquid Staking

Stakelink
DeFiHardhatOracle
50,000 USDC
View results
Submission Details
Severity: medium
Invalid

Inefficient Liquidity Withdrawal and Potential Token Burning Discrepancy

Summary

The withdraw function in the StakingPool contract contains inefficiencies in its liquidity withdrawal process and a potential discrepancy between burned tokens and actual withdrawals.

Details

The withdraw function in StakingPool.sol (lines 142-165) has two main issues:

Inefficient Liquidity Check: The function attempts to withdraw liquidity before checking if the contract has sufficient balance, potentially wasting gas on unnecessary operations.

Token Burning Discrepancy: The function burns the full requested amount of tokens from the user's account before ensuring that the full amount can be withdrawn, potentially leading to a mismatch between burned tokens and actual withdrawals.

The function first calculates the amount to withdraw, then checks the contract's balance. If the withdrawal amount exceeds the balance, it calls _withdrawLiquidity to attempt to cover the difference. However, it doesn't check if this operation was successful before proceeding.

After these operations, it performs a require check to ensure sufficient liquidity, but this check comes too late in the process. If it fails, all previous operations (including the potentially expensive _withdrawLiquidity call) will be reverted, wasting gas.

Finally, the function burns the full requested amount from the user's account and updates totalStaked, regardless of whether the full amount could actually be withdrawn.

Impact

This implementation could lead to several issues:

Gas Wastage: Attempting to withdraw liquidity before checking the balance can lead to unnecessary gas consumption, especially if the withdrawal ultimately fails.

Incorrect Token Burning: Users might have more tokens burned than they actually receive if the contract can't fulfill the full withdrawal amount, leading to a loss of user funds.

Inaccurate Total Staked Tracking: The totalStaked variable might be reduced by more than the actual withdrawn amount, leading to accounting discrepancies.

Potential for Imaginary Withdrawals: The receiver might receive less than the burned amount, or in extreme cases, no tokens at all while still having their staked balance reduced.

Code Snippet

function withdraw(
address _account,
address _receiver,
uint256 _amount,
bytes[] calldata _data
) external onlyPriorityPool {
uint256 toWithdraw = _amount;
if (_amount == type(uint256).max) {
toWithdraw = balanceOf(_account);
}
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);
totalStaked -= toWithdraw;
token.safeTransfer(_receiver, toWithdraw);
}

Recommendation

To address these issues, consider the following changes:

Check the contract's balance before attempting to withdraw liquidity:

uint256 balance = token.balanceOf(address(this));
if (toWithdraw > balance) {
uint256 additionalLiquidity = _withdrawLiquidity(toWithdraw - balance, _data);
balance += additionalLiquidity;
}
if (balance < toWithdraw) {
toWithdraw = balance;
}

Only burn and transfer the amount that can actually be withdrawn:

_burn(_account, toWithdraw);
totalStaked -= toWithdraw;
token.safeTransfer(_receiver, toWithdraw);

You can also Consider returning the actual withdrawn amount to the caller, so they can handle partial withdrawals appropriately.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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