Summary
addLiquidityProportional() is used to add liquidity. The issue is that while the maximum number of deposits allowed for a user is 100, 101 deposits can actually be made due to the use of > instead of >=.
Vulnerability Details
The following can be found in the contract's @notice.
The user is restricted to 100 deposits to avoid Ddos issues.
With this in mind, the addLiquidityProportional() is implemented as follows.
function addLiquidityProportional(
address pool,
uint256[] memory maxAmountsIn,
uint256 exactBptAmountOut,
bool wethIsEth,
bytes memory userData
) external payable saveSender(msg.sender) returns (uint256[] memory amountsIn) {
@> if (poolsFeeData[pool][msg.sender].length > 100) {
revert TooManyDeposits(pool, msg.sender);
}
amountsIn = _addLiquidityProportional(
pool,
msg.sender,
address(this),
maxAmountsIn,
exactBptAmountOut,
wethIsEth,
userData
);
uint256 tokenID = lpNFT.mint(msg.sender);
uint256 depositValue = getPoolLPTokenValue(
IUpdateWeightRunner(_updateWeightRunner).getData(pool),
pool,
MULDIRECTION.MULDOWN
);
poolsFeeData[pool][msg.sender].push(
FeeData({
tokenID: tokenID,
amount: exactBptAmountOut,
lpTokenDepositValue: depositValue,
blockTimestampDeposit: uint40(block.timestamp),
upliftFeeBps: upliftFeeBps
})
);
nftPool[tokenID] = pool;
}
As seen in the first line, the function checks if the user has made 100 deposits and reverts if that's the case. However, the issue lies in the use of > instead of >=, which allows the user to make 101 deposits instead of just 100.
The error arises from not accounting for the fact that the deposit at index 0 is considered the first one, remember deposits = length + 1.
For instance, if the length of the array is 100, there are actually 101 deposits, as the index range from 0 to 100 includes 101 numbers. This is why users are able to make 101 deposits instead of the intended 100.
POC
To better illustrate the issue, copy the following POC into UpliftExample.t.sol.
function test_101Deposits() public {
uint256[] memory maxAmountsIn = [dai.balanceOf(bob), usdc.balanceOf(bob)].toMemoryArray();
vm.startPrank(bob);
uint256 bptAmountDeposit = bptAmount / 101;
for (uint256 i = 0; i < 101; i++) {
upliftOnlyRouter.addLiquidityProportional(pool, maxAmountsIn, bptAmountDeposit, false, bytes(""));
}
(uint256 tokenID, uint256 amount, uint256 lpTokenDepositValue, uint40 blockTimestampDeposit, uint64 upliftFeeBps) = upliftOnlyRouter.poolsFeeData(pool, bob, 100);
vm.stopPrank();
}
Impact
Deposits can be bypassed by allowing 101 deposits instead of the intended 100.
Tools Used
Manual review.
Recommendations
To resolve the issue, replace > with >=.
function addLiquidityProportional(
address pool,
uint256[] memory maxAmountsIn,
uint256 exactBptAmountOut,
bool wethIsEth,
bytes memory userData
) external payable saveSender(msg.sender) returns (uint256[] memory amountsIn) {
- if (poolsFeeData[pool][msg.sender].length > 100) {
+ if (poolsFeeData[pool][msg.sender].length >= 100) {
revert TooManyDeposits(pool, msg.sender);
}
// Do addLiquidity operation - BPT is minted to this contract.
amountsIn = _addLiquidityProportional(
pool,
msg.sender,
address(this),
maxAmountsIn,
exactBptAmountOut,
wethIsEth,
userData
);
uint256 tokenID = lpNFT.mint(msg.sender);
//this requires the pool to be registered with the QuantAMM update weight runner
//as well as approved with oracles that provide the prices
uint256 depositValue = getPoolLPTokenValue(
IUpdateWeightRunner(_updateWeightRunner).getData(pool),
pool,
MULDIRECTION.MULDOWN
);
poolsFeeData[pool][msg.sender].push(
FeeData({
tokenID: tokenID,
amount: exactBptAmountOut,
//this rounding favours the LP
lpTokenDepositValue: depositValue,
//known use of timestamp, caveats are known.
blockTimestampDeposit: uint40(block.timestamp),
upliftFeeBps: upliftFeeBps
})
);
nftPool[tokenID] = pool;
}
