Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The lack of a WETH-Profits Token pair upon calling sellProfits can expose it to malicious pool creation #2118

Open
codehawks-bot opened this issue Aug 8, 2023 · 3 comments

Comments

@codehawks-bot
Copy link

The lack of a WETH-Profits Token pair upon calling sellProfits can expose it to malicious pool creation

Severity

Medium Risk

Relevant GitHub Links

https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Fees.sol#L26-L44

Summary

Some tokens do not have Uniswap V3 pools with WETH, which allows a bad actor to frontrun the transaction and create a pool with an offset liquidity ratio and subsequently steal the funds.

Vulnerability Details

When calling sellProfits the function calls exactInputSingle on the Uniswap V3 SwapRouter with a token pair of WETH and the profits token address from calldata. In the case of a token pair between the two not existing the call will revert. This absence of such a pool can be used maliciously by a bad actor to frontrun the call and create a pool with an offset price so they can extract the tokens.

Impact

The tokens will be mostly lost due to the bad exchange rate. This will occur very rarely due to most tokens already having Uniswap v3 pairs with WETH, hence the medium severity.

Tools Used

Manual Review

Recommendations

Consider using swapExactInputMultihop with a path that is first swapping the less popular token into some other token with higher liquidity like USDC. It will be better for the specific case of the contract due to the arbitrary nature of the tokens it will be working with.

@PatrickAlphaC
Copy link
Member

I disagree with the recommendation. I think the recommendation should be to abandon Uniswap and use auctions instead.

@kosedogus
Copy link

Issue #908 has some duplicates that are talking about this exact issue and not hardcoded fee's (908 is about hardcoded fees) I recommend either make this to duplicate of 908 or check for 908's duplicates and make the ones that are specifying token pairs not fee's to this issue.

@PatrickAlphaC
Copy link
Member

The source of the issue is the same, but the actual issue is very different and these are different findings.

One talks about being front-run to lose funds.
One talks about not getting the best price from hopping pools.

Part of the job is convincing the client why an issue exists. A protocol getting front run vs a protocol getting unoptimal pricing due to the fee structure are very different.

Thank you for your escalation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants