Summary
The onAfterRemoveLiquidity function in UpliftOnlyExample.sol contains a critical vulnerability where division by zero occurs during the calculation of lpTokenDepositValueChange. This happens because lpTokenDepositValue is initialized to zero and used as a denominator in a division operation, causing the function to revert and preventing users from removing their liquidity.
Vulnerability Details
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;
}
function onAfterRemoveLiquidity(...) {
AfterRemoveLiquidityData memory localData = AfterRemoveLiquidityData({
lpTokenDepositValueNow: 0,
lpTokenDepositValue: 0,
});
localData.lpTokenDepositValueChange =
(int256(localData.lpTokenDepositValueNow) - int256(localData.lpTokenDepositValue)) /
int256(localData.lpTokenDepositValue);
}
) public override onlySelfRouter(router) returns (bool, uint256[] memory hookAdjustedAmountsOutRaw) {
address userAddress = address(bytes20(userData));
AfterRemoveLiquidityData memory localData = AfterRemoveLiquidityData({
pool: pool,
bptAmountIn: bptAmountIn,
amountsOutRaw: amountsOutRaw,
minAmountsOut: new uint256[](amountsOutRaw.length),
accruedFees: new uint256[](amountsOutRaw.length),
accruedQuantAMMFees: new uint256[](amountsOutRaw.length),
currentFee: minWithdrawalFeeBps,
feeAmount: 0,
prices: IUpdateWeightRunner(_updateWeightRunner).getData(pool),
lpTokenDepositValueNow: 0,
lpTokenDepositValueChange: 0,
lpTokenDepositValue: 0,
tokens: new IERC20[](amountsOutRaw.length),
feeDataArrayLength: 0,
amountLeft: 0,
feePercentage: 0,
adminFeePercent: 0
});
hookAdjustedAmountsOutRaw = amountsOutRaw;
localData.lpTokenDepositValueNow = getPoolLPTokenValue(localData.prices, pool, MULDIRECTION.MULDOWN);
FeeData[] storage feeDataArray = poolsFeeData[pool][userAddress];
localData.feeDataArrayLength = feeDataArray.length;
localData.amountLeft = bptAmountIn;
for (uint256 i = localData.feeDataArrayLength - 1; i >= 0; --i) {
localData.lpTokenDepositValue = feeDataArray[i].lpTokenDepositValue;
@> localData.lpTokenDepositValueChange =
@> (int256(localData.lpTokenDepositValueNow) - int256(localData.lpTokenDepositValue)) /
@> int256(localData.lpTokenDepositValue);
POC
Welcome to Chisel! Type !help to show available commands.
➜ uint256 lpTokenDepositValueNow = 0
➜ uint256 lpTokenDepositValue = 0
➜ int256 result = (int256(lpTokenDepositValueNow) - int256(lpTokenDepositValue)) / int256(lpTokenDepositValue)
Traces:
[536] 0xBd770416a3345F91E4B34576cb804a576fa48EB1::run()
└─ ← [Revert] panic: division or modulo by zero (0x12)
⚒️ Chisel Error: Failed to execute REPL contract!
➜
Impact
Funds become locked due to function reverting
Tools Used
Chisel, Github
Recommendations
Add a zero-value check before performing the division:
function onAfterRemoveLiquidity(...) {
if (localData.lpTokenDepositValue == 0) {
localData.lpTokenDepositValueChange = 0;
} else {
localData.lpTokenDepositValueChange =
(int256(localData.lpTokenDepositValueNow) - int256(localData.lpTokenDepositValue)) /
int256(localData.lpTokenDepositValue);
}
}
