Summary
Node operators are required to stake LSTs in the OperatorStakingPool contract. The staked tokens should be withdrawable anytime by calling the withdraw function. Additionally, tokens should be returned to the operators when they are removed via the owner-controlled removeOperators function. However, the withdrawal implementation lacks the actual transfer call, causing operators to lose their LST tokens.
Vulnerability Details
The contract provides two scenarios where staked tokens are expected to be returned:
In both scenarios,_withdraw function is used to handle withdrawal logic (OperatorStakingPool.sol#L199-L205):
function _withdraw(address _operator, uint256 _amount) private {
uint256 sharesAmount = lst.getSharesByStake(_amount);
shareBalances[_operator] -= sharesAmount;
totalShares -= sharesAmount;
emit Withdraw(_operator, _amount, sharesAmount);
}
While this function adjusts the operator's share balance and the total account balance, it does not perform the actual token transfer, effectively leaving the tokens in the contract.
PoC
Add account balance check into withdrawal test case after line #83:
it('withdraw should work correctly', async () => {
const { signers, accounts, opPool, lst } = await loadFixture(deployFixture)
await lst.transferAndCall(opPool.target, toEther(1000), '0x')
await lst.connect(signers[1]).transferAndCall(opPool.target, toEther(500), '0x')
await expect(opPool.connect(signers[3]).withdraw(toEther(100))).to.be.revertedWithCustomError(
opPool,
'SenderNotAuthorized()'
)
await opPool.withdraw(toEther(1000))
await opPool.connect(signers[1]).withdraw(toEther(200))
assert.equal(fromEther(await lst.balanceOf(signers[1])), 9700)
assert.equal(fromEther(await opPool.getOperatorPrincipal(accounts[0])), 0)
assert.equal(fromEther(await opPool.getOperatorStaked(accounts[0])), 0)
assert.equal(fromEther(await opPool.getOperatorPrincipal(accounts[1])), 300)
assert.equal(fromEther(await opPool.getOperatorStaked(accounts[1])), 300)
assert.equal(fromEther(await opPool.getTotalPrincipal()), 300)
assert.equal(fromEther(await opPool.getTotalStaked()), 300)
await lst.setMultiplierBasisPoints(20000)
await opPool.connect(signers[1]).withdraw(toEther(500))
assert.equal(fromEther(await opPool.getOperatorPrincipal(accounts[0])), 0)
assert.equal(fromEther(await opPool.getOperatorStaked(accounts[0])), 0)
assert.equal(fromEther(await opPool.getOperatorPrincipal(accounts[1])), 100)
assert.equal(fromEther(await opPool.getOperatorStaked(accounts[1])), 100)
assert.equal(fromEther(await opPool.getTotalPrincipal()), 100)
assert.equal(fromEther(await opPool.getTotalStaked()), 100)
})
Test run output:
npx hardhat test test/linkStaking/operator-staking-pool.test.ts --network hardhat
OperatorStakingPool
✔ onTokenTransfer should work correctly (2428ms)
1) withdraw should work correctly
✔ addOperators should work correctly
✔ removeOperators should work correctly
3 passing (3s)
1 failing
1) OperatorStakingPool
withdraw should work correctly:
AssertionError: expected 9500 to equal 9700
+ expected - actual
-9500
+9700
The balance of signers[1] before the withdrawal is 9500 tokens and must become 9700 after withdrawing 200, however, it remains unchanged causing the test to fail.
Impact
Node operators are unable to withdraw staked tokens.
Tools Used
Manual review
Recommendations
Add token transfer call after balance adjustments:
function _withdraw(address _operator, uint256 _amount) private {
uint256 sharesAmount = lst.getSharesByStake(_amount);
shareBalances[_operator] -= sharesAmount;
totalShares -= sharesAmount;
>>> IERC20Upgradeable(address(lst)).safeTransfer(_operator, _amount);
emit Withdraw(_operator, _amount, sharesAmount);
}
