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

`TSwapPool::getInputAmpuntBasedOnOutput` uses incorrect scaling factor of `10000` causing user to be charged with higher fee

4lifemen

Summary

The function getInputAmountBasedOnOutput in the TSwapPool contract incorrectly uses a scaling factor of 10000 instead of 1000 when calculating fees. This results in users being charged an excessively high fee.

Vulnerability Details

Wrong scaling factor of 10000 being used in fee calculation.

function getInputAmountBasedOnOutput(
....
{
return ((inputReserves * outputAmount) * 10000) / ((outputReserves - outputAmount) * 997);
}
Proof of Concept

Add this function in TSwapPool.t.sol

function testFeeCalculation() public view {
uint256 outputAmount = 1e18; // 1 token
uint256 inputReserves = 100e18; // 100 tokens in reserve
uint256 outputReserves = 200e18; // 200 tokens in reserve
​
// Expected input amount with correct fee calculation
uint256 expectedInputAmount = ((inputReserves * outputAmount) * 1000) / ((outputReserves - outputAmount) * 997);
​
// Actual input amount with incorrect fee calculation (current implementation)
uint256 actualInputAmount = pool.getInputAmountBasedOnOutput(outputAmount, inputReserves, outputReserves);
​
//
assert(expectedInputAmount < actualInputAmount);
}

Actual input token amount is more than expected implying an extra fee of more than 0.3% being charged on user

Impact

The incorrect scaling factor results in users being charged a fee approximately 10 times higher than intended. This can lead to significant financial losses for users, as they received much lesser token than expected. This discrepancy can reduce trust in the protocol.

Tools Used

  • Manual review

Recommendations

Replace the magic numbers with defined constants to ensure the fee calculations are correct and consistent.

+ uint256 constant FEE_NUMERATOR = 997;
+ uint256 constant FEE_DENOMINATOR = 1000;
​
function getInputAmountBasedOnOutput(
...
{
+ return ((inputReserves * outputAmount) * FEE_DENOMINATOR) / ((outputReserves - outputAmount) * FEE_NUMERATOR);
}
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.