Summary
When TSwapPool::swapExactInput is called it's make the swap but don't return the output amount to the user.
Vulnerability Details
When TSwapPool::swapExactInput is called we don't have the output amount returned as we can see below:
function swapExactInput(
IERC20 inputToken,
uint256 inputAmount,
IERC20 outputToken,
uint256 minOutputAmount,
uint64 deadline
)
public
revertIfZero(inputAmount)
revertIfDeadlinePassed(deadline)
returns (uint256 output)
{
uint256 inputReserves = inputToken.balanceOf(address(this));
uint256 outputReserves = outputToken.balanceOf(address(this));
uint256 outputAmount = getOutputAmountBasedOnInput(
inputAmount,
inputReserves,
outputReserves
);
if (outputAmount < minOutputAmount) {
revert TSwapPool__OutputTooLow(outputAmount, minOutputAmount);
}
_swap(inputToken, inputAmount, outputToken, outputAmount);
}
Impact
Because of lack this return, we don't have the output amount that user receives when make the swap
Tools Used
Solidity and Foundry
Proof of Concept
Add the following PoC to test/unit/TSwapPool.t.sol:
function testDepositSwapExactInput() public {
vm.startPrank(liquidityProvider);
weth.approve(address(pool), 100e18);
poolToken.approve(address(pool), 100e18);
pool.deposit(100e18, 100e18, 100e18, uint64(block.timestamp));
vm.stopPrank();
vm.startPrank(user);
poolToken.approve(address(pool), 10e18);
uint256 expected = 9e18;
uint256 userInitialPoolTokenBalance = poolToken.balanceOf(user);
uint256 userInitialWethBalance = weth.balanceOf(user);
uint256 outputAmount = pool.swapExactInput(poolToken, userInitialPoolTokenBalance, weth, expected, uint64(block.timestamp));
uint256 userFinalPoolTokenBalance = poolToken.balanceOf(user);
uint256 userFinalWethBalance = weth.balanceOf(user);
assertEq(userFinalPoolTokenBalance, 0, "The balance of pool tokens should be 0");
assertEq(userFinalWethBalance > userInitialWethBalance, true, "The balance of WETH should be greater than the initial balance");
assertEq(outputAmount >= expected, true, "The output amount should be greather than or equal to the expected amount");
}
Recommendations
In the TSwapPool::swapExactInput you could:
Rename from returns (uint256 output) to returns (uint256 outputAmount)
And remove the other declaration from uint256 outputAmount = getOutputAmountBasedOnInput( to outputAmount = getOutputAmountBasedOnInput(
function swapExactInput(
IERC20 inputToken,
uint256 inputAmount,
IERC20 outputToken,
uint256 minOutputAmount,
uint64 deadline
)
public
revertIfZero(inputAmount)
revertIfDeadlinePassed(deadline)
- returns (uint256 output)
+ returns (uint256 outputAmount)
{
uint256 inputReserves = inputToken.balanceOf(address(this));
uint256 outputReserves = outputToken.balanceOf(address(this));
- uint256 outputAmount = getOutputAmountBasedOnInput(
+ outputAmount = getOutputAmountBasedOnInput(
inputAmount,
inputReserves,
outputReserves
);
if (outputAmount < minOutputAmount) {
revert TSwapPool__OutputTooLow(outputAmount, minOutputAmount);
}
_swap(inputToken, inputAmount, outputToken, outputAmount);
}
