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

Repeated batch ID entries will be pushed to `WithdrawalPool::withdrawalBatches` as a consequence of partially fulfilled withdrawals

robertodf99

Summary

When withdrawals are processed, a loop distributes the deposited amount across all queued withdrawals. If the remaining amount during the final withdrawal in the batch is smaller than the amount queued, only part of it is fulfilled, and the remaining shares are stored for future use. The batch ID is then added to the WithdrawalPool::withdrawalBatches array.

if (sharesRemaining > sharesToWithdraw) {
// partially finalize withdrawal
queuedWithdrawals[i] = Withdrawal(
uint128(sharesRemaining - sharesToWithdraw),
uint128(
queuedWithdrawals[i].partiallyWithdrawableAmount +
_getStakeByShares(sharesToWithdraw)
)
);
indexOfNextWithdrawal = i;
​
withdrawalBatches.push(
WithdrawalBatch(
uint128(i - 1),
uint128(_getStakeByShares(1 ether))
)
);

If multiple withdrawals are finalized within a short time frame and the sum of the deposited amounts does not exceed the total queued for withdrawal, the same batch ID can be pushed to the array multiple times.

Vulnerability Details

Pushing the same batch ID multiple times inflates the size of the array, increasing the cost of future calls to functions that rely on it, such as WithdrawalPool::getFinalizedWithdrawalIdsByOwner when queried on-chain. Note that since the validation checks in WithdrawalPool::withdraw require users to use the first pushed batch ID, this prevents users from claiming a higher stake-per-share rate than the one applied when the tokens were initially deposited.

Impact

To test the issue you can refer to the PoC below for which the visibility of the array has been changed from internal to public.

See PoC
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.15;
​
import {Test, console2} from "forge-std/Test.sol";
import {ERC677} from "../src/contracts/core/tokens/base/ERC677.sol";
import {StakingPool} from "../src/contracts/core/StakingPool.sol";
import {StrategyMock} from "../src/contracts/core/test/StrategyMock.sol";
import {WithdrawalPool} from "../src/contracts/core/priorityPool/WithdrawalPool.sol";
import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
import {LSTRewardsSplitterController} from "../src/contracts/core/lstRewardsSplitter/LSTRewardsSplitterController.sol";
import {LSTRewardsSplitter} from "../src/contracts/core/lstRewardsSplitter/LSTRewardsSplitter.sol";
​
contract WithdrawalPoolTest is Test {
ERC677 token;
StakingPool stakingPool;
StrategyMock strategy;
WithdrawalPool withdrawalPool;
StakingPool stakingPoolImpl = new StakingPool();
WithdrawalPool withdrawalPoolImpl = new WithdrawalPool();
StrategyMock strategyImpl = new StrategyMock();
LSTRewardsSplitterController rewardsSplitterController;
address account0 = makeAddr("account0");
​
struct StakingPoolParams {
address token;
string name;
string symbol;
StakingPool.Fee[] fee;
uint256 unusedDepositLimit;
}
​
struct WithdrawalPoolParams {
address token;
address lst;
address pp;
uint256 minWithdrawalAmount;
uint64 minTimeBetweenWithdrawals;
}
​
function setUp() public {
// Deploy implementation contracts
token = new ERC677("MOCK_TOKEN", "MT", 100_000 ether);
deployStakingPool();
deployStrategy();
deployWithdrawalPool();
uint256 rewardThreshold = 0;
rewardsSplitterController = new LSTRewardsSplitterController(
address(stakingPool),
rewardThreshold
);
}
​
function testPushTwiceBatchID() public {
deal(address(token), address(this), 100 ether);
token.approve(address(stakingPool), type(uint256).max);
bytes[] memory data = new bytes[]();
stakingPool.setPriorityPool(address(this));
stakingPool.addStrategy(address(strategy));
stakingPool.deposit(address(this), 1 ether, data);
stakingPool.approve(address(withdrawalPool), type(uint256).max);
withdrawalPool.queueWithdrawal(address(this), 0.1 ether);
withdrawalPool.queueWithdrawal(address(this), 0.1 ether);
token.approve(address(withdrawalPool), type(uint256).max);
// fulfill first withdrawal and part of the second
withdrawalPool.deposit(0.15 ether);
// fulfill part of the remaining part of the second withdrwal
withdrawalPool.deposit(0.045 ether);
// the two batches are identical
(uint128 withdrawalIdBatch1, ) = withdrawalPool.withdrawalBatches(1); // change withdrawalBatches visibility to public
(uint128 withdrawalIdBatch2, ) = withdrawalPool.withdrawalBatches(2);
assertEq(withdrawalIdBatch1, withdrawalIdBatch2);
}
​
function deployStakingPool() public {
StakingPool.Fee[] memory _fees = new StakingPool.Fee[]();
​
StakingPool.Fee memory fee = StakingPool.Fee({
receiver: address(this),
basisPoints: 100
});
_fees[0] = fee;
​
StakingPoolParams memory params = StakingPoolParams({
token: address(token),
name: "LIQUID_TOKEN",
symbol: "LT",
fee: _fees,
unusedDepositLimit: 1000 ether
});
​
bytes memory _data = abi.encodeWithSignature(
"initialize(address,string,string,(address,uint256)[],uint256)",
params.token,
params.name,
params.symbol,
params.fee,
params.unusedDepositLimit
);
​
ERC1967Proxy stakingPoolProxy = new ERC1967Proxy(
address(stakingPoolImpl),
_data
);
stakingPool = StakingPool(address(stakingPoolProxy));
}
​
function deployStrategy() public {
bytes memory _data;
​
uint256 _maxDeposits = type(uint256).max;
uint256 _minDeposits = 0;
​
_data = abi.encodeWithSignature(
"initialize(address,address,uint256,uint256)",
address(token),
address(stakingPool),
_maxDeposits,
_minDeposits
);
​
ERC1967Proxy strategyProxy = new ERC1967Proxy(
address(strategyImpl),
_data
);
strategy = StrategyMock(address(strategyProxy));
}
​
function deployWithdrawalPool() public {
WithdrawalPoolParams memory params = WithdrawalPoolParams({
token: address(token),
lst: address(stakingPool),
pp: address(this),
minWithdrawalAmount: 0,
minTimeBetweenWithdrawals: 0
});
​
bytes memory _data = abi.encodeWithSignature(
"initialize(address,address,address,uint256,uint64)",
params.token,
params.lst,
params.pp,
params.minWithdrawalAmount,
params.minTimeBetweenWithdrawals
);
​
ERC1967Proxy withdrawalPoolProxy = new ERC1967Proxy(
address(withdrawalPoolImpl),
_data
);
withdrawalPool = WithdrawalPool(address(withdrawalPoolProxy));
}
}
​

Tools Used

Manual review.

Recommendations

Only push the batch ID to the array if it was not done before:

if (
withdrawalBatches[withdrawalBatches.length - 1]
.indexOfLastWithdrawal != uint128(i - 1)
) {
withdrawalBatches.push(
WithdrawalBatch(
uint128(i - 1),
uint128(_getStakeByShares(1 ether))
)
);
}
Updates

Lead Judging Commences

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