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

`OperatorStakingPool::withdraw` does not actually transfer the tokens back to the operator

federodes

Summary

The internal function OperatorStakingPool::_withdraw correctly updates the internal share balance during the withdrawal process. However, it fails to transfer the tokens back to the operator invoking the function.

Vulnerability Details

In https://github.com/Cyfrin/2024-09-stakelink/blob/main/contracts/linkStaking/OperatorStakingPool.sol#L116-L126, the onTokenTransfer function enables an operator to deposit liquid staking tokens into the OperatorStakingPool contract.

function onTokenTransfer(address _sender, uint256 _value, bytes calldata) external {
if (msg.sender != address(lst)) revert InvalidToken();
if (!isOperator(_sender)) revert SenderNotAuthorized();
if (getOperatorStaked(_sender) + _value > depositLimit) revert ExceedsDepositLimit();
​
uint256 sharesAmount = lst.getSharesByStake(_value);
shareBalances[_sender] += sharesAmount;
totalShares += sharesAmount;
​
emit Deposit(_sender, _value, sharesAmount);
}

In https://github.com/Cyfrin/2024-09-stakelink/blob/main/contracts/linkStaking/OperatorStakingPool.sol#L199-L205, the _withdraw function accurately updates the balances; however, it does not send the liquid staking tokens back to the operator.

function _withdraw(address _operator, uint256 _amount) private {
uint256 sharesAmount = lst.getSharesByStake(_amount);
​
shareBalances[_operator] -= sharesAmount;
totalShares -= sharesAmount;
@> Function never transfers the tokens to the caller
​
emit Withdraw(_operator, _amount, sharesAmount);
}

PoC

Add the following test to /test/linkStaking/operator-staking-pool.test.ts:

In this test, accounts[0] first deposits 1000 ETH into the OperatorStakingPool. Immediately after, the same account calls the withdraw function. However, the final balance is 1000 ETH less than expected, indicating an issue. Additionally, the OperatorStakingPool balance has increased by 1000 ETH, further highlighting the discrepancy.

it.only('does NOT transfer on withdraw', async () => {
const { signers, accounts, opPool, lst } = await loadFixture(deployFixture)
​
const accountZeroBalanceBefore = await lst.balanceOf(accounts[0])
console.log({ bbzero: fromEther(accountZeroBalanceBefore) })
​
const opPoolBalanceBefore = await lst.balanceOf(opPool)
console.log({ opb: fromEther(opPoolBalanceBefore) })
​
await lst.transferAndCall(opPool.target, toEther(1000), '0x')
​
const opPoolBalanceMiddle = await lst.balanceOf(opPool)
console.log({ opm: fromEther(opPoolBalanceMiddle) })
​
await opPool.withdraw(toEther(1000))
​
const accountZeroBalanceAfter = await lst.balanceOf(accounts[0])
console.log({ bazero: fromEther(accountZeroBalanceAfter) })
​
const opPoolBalanceAfter = await lst.balanceOf(opPool)
console.log({ opa: fromEther(opPoolBalanceAfter) })
​
// Account 0 lost 1000 ETH
assert.equal(fromEther(accountZeroBalanceBefore - accountZeroBalanceAfter), 1000)
​
// 1000 ETH are stuck in the OperatorStakingPool contract
assert.equal(fromEther(opPoolBalanceAfter - opPoolBalanceBefore), 1000)
})

Impact

Operators are unable to unstake (withdraw) tokens from the OperatorStakingPool, resulting in the tokens being locked within the contract.

Tools Used

Manual review

Recommendations

Perform the actual token transfer after the internal account balance updates.

function _withdraw(address _operator, uint256 _amount) private {
uint256 sharesAmount = lst.getSharesByStake(_amount);
​
shareBalances[_operator] -= sharesAmount;
totalShares -= sharesAmount;
+ lst.safeTransfer(_operator, _amount);
​
emit Withdraw(_operator, _amount, sharesAmount);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

`OperatorStakingPool::_withdraw()` function doesn't transfer the tokens

Support

FAQs

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