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

`TSwapPool::getInputAmountBasedOnOutput` miscalculation leads to users overpaying

kildren

Summary

According to the documentation, the protocol charges a 0.3% fee. Therefore, getInputAmountBasedOnOutput computes the fee that the user needs to pay, using 0.997 * InputAmount to exchange outputAmount. However, in the calculation involving 0.997, it erroneously uses 10,000 instead of 1,000, leading to users paying significantly more than expected.

Vulnerability Details

According to the constant product formula in the documentation, the inputAmount can be calculated as inputAmount = outputAmount * inputReserves / (outputReserves - outputAmount). The protocol charges a 0.3% fee, so the actualInputAmount should be inputAmount / 0.997, which is outputAmount * inputReserves * 1,000 / (outputReserves - outputAmount * 0.997). However, the implementation incorrectly uses 10_000 instead of 1_000.

Proof of Concept

Place the following test into TSwapPool.t.sol. The following test shows that the TSwapPool::getInputAmountBasedOnOutput is calculating the price as ten times the expected value:

function testInputAmountBasedOutput() public {
vm.startPrank(liquidityProvider);
weth.approve(address(pool), 100e18);
poolToken.approve(address(pool), 100e18);
pool.deposit(100e18, 100e18, 100e18, uint64(block.timestamp));
vm.stopPrank();
​
// Obviously, at this point, the probability of weth and poolToken is clearly 1:1
// If user wants to swap 5e18 weth, user needs to give the pool 5e18 poolTokens
// Considering the protocol's 0.3% fee, the user needs to
// provide approximately 5e18 / 0.997 ≈ 5.015e18 poolTokens
// However, due to the calculation error, the user needs to spend 5e19 poolTokens,
// which exceeds the user's balance and makes the transaction impossible.
​
vm.startPrank(user);
poolToken.approve(address(pool), type(uint256).max);
uint256 expectWeth = 5e18;
vm.expectRevert();
// The details in the foundry test show that the user attempted to transfer 5e19 poolTokens to the pool.
pool.swapExactOutput(poolToken, weth, expectWeth, uint64(block.timestamp));
vm.stopPrank();
}

Impact

Due to this error, the actual fee paid by users is inputAmount / 0.0997, which is approximately ten times the correct fee, leading to significant losses for users.

Tools Used

Manual review

Recommendations

It's best to use variables instead of magic numbers to completely avoid making such mistakes again. If must use numbers, it's better to use _ for separation to reduce the likelihood of errors.

{
return
- ((inputReserves * outputAmount) * 10000) /
+ ((inputReserves * outputAmount) * 1_000) /
((outputReserves - outputAmount) * 997);
}
Updates

Appeal created

inallhonesty Lead Judge over 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.