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

The contract uses the `stake token` amount to convert it into the equivalent `share` amount during withdrawals. This design makes it nearly impossible for users to fully withdraw their shares, leaving behind dust balances in the account.

joicygiore

Summary

The contract uses the stake token amount to convert it into the equivalent share amount during withdrawals. This design makes it nearly impossible for users to fully withdraw their shares, leaving behind dust balances in the account.

Vulnerability Details

For example, let's consider the withdrawal process in PriorityPool::withdraw(). As marked with @>, when a user calls withdraw() to retrieve funds, the actual number of shares is calculated using getSharesByStake(). Due to rounding inaccuracies and changes in the exchange rate between when the user initiates the call and when the transaction is executed, it becomes highly unlikely that shares[_sender] == sharesToTransfer. To ensure the call succeeds, in most cases, the system opts for shares[_sender] > sharesToTransfer, which results in users being unable to completely withdraw their shares, leaving dust amounts in their accounts.

// PriorityPool::withdraw()
function withdraw(
uint256 _amountToWithdraw,
uint256 _amount,
uint256 _sharesAmount,
bytes32[] calldata _merkleProof,
bool _shouldUnqueue,
bool _shouldQueueWithdrawal
) external {
if (_amountToWithdraw == 0) revert InvalidAmount();
​
// SNIP...
​
// attempt to withdraw if tokens remain after unqueueing
if (toWithdraw != 0) {
@> IERC20Upgradeable(address(stakingPool)).safeTransferFrom(
account,
address(this),
toWithdraw
);
toWithdraw = _withdraw(account, toWithdraw, _shouldQueueWithdrawal);
}
​
​
token.safeTransfer(account, _amountToWithdraw - toWithdraw);
}
​
// SafeERC20Upgradeable::safeTransferFrom()
function safeTransferFrom(IERC20Upgradeable token, address from, address to, uint256 value) internal {
@> _callOptionalReturn(token, abi.encodeWithSelector(token.transferFrom.selector, from, to, value));
}
​
// StakingPool::StakingRewardsPool::ERC677Upgradeable::ERC20Upgradeable::transferFrom()
function transferFrom(address from, address to, uint256 amount) public virtual override returns (bool) {
address spender = _msgSender();
_spendAllowance(from, spender, amount);
@> _transfer(from, to, amount);
return true;
}
​
// StakingPool::StakingRewardsPool::_transfer()
function _transfer(address _sender, address _recipient, uint256 _amount) internal override {
@> uint256 sharesToTransfer = getSharesByStake(_amount);
​
​
require(_sender != address(0), "Transfer from the zero address");
require(_recipient != address(0), "Transfer to the zero address");
require(shares[_sender] >= sharesToTransfer, "Transfer amount exceeds balance");
​
​
@> shares[_sender] -= sharesToTransfer;
shares[_recipient] += sharesToTransfer;
​
​
emit Transfer(_sender, _recipient, _amount);
}
// StakingPool::StakingRewardsPool::getSharesByStake()
function getSharesByStake(uint256 _amount) public view returns (uint256) {
uint256 totalStaked = _totalStaked();
if (totalStaked == 0) {
return _amount;
} else {
@> return (_amount * totalShares) / totalStaked;
}
}

Impact

The reliance on converting stake tokens into share for withdrawal purposes makes it nearly impossible for users to fully withdraw their shares. This leaves small amounts, or dust, in user accounts

Tools Used

Manual Review

Recommendations

It is recommended to modify the withdrawal logic to use share as the basis for withdrawals, which would then be converted into the corresponding stake token amount. This would prevent dust from accumulating in user accounts and allow for more precise withdrawals.

Updates

Lead Judging Commences

inallhonesty Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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