First Flight #18: T-Swap

First Flight #18
Beginner FriendlyDeFiFoundry
100 EXP
View results
Submission Details
Severity: high
Valid

Incorrect fee calculation in `TSwapPool::getInputAmountBasedOnOutput` causes protocll to take too many tokens from users, resulting in lost fees

Lack of slippage protection in TSwapPool::swapExactOutput causes users to potentially receive way fewer tokens

Description: The swapExactOutput function does not include any sort of slippage protection. A similar case is found in TSwapPool::swapExactInput, here the function specifies a minOutputAmount, the swapExactOutput function should specify a maxInputAmount.

Impact: If market conditions change before the transaciton processes, the user could get a much worse swap.

Proof of Concept:

  1. The price of 1 WETH right now is 1,000 USDC

  2. User inputs a swapExactOutput looking for 1 WETH

    1. inputToken = USDC

    2. outputToken = WETH

    3. outputAmount = 1

    4. deadline = whatever

  3. The function does not offer a maxInput amount

  4. As the transaction is pending in the mempool, the market changes! And the price moves HUGE -> 1 WETH is now 10,000 USDC. 10x more than the user expected

  5. The transaction completes, but the user sent the protocol 10,000 USDC instead of the expected 1,000 USDC

Recommended Mitigation: We should include a maxInputAmount so the user only has to spend up to a specific amount, and can predict how much they will spend on the protocol.

function swapExactOutput(
IERC20 inputToken,
+ uint256 maxInputAmount,
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);
+ if(inputAmount > maxInputAmount){
+ revert();
+ }
_swap(inputToken, inputAmount, outputToken, outputAmount);
Updates

Appeal created

inallhonesty Lead Judge 12 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Incorrect fee calculation in TSwapPool::getInputAmountBasedOnOutput causes protocol to take too many tokens from users, resulting in lost fees

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.