Summary
Input is handled incorrectly which provide Incorrect amount to tokens to users.
Vulnerability Details
sellPoolTokens function is used to allows users to make sells of given tokens and get WETH as a output. User can input, how much tokens they want to sell by specifying the poolTokenAmount parameter. But current logic in the correct incorrectly handle this input and miscalculate the swapped amount.
function sellPoolTokens(
uint256 poolTokenAmount
) external returns (uint256 wethAmount) {
return
@> swapExactOutput(
i_poolToken,
i_wethToken,
poolTokenAmount,
uint64(block.timestamp)
);
}
function swapExactOutput(
IERC20 inputToken,
IERC20 outputToken,
@> uint256 outputAmount,
uint64 deadline
)
public
revertIfZero(outputAmount)
revertIfDeadlinePassed(deadline)
returns (uint256 inputAmount)
{
uint256 inputReserves = inputToken.balanceOf(address(this));
uint256 outputReserves = outputToken.balanceOf(address(this));
inputAmount = getInputAmountBasedOnOutput(
outputAmount,
inputReserves,
outputReserves
);
_swap(inputToken, inputAmount, outputToken, outputAmount);
}
As we can see, the swapExactOutput function is being used which is incorrect in current context.
As users is specifying the exact amount of input tokens so swapExactInput should be called.
POC
In existing test suite, add following test -
function testAmountMismatchDueToWrongInputHandling () public {
vm.startPrank(liquidityProvider);
weth.approve(address(pool), 100e18);
poolToken.approve(address(pool), 50e18);
pool.deposit(100e18, 100e18, 50e18, uint64(block.timestamp));
assertEq(pool.balanceOf(liquidityProvider), 100e18);
assertEq(weth.balanceOf(liquidityProvider), 100e18);
vm.stopPrank();
vm.startPrank(user);
poolToken.approve(address(pool), 10e18);
console.log("user token balance before swap:", poolToken.balanceOf(user));
console.log("user weth balance before swap: ", weth.balanceOf(user));
pool.sellPoolTokens(1e18);
console.log("user balance:", poolToken.balanceOf(user));
console.log("user weth balance after swap: ", weth.balanceOf(user));
}
now run forge test --mt testAmountMismatchDueToWrongInputHandlin -vv and it will show following results in output.
[⠊] Compiling...
[⠑] Compiling 1 files with Solc 0.8.20
[⠘] Solc 0.8.20 finished in 1.63s
Compiler run successful :
Ran 1 test for test/unit/TSwapPool.t.sol:TSwapPoolTest
[PASS] testAmountMismatchDueToWrongInputHandling() (gas: 233011)
Logs:
user token balance before swap: 10000000000000000000
user weth balance before swap: 10000000000000000000
user balance: 4934297843024021560
user weth balance after swap: 11000000000000000000
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.15ms (244.79µs CPU time)
Ran 1 test suite in 154.09ms (1.15ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
As we can see that we inputted 1 token but it deducted more than 5 tokens and output token we received was exactly 1 weth. This means input and output amounts are being flipped at the moment at function level.
Impact
Users will get mismatched amounts of tokens, which will affect the core protocol.
Tools Used
Manual Review
Recommendations
Update the current logic to use swapExactInput instead of swapExactOutput. To make this change properly, sellPoolTokens function requires a new parameter (ie minWethToReceive to be passed to swapExactInput).
Example implementation is attached below:
function sellPoolTokens(
uint256 poolTokenAmount,
+ uint256 minWethToReceive,
) external returns (uint256 wethAmount) {
- return swapExactOutput(i_poolToken, i_wethToken, poolTokenAmount, uint64(block.timestamp));
+ return swapExactInput(i_poolToken, poolTokenAmount, i_wethToken, minWethToReceive, uint64(block.timestamp));
}
