Summary
When the function TSwapPool::withdraw is called, it don't verify if totalLiquidityTokenSupply` is greather than zero
Vulnerability Details
When the user try to withdraw funds but the Pool contract have a supply of liquidity token equal to zero the function will return a panic: division or modulo by zero because the function don't have verification if totalLiquidityTokenSupply is zero:
function withdraw(
uint256 liquidityTokensToBurn,
uint256 minWethToWithdraw,
uint256 minPoolTokensToWithdraw,
uint64 deadline
)
external
revertIfDeadlinePassed(deadline)
revertIfZero(liquidityTokensToBurn)
revertIfZero(minWethToWithdraw)
revertIfZero(minPoolTokensToWithdraw)
{
uint256 wethToWithdraw =
(liquidityTokensToBurn * i_wethToken.balanceOf(address(this))) / totalLiquidityTokenSupply();
uint256 poolTokensToWithdraw =
(liquidityTokensToBurn * i_poolToken.balanceOf(address(this))) / totalLiquidityTokenSupply();
if (wethToWithdraw < minWethToWithdraw) {
revert TSwapPool__OutputTooLow(wethToWithdraw, minWethToWithdraw);
}
if (poolTokensToWithdraw < minPoolTokensToWithdraw) {
revert TSwapPool__OutputTooLow(poolTokensToWithdraw, minPoolTokensToWithdraw);
}
_burn(msg.sender, liquidityTokensToBurn);
emit LiquidityRemoved(msg.sender, wethToWithdraw, poolTokensToWithdraw);
i_wethToken.safeTransfer(msg.sender, wethToWithdraw);
i_poolToken.safeTransfer(msg.sender, poolTokensToWithdraw);
}
Impact
It will not revert the transaction like TSwapPool__TotalLiquidityTokenSupplyIsZero , but will return a panic error panic: division or modulo by zero
Tools Used
Solidity and Foundry
Proof of Concept
Add the following PoC to test/unit/TSwapPool.t.sol:
function testTryWithdrawWhenTotalLiquidityTokenSupplyIsZero() public {
vm.startPrank(user);
poolToken.approve(address(pool), 10e18);
uint256 liquidityTokensToBurn = 1e18;
uint256 minWethToWithdraw = 1e18;
uint256 minPoolTokensToWithdraw = 1e18;
uint64 deadline = uint64(block.timestamp);
vm.expectRevert(TSwapPool.TSwapPool__TotalLiquidityTokenSupplyIsZero.selector);
pool.withdraw(liquidityTokensToBurn, minWethToWithdraw, minPoolTokensToWithdraw, deadline);
vm.stopPrank();
}
Recommendations
You need to add the custom error in the src/TSwapPool.sol:
error TSwapPool__MustBeMoreThanZero();
+ error TSwapPool__TotalLiquidityTokenSupplyIsZero();
And add the verification on TSwapPool::withdraw:
function withdraw(
uint256 liquidityTokensToBurn,
uint256 minWethToWithdraw,
uint256 minPoolTokensToWithdraw,
uint64 deadline
)
external
revertIfDeadlinePassed(deadline)
revertIfZero(liquidityTokensToBurn)
revertIfZero(minWethToWithdraw)
revertIfZero(minPoolTokensToWithdraw)
{
+ if (totalLiquidityTokenSupply() == 0) {
+ revert TSwapPool__TotalLiquidityTokenSupplyIsZero();
+ }
// We do the same math as above
uint256 wethToWithdraw =
(liquidityTokensToBurn * i_wethToken.balanceOf(address(this))) / totalLiquidityTokenSupply();
uint256 poolTokensToWithdraw =
(liquidityTokensToBurn * i_poolToken.balanceOf(address(this))) / totalLiquidityTokenSupply();
if (wethToWithdraw < minWethToWithdraw) {
revert TSwapPool__OutputTooLow(wethToWithdraw, minWethToWithdraw);
}
if (poolTokensToWithdraw < minPoolTokensToWithdraw) {
revert TSwapPool__OutputTooLow(poolTokensToWithdraw, minPoolTokensToWithdraw);
}
_burn(msg.sender, liquidityTokensToBurn);
emit LiquidityRemoved(msg.sender, wethToWithdraw, poolTokensToWithdraw);
i_wethToken.safeTransfer(msg.sender, wethToWithdraw);
i_poolToken.safeTransfer(msg.sender, poolTokensToWithdraw);
}
