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 protocol to take too many tokens from users

Summary

The getInputAmountBasedOnOutput function miscalculates the fee by scaling the amount by 10,000 instead of 1,000, causing the protocol to take more tokens from users than intended.

Vulnerability Details

The function getInputAmountBasedOnOutput calculates the input amount required to receive a specified output amount of tokens. During this calculation, the function applies a fee by scaling the output amount by 10,000 instead of the correct value of 1,000. This incorrect scaling results in the protocol charging users higher fees than expected.

Impact

The incorrect fee calculation results in users being charged more tokens than necessary, leading to lost fees for the users and potentially reducing their trust in the protocol. This overcharging can significantly affect user experience and the overall usability of the platform.
//(((1000 * 100) * 10000) / ((1000 - 100) * 997)) === 1114.45447453
//(((1000 * 100) * 1000) / ((1000 - 100) * 997)) === 111.445447453

POC

function test_getInputAmountBasedOnOutput_SHOULDBE1000NOT10000() public {
uint256 OutputReserve = 1000;
uint256 inputReserve = 1000;
uint256 outputAmount = 100;
uint256 amountBasedOnOutput = pool.getInputAmountBasedOnOutput(outputAmount, inputReserve, OutputReserve);
vm.expectRevert();
assertGe(OutputReserve, amountBasedOnOutput);
}

Tools Used

Manual Code Review + foundry Unit Test .

Recommendations

Correct the fee scaling factor in the getInputAmountBasedOnOutput function from 10,000 to 1,000. This adjustment ensures the protocol charges the appropriate fee and maintains user trust.

Proposed implementation:

function getInputAmountBasedOnOutput(
uint256 outputAmount,
uint256 inputReserves,
uint256 outputReserves
)
public
pure
revertIfZero(outputAmount)
revertIfZero(outputReserves)
returns (uint256 inputAmount)
{
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.