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)
assert.equal(fromEther(await lst.balanceOf(accounts[1])), 10000)
await lst.connect(signers[1]).transferAndCall(opPool.target, toEther(1000), '0x')
assert.equal(fromEther(await lst.balanceOf(accounts[1])), 9000)
await opPool.connect(signers[1]).withdraw(toEther(700))
assert.equal(fromEther(await opPool.getOperatorPrincipal(accounts[1])), 300)
assert.equal(fromEther(await opPool.getOperatorStaked(accounts[1])), 300)
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.