hats-finance / Fenix--0x9d7765a7ebd5b6322a30797a44a5428531970d3d

0 stars 1 forks source link

Missing Explicit Slippage Check in buybackTokenByV2 Function #54

Open hats-bug-reporter[bot] opened 1 month ago

hats-bug-reporter[bot] commented 1 month ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0xd38cf251bca8c4490e2a553d8b0505820c77d8ffdfe56c1cca71e4a1e16cf9b1 Severity: high

Description: Description\ The buybackTokenByV2 function in the SingelTokenBuybackUpgradeable contract adjusts the expected output amount (amountOutQuote) based on the specified slippage percentage before executing the token swap. However, the function does not include an explicit check at the end to ensure that the actual received amount (amountOut) adheres to the slippage constraints. This can lead to scenarios where the actual received amount is less than the acceptable range defined by the slippage, potentially resulting in unfavorable trades.

Attack Scenario\ Describe how the vulnerability can be exploited.

Attachments

  1. Proof of Concept (PoC) File
    
    ................

amountOutQuote = amountOutQuote - (amountOutQuote * slippage_) / SLIPPAGE_PRECISION; if (amountOutQuote <= 0) { revert RouteNotFound(); }

IRouterV2 router = IRouterV2(routerV2PathProviderCache.router()); inputTokenCache.safeApprove(address(router), 0); // Reset approval to zero first inputTokenCache.safeApprove(address(router), amountIn); // Set new approval

uint256 balanceBefore = IERC20(targetToken).balanceOf(address(this));

uint256[] memory amountsOut = router.swapExactTokensForTokens(amountIn, amountOutQuote, optimalRoute, address(this), deadline_);

uint256 amountOut = amountsOut[amountsOut.length - 1];

assert(IERC20(targetToken).balanceOf(address(this)) - balanceBefore == amountOut); assert(amountOut > 0);

.....


* Without an explicit check to ensure that the actual received amount adheres to the slippage constraints, the function might complete a trade where the received amount is significantly less than expected. This can result in unfavorable trades and potential loss of value.

* The function relies on the DEX router to execute the swap within the specified slippage range. However, without an explicit check, there is no guarantee that the actual received amount will always meet the slippage constraints, leading to inconsistent behavior.

* Not verifying the actual received amount against the slippage constraints can expose the contract to potential security risks, where an attacker might exploit this behavior to manipulate the received amount.
2. **Revised Code File (Optional)**
<!-- If possible, please provide a second file containing the revised code that offers a potential fix for the vulnerability. This file should include the following information:
- Comment with a clear explanation of the proposed fix.
- The revised code with your suggested changes.
- Any additional comments or explanations that clarify how the fix addresses the vulnerability. -->
- To fix this issue, an explicit check should be added at the end of the function to ensure that the actual received amount adheres to the slippage constraints.

1. Calculate Minimum Acceptable Amount:

uint256 minAcceptableAmount = amountOutQuote - (amountOutQuote * slippage_) / SLIPPAGE_PRECISION;

2. Check Actual Received Amount:

* Check if the actual received amount is greater than or equal to the minimum acceptable amount.

if (amountOut < minAcceptableAmount) { revert SlippageExceeded(); }

0xmahdirostami commented 1 month ago

I think this is similar to issue #7 as their outcomes and issue reasons are the same