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

`TSwapPool::getPriceOfOneWethInPoolTokens()` and `TSwapPool::getPriceOfOnePoolTokenInWeth()` return incorrect price values

paprikrumplikas

Vulnerability Details

getPriceOfOneWethInPoolTokens is supposed to return the price of 1 WETH in terms of pool tokens, and TSwapPool::getPriceOfOnePoolTokenInWeth is supposed to return the price of 1 pool token in terms of WETH. However, the return values are incorrect. Both functions return the amount of output tokens, which is not the same as the price of 1 output token in input tokens. (Consider this: as compared to a fee-less protocol, if there are fees, the amount of output tokens should be lower, while the price should be not lower but higher.)

Proof of concept: consider the following scenario:

  1. A user has 1 WETH, and wants to swap it for pool tokens.

  2. The user calls getPriceOfOneWethInPoolTokens and sees an incorrect price that is the inverse of the actual price.

  3. User finds the price appealing and swaps his WETH.

  4. User ends up with a lot less pool tokens than he expected.

Proof of code: Insert this piece of code to TSwapPool.t.sol (note that it demonstrates a different scenario than the one written under "Proof of Concept"):

Proof of Code
/**
* @notice In scenarios where inputAmount is close to the minOutputAmount, even a small loss of precision can lead
* to the outputAmount falling below minOutputAmount, triggering the TSwapPool__OutputTooLow error.
*/
function test_incorrectPriceValueReturnedByGetPriceOfOneWethInPoolTokens() public {
uint256 precision = 1 ether;
​
// we need more liquidity in the pool, so granting additional money for the provider
weth.mint(liquidityProvider, 800e18);
poolToken.mint(liquidityProvider, 800e18);
​
// providing liquidity to the pool
uint256 initialLiquidity = 1000e18;
​
vm.startPrank(liquidityProvider);
weth.approve(address(pool), initialLiquidity);
poolToken.approve(address(pool), initialLiquidity);
​
pool.deposit({
wethToDeposit: initialLiquidity,
minimumLiquidityTokensToMint: 0,
maximumPoolTokensToDeposit: initialLiquidity,
deadline: uint64(block.timestamp)
});
vm.stopPrank();
​
uint256 incorrectPriceOfOneWeth = pool.getPriceOfOneWethInPoolTokens();
uint256 correctPriceOfOneWeth = (1e18 * precision) / incorrectPriceOfOneWeth;
​
console.log("Incorrect price: ", incorrectPriceOfOneWeth); // 987_158_034_397_061_298 = 9.87*10**17
console.log("Correct price: ", correctPriceOfOneWeth);
​
address userSuccess = makeAddr("userSuccess");
address userFail = makeAddr("userFail");
​
// userFail attempts to buy 1 weth with a balance of pool tokens that equals the incorrect price of 1 weth
poolToken.mint(userFail, incorrectPriceOfOneWeth);
vm.startPrank(userFail);
poolToken.approve(address(pool), type(uint256).max);
// expect a revert (specifically, TSwapPool__OutputTooLow)
vm.expectRevert();
// using swapExactOutput() would be more appropriate here, but that one has a huge bug
pool.swapExactInput({
inputToken: poolToken,
inputAmount: incorrectPriceOfOneWeth,
outputToken: weth,
minOutputAmount: 99999e13, // due to precision loss, we cant really expect to get 1 full weth (1000e15), we
// can only approximate
deadline: uint64(block.timestamp)
});
vm.stopPrank();
​
// userSuccess attempts to buy 1 weth with a balance of pool tokens that equals the correct price of 1 weth
poolToken.mint(userSuccess, correctPriceOfOneWeth);
vm.startPrank(userSuccess);
poolToken.approve(address(pool), type(uint256).max);
// using swapExactOutput() would be more appropriate here, but that one has a huge bug
pool.swapExactInput({
inputToken: poolToken,
inputAmount: correctPriceOfOneWeth,
outputToken: weth,
minOutputAmount: 99999e13, // due to precision loss, we cant really expect to get 1 full weth (1000e15), we
// can only approximate
deadline: uint64(block.timestamp)
});
vm.stopPrank();
​
assert(weth.balanceOf(userSuccess) > 999e15); // has nearly 1 full weth
assertEq(poolToken.balanceOf(userSuccess), 0); // spent all his poolToken
}

Impact

User will think that the WETH / pool token is cheaper that it actually is, and they might make their trading decisions based on this incorrect price information. E.g. they might think the price of their token is falling, might panic and sell their tokens to avoid further losses by calling sellPoolTokens().

Tools Used

Manual review, Foundry.

Recommendations

Perform the following changes:

function getPriceOfOneWethInPoolTokens() external view returns (uint256) {
- return getOutputAmountBasedOnInput(
- 1e18, i_wethToken.balanceOf(address(this)), i_poolToken.balanceOf(address(this))
- );
+ uint256 precision = 1e18;
+ uint256 amountOfPoolTokensReceivedForOneWeth = getOutputAmountBasedOnInput(
+ 1e18, i_wethToken.balanceOf(address(this)), i_poolToken.balanceOf(address(this)));
+
+ uint256 priceOfOneWethInPoolTokens = (1e18 * precision) / amountOfPoolTokensReceivedForOneWeth;
+
+ return priceOfOneWethInPoolTokens;
}
​
function getPriceOfOnePoolTokenInWeth() external view returns (uint256) {
- return getOutputAmountBasedOnInput(
- 1e18, i_poolToken.balanceOf(address(this)), i_wethToken.balanceOf(address(this))
- );
+ uint256 precision = 1e18;
+ uint256 amountOfWethReceivedForOnePoolToken = getOutputAmountBasedOnInput(
+ 1e18, i_poolToken.balanceOf(address(this)), i_wethToken.balanceOf(address(this)));
+
+ uint256 priceOfOnePoolTokenInWeth = (1e18 * precision) / amountOfWethReceivedForOnePoolToke;
+
+ return priceOfOnePoolTokenInWeth;
}
Updates

Appeal created

inallhonesty Lead Judge 12 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Hardcoded decimal value leads to incorrect conversion when ERC20 does not use 18 decimals.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.