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

Incorrect Fee Factor in getInputAmountBasedOnOutput Calculation

josh4324

Summary

The getInputAmountBasedOnOutput function incorrectly uses a fee factor of 10000 instead of the intended 1000 when calculating the input amount required to obtain a specific output amount. This discrepancy can lead to significant overestimation of the required input amount, causing users to provide more input tokens than necessary.

Vulnerability Details

In automated market makers (AMMs) like Uniswap, the correct fee factor is typically derived from the transaction fee structure. For example, with a 0.3% fee, the correct multiplier would be 997 out of 1000. Using a factor of 10000 instead of 1000 significantly distorts the input amount calculations.

function getInputAmountBasedOnOutput(
uint256 outputAmount,
uint256 inputReserves,
uint256 outputReserves
)
public
pure
revertIfZero(outputAmount)
revertIfZero(outputReserves)
returns (uint256 inputAmount)
{
@> return (inputReserves * outputAmount * 10000) / ((outputReserves - outputAmount) * 997);
}

POC

function testFeeFactor() public {
vm.startPrank(liquidityProvider);
weth.approve(address(pool), 200e18);
poolToken.approve(address(pool), 200e18);
pool.deposit(100e18, 100e18, 100e18, uint64(block.timestamp));
​
assertEq(pool.balanceOf(liquidityProvider), 100e18);
assertEq(weth.balanceOf(liquidityProvider), 100e18);
assertEq(poolToken.balanceOf(liquidityProvider), 100e18);
​
assertEq(weth.balanceOf(address(pool)), 100e18);
assertEq(poolToken.balanceOf(address(pool)), 100e18);
​
pool.deposit(100e18, 100e18, 100e18, uint64(block.timestamp));
​
assertEq(pool.balanceOf(liquidityProvider), 200e18);
assertEq(weth.balanceOf(liquidityProvider), 0);
​
uint256 expected =
pool.getInputAmountBasedOnOutput(10e18, weth.balanceOf(address(pool)), poolToken.balanceOf(address(pool)));
​
// assertion will fail because expected result is 105579897587499340125 (105e18)
// this is because of the wrong fee factor 10000 used in the calculation
// the correct fee factor to be used is 1000
assert(expected <= 10e18);
​
vm.stopPrank();
}

Impact

  1. User Overpayment: Users will overpay for their trades, resulting in a much less favorable exchange rate.

  2. Market Inefficiency: The incorrect calculations can lead to substantial inefficiencies in the liquidity pool, affecting the overall trading experience.

Tools Used

Manual Review

Recommendations

  1. Correct Fee Factor: Update the getInputAmountBasedOnOutput function to use the correct fee factor of 1000 instead of 10000.

  2. Review and Testing: Conduct thorough reviews and tests of all functions that involve fee calculations to ensure accuracy.

function getInputAmountBasedOnOutput(
uint256 outputAmount,
uint256 inputReserves,
uint256 outputReserves
)
public
pure
revertIfZero(outputAmount)
revertIfZero(outputReserves)
returns (uint256 inputAmount)
{
- return (inputReserves * outputAmount * 10000) / ((outputReserves - outputAmount) * 997);
+ return (inputReserves * outputAmount * 1000) / ((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.