First Flight #18: T-Swap
First Flight #18
Beginner FriendlyDeFiFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Incorrect fee calculation in `TSwapPool::getInputAmountBasedOnOutput` causes protocol to overtax users with a 90.3% fee

paprikrumplikas

Summary

TSwapPool::getInputAmountBasedOnOutput overtaxes users with a 90.3% swap fee.

Vulnerability Details

The getInputAmountBasedOnOutput function is intended to calculate the amount of tokens the users should deposit given the amount of output tokens. However, the function currently miscalculates the resulting amount. When calculating the fee, it scales the amount by 10_000 instead of 1_000, resulting in a 90.3% fee.

Consider the following scenario:

  1. The user calls TSwapPool::swapExactOutput to buy a predefined amount of output tokens in exchange for an undefined amount of input tokens.

  2. TSwapPool::swapExactOutput calls the getInputAmountBasedOnOutput function that is supposed to calculate the amount of input tokens required to result in the predefined amount of output tokens. However, the fee calculation in this function in incorrect.

  3. The user gets overtaxed with a 90.4% fee.

Also consider the following test as proof of code:

Proof of Code
function test_overTaxingUsersInSwapExactOutput() public {
// providing liquidity to the pool
uint256 initialLiquidity = 100e18;
​
vm.startPrank(liquidityProvider);
weth.approve(address(pool), initialLiquidity);
poolToken.approve(address(pool), initialLiquidity);
​
pool.deposit({
wethToDeposit: initialLiquidity,
minimumLiquidityTokensToMint: 0,
maximumPoolTokensToDeposit: initialLiquidity,
deadline: uint64(block.timestamp)
});
vm.stopPrank();
​
// @audit this function actually returns an incorrect value
// uint256 priceOfOneWeth = pool.getPriceOfOneWethInPoolTokens();
​
address someUser = makeAddr("someUser");
uint256 userInitialPoolTokenBalance = 11e18;
poolToken.mint(someUser, 11e18); // now the user has 11 pool tokens
​
// user intends to buy 1 weth with pool tokens
vm.startPrank(someUser);
poolToken.approve(address(pool), type(uint256).max);
pool.swapExactOutput(poolToken, weth, 1e18, uint64(block.timestamp));
vm.stopPrank();
​
uint256 userEndingPoolTokenBalance = poolToken.balanceOf(someUser);
​
console.log("Initial user balance: ", userInitialPoolTokenBalance);
// console.log("Price of 1 weth: ", priceOfOneWeth);
console.log("Ending user balance: ", userEndingPoolTokenBalance);
​
// Initial liquidity was 1:1, so user should have paid ~1 pool token
// However, it spent much more than that. The user started with 11 tokens, and now only has less than 1.
assert(userEndingPoolTokenBalance < 1 ether);
}

Impact

The protocol takes way more fees than expected by the users.

Tools Used

Manual review, Foundry.

Recommendations

Perform the following corrections in TSwapPool::getInputAmountBasedOnOutput:

function getInputAmountBasedOnOutput(
uint256 outputAmount,
uint256 inputReserves,
uint256 outputReserves
)
public
pure
revertIfZero(outputAmount)
revertIfZero(outputReserves)
returns (uint256 inputAmount)
{
- return ((inputReserves * outputAmount) * 10_000) / ((outputReserves - outputAmount) * 997);
+ return ((inputReserves * outputAmount) * 1_000) / ((outputReserves - outputAmount) * 997);
​
}
Updates

Appeal created

inallhonesty Lead Judge about 1 year 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.