First Flight #18: T-Swap

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

Fee calculation is wrong in `TswapPool::getInputAmountBasedOnOutput` causes the protocol to take too many tokens from users, resulting in lost fees

Description: The getInputAmountBasedOnOutput function is intended to calculate the amount of tokens a user should deposit given an amount of tokens of output tokens. However, the function miscalculates the resulting amount. When calculating the fee, it scales the amount by 10000 instead of 1000.

Impact Protocol takes more fees than expected from users.

Proof of Concept

function testGetInputBasedOnOutputMiscalculatesWhichResultsInLiquidityProviderTwoGainingMoreFess() public {
/**
* firstly we check how much liquidityprovider is able to withdraw after user swaps using swapExactInput which uses the function TswapPool::getOutputAmountBasedOnInput
* which uses the correct calculation for output
*/
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);
uint256 expected = 9e18;
poolToken.approve(address(pool), 10e18);
pool.swapExactInput(poolToken, 10e18, weth, expected, uint64(block.timestamp));
vm.stopPrank();
vm.startPrank(liquidityProvider);
pool.approve(address(pool), 100e18);
pool.withdraw(100e18, 90e18, 100e18, uint64(block.timestamp));
assertEq(pool.totalSupply(), 0);
/**
* Secondly we check how much liquidityproviderTwo is able to withdraw after user swaps using swapExactOutput which uses the function TswapPool::getInputAmountBasedOnOutput
* which uses the wrong calculation for Input
*/
vm.startPrank(liquidityProviderTwo);
weth.approve(address(pool), 100e18);
poolToken.approve(address(pool), 100e18);
pool.deposit(100e18, 100e18, 100e18, uint64(block.timestamp));
vm.stopPrank();
vm.startPrank(user);
uint256 expectedTwo = 9e18;
poolToken.approve(address(pool), 100e18);
pool.swapExactOutput(poolToken, weth, expectedTwo, uint64(block.timestamp));
vm.stopPrank();
vm.startPrank(liquidityProviderTwo);
pool.approve(address(pool), 100e18);
pool.withdraw(100e18, 90e18, 100e18, uint64(block.timestamp));
assertEq(pool.totalSupply(), 0);
//In the end we can verify that liquidityproviderTwo has more funds, even though both liquidityProviders deposited the same amount of tokens
assert(weth.balanceOf(liquidityProvider) + poolToken.balanceOf(liquidityProvider) < weth.balanceOf(liquidityProviderTwo) + poolToken.balanceOf(liquidityProviderTwo));
}

Recommended Mitigation:

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