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

Incorrect return function implemented for `TSwapPool::sellPoolTokens` causes user to receive incorrect amount of weth and incorrect amount of pool token being deducted after sell transaction

soupy

Summary

The function TSwapPool::sellPoolTokens is used by user to sell the pool token in exchange of weth. User inputs the amount of pool token he intends to sell and the function will calculate the weth that he will get in return. However, TSwapPool::sellPoolTokens wrongly implement the return function swapExactOutput resulting incorrect amount of weth paid to user and incorrect amount of pool token being deducted from user account after the sell transaction

Vulnerability Details

According to the natspec, TSwapPool::sellPoolTokens is used to facilitate users selling pool tokens in exchange of weth. However, TSwapPool::sellPoolTokens implements wrong function swapExactOutput to calculate the weth amount user should get after the user inputs the amount of pool token he intends to sell. The correct return value should be calculated through function swapExactInput instead

Proof of Concept

Place the following test case in test/unit/TSwapPool.t.sol :

** Prerequisite:
In contract TswapPool, correct back another audit finding ( change 10000 to 1000) on getInputAmountBasedOnOutput under the swapExactOutput, so that we only test swapExactOutput validity in the sellPoolToken function and not influenced by the error caused by other factor.

​
function testSellPoolTokenReturnValues() public {
// injecting funds into pool by liquidity provider
vm.startPrank(liquidityProvider);
weth.approve(address(pool), 100e18);
poolToken.approve(address(pool), 100e18);
pool.deposit(100e18, 100e18, 100e18, uint64(block.timestamp));
vm.stopPrank();
​
// starting pranking that a user want to sell his pool token
vm.startPrank(user);
uint256 balanceWethB4Sell = weth.balanceOf(user); // will return 10e18 as minted in setUp
uint256 balancePoolTokenB4Sell = poolToken.balanceOf(user); // will return 10e18 as minted in setUp
console.log("balance weth_user before sell: ", balanceWethB4Sell);
console.log("balance poolToken_user before sell: ", balancePoolTokenB4Sell);
​
poolToken.approve(address(pool), 10e18);
​
// assuming user only want to sell 1e16 poolToken out of his original poolToken amount of 10e18
uint256 poolTokenAmountToSell = 1e16;
console.log("poolTokenAmountToSell: ", poolTokenAmountToSell);
​
// Prerequisite: correct back another audit finding ( change 10000 to 1000) on `getInputAmountBasedOnOutput` under the `swapExactOutput`, so that we only test `swapExactOutput` validity in the `sellPoolToken` function and not influenced by the error caused by other factor.
// As formulated in `getInputAmountBasedOnOutput`, calculatedWethToReceive = ((100e18 * 1e16) * uint256(1000)) / ((100e18 - 1e16) * uint256(997)) which factor in the fee charged by the protocol
uint256 calculatedWethToReceive = pool.sellPoolTokens(poolTokenAmountToSell);
console.log("calculatedWethToReceive: ", calculatedWethToReceive);
​
// check balance after the sell transaction
uint256 balanceWethAfterSell = weth.balanceOf(user);
uint256 balancePoolTokenAfterSell = poolToken.balanceOf(user);
​
// tapout the expected balance of weth and poolToken after the sell
// factoring in the fee charged by protocol for the execution of the transaction
uint256 expectedBalanceWethAftSell = balanceWethB4Sell + calculatedWethToReceive;
uint256 expectedBalancePoolTokenAftSell = balancePoolTokenB4Sell - poolTokenAmountToSell;
​
console.log("balanceWethAfterSell: ", balanceWethAfterSell);
console.log("expectedBalanceWethAftSell: ", expectedBalanceWethAftSell);
​
console.log("balancePoolTokenAfterSell: ", balancePoolTokenAfterSell);
console.log("expectedPoolTokenBalanceAftSell: ", expectedBalancePoolTokenAftSell);
​
assertEq(balanceWethAfterSell, expectedBalanceWethAftSell);
assertEq(balancePoolTokenAfterSell, expectedBalancePoolTokenAftSell);
}
​

The test above will fail indicating that the currect implementation of swapExactOutput in TSwapPool::sellPoolTokens is wrong. However if we change the return function from swapExactOutput to swapExactInput with the correct corresponding input parameters and return variable, the test above will pass.

Impact

User will receive an incorrect amount of weth and also will have incorrect amount of pool token being deducted as the result of the execution of TSwapPool::sellPoolTokens with wrong return function implemented

Tools Used

Manual review and test case

Recommendations

Make correction to the return function used in TSwapPool::sellPoolTokens

​
function sellPoolTokens(
uint256 poolTokenAmount,
+ uint256 minOutputAmount,
) external returns (uint256 wethAmount) {
return
- swapExactOutput(
- i_poolToken,
- i_wethToken,
- poolTokenAmount,
- uint64(block.timestamp)
- );
​
+ swapExactInput(
+ i_poolToken,
+ poolTokenAmount,
+ i_wethToken,
+ minOutputAmount,
+ uint64(block.timestamp)
+ );
}
Updates

Appeal created

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

`sellPoolTokens` mismatches input and output tokens causing users to receive the incorrect amount of tokens

Support

FAQs

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