Liquid Staking

Stakelink
DeFiHardhatOracle
50,000 USDC
View results
Submission Details
Severity: high
Valid

OperatorStakingPool::withdraw - LST tokens are not sent to the operator and they remain stuck in the contract forever

Summary

Link: https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/linkStaking/OperatorStakingPool.sol#L132

The withdraw function updates shareBalances for the operator, but no LST tokens are sent to the operator. Also, there is no general withdraw function that would allow the contract owner to transfer LST out of the contract. So, those tokens remain stuck in the contract forever.

Vulnerability Details

Proof of Concept:

Add the following test to operator-staking-pool.test.ts

it('withdraw does not send LST back to the operator', async () => {
const { signers, accounts, opPool, lst } = await loadFixture(deployFixture)
//account1 initially has 10000 LST
assert.equal(fromEther(await lst.balanceOf(accounts[1])), 10000)
//account1 deposits 1000 LST
await lst.connect(signers[1]).transferAndCall(opPool.target, toEther(1000), '0x')
assert.equal(fromEther(await lst.balanceOf(accounts[1])), 9000)
//account1 withdraws 700 LST
await opPool.connect(signers[1]).withdraw(toEther(700))
//the internal accounting is correct
assert.equal(fromEther(await opPool.getOperatorPrincipal(accounts[1])), 300)
assert.equal(fromEther(await opPool.getOperatorStaked(accounts[1])), 300)
// !!! this currently fails !!! account1 should now hold 9700 LST, but holds only 9000 LST
assert.equal(fromEther(await lst.balanceOf(accounts[1])), 9700)
})

Impact

As already mentioned above, operators cannot recover their LST and all deposited LST remains stuck in the contract forever.

Tools Used

Manual Review

Recommendations

Modify the _withdraw function

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

Ideally, the contract should also have a function that allows the owner to withdraw LST.

Updates

Lead Judging Commences

inallhonesty Lead Judge 11 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.