First Flight #18: T-Swap

First Flight #18
Beginner FriendlyDeFiFoundry
100 EXP
View results
Submission Details
Severity: high
Valid

`TSwapPool::sellPoolTokens` mismatches input and output tokens causing users to spend more than intended

Summary

The TSwapPool::sellPoolTokens function is intended to allow users to easily sell pool tokens and receive WETH in exchange. Users indicate how many pool tokens they're willing to sell in the poolTokenAmount parameter. However, the function currently miscalculates the swapped amount, making the user spend more than intended to get the exact amount of WETH specified in the parameter.

Impact

Users will swap the wrong amount of tokens, which breaks the protocol functionality.

Proof of Concept

Add this to test/uint/TSwapPool.t.sol

function testSellPoolTokensWorksBackwards() public {
// Liquidity provider adds liquidity to the pool
testDeposit();
uint256 USER_POOL_TOKENS = 1e18;
poolToken.mint(user, USER_POOL_TOKENS);
uint256 balanceWethBefore = weth.balanceOf(address(user));
uint256 balancePoolTokenBefore = poolToken.balanceOf(address(user));
vm.startPrank(user);
poolToken.approve(address(pool), type(uint256).max);
pool.sellPoolTokens(USER_POOL_TOKENS);
uint256 balanceWethAfter = weth.balanceOf(address(user));
uint256 balancePoolTokenAfter = poolToken.balanceOf(address(user));
// Here we assert that the user bought the amount of WETH specified in the parameter of sellPoolTokens
assert(balanceWethAfter - balanceWethBefore == USER_POOL_TOKENS);
// And here we assert the sellPoolTokens makes the user spend more than intended in the badly named parameter
assert(balancePoolTokenBefore - balancePoolTokenAfter > USER_POOL_TOKENS);
}

Tools Used

Foundry and manual review

Recommendations

Recommended Mitigation: Consider changing the implementation to use swapExactInput instead of swapExactOutput. Note that this would also require changing the sellPoolTokens function to accept a new parameter (ie minWethToReceive to be passed to swapExactInput).

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

Additionally, it might be wise to add a deadline to the function, as there is currently no deadline. Could expose users to MEV attacks.

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.