Submission Details
Severity:
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 about 1 year ago
Submission Judgement Published Assigned finding tags:
Incorrect fee calculation in TSwapPool::getInputAmountBasedOnOutput causes protocol to take too many tokens from users, resulting in lost fees
Rewards breakdown
nSLOC
276This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.