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

frndz0ne - Insufficient checks in AmirX::swap will allow change of protocol's state without verification #222

Open
sherlock-admin2 opened this issue Nov 13, 2024 · 0 comments

Comments

@sherlock-admin2
Copy link
Contributor

sherlock-admin2 commented Nov 13, 2024

frndz0ne

High

Insufficient checks in AmirX::swap will allow change of protocol's state without verification

Summary

AmirX::swap function validates stablecoin swap parameters, performs swaps, and handles DeFi interactions based on provided DefiSwap details. Insufficient checks in the function allow the StablecoinHandler::_stablecoinSwap function to be called without verification from StablecoinHandler::_verifyStablecoinSwap causing potential burning of stablecoins below the minimum limit or minting of stablecoins above the maximum limit

Root Cause

Here in AmirX::swap if ss.destination is not set (i.e. address(0)) the _verifyStablecoinSwap function (responsible for checking the values for correct burning or minting of stablecoins) will be skipped due to the if statement. Next, if directional = false the else block will get executed:
https://github.com/sherlock-audit/2024-11-telcoin/blob/b9c751b59e78a7123a636e31ecafc9147046f190/telcoin-audit/contracts/swap/AmirX.sol#L96

} else {
     // if stablecoin swap
    _stablecoinSwap(wallet, ss);
     // if only stablecoin swap
    if (defi.walletData.length != 0) _defiSwap(wallet, defi);
}

The _stablecoinSwap function will get called potentially leading to incorrect burning or minting of stablecoins if the function parameters are set incorrectly.

Internal pre-conditions

  1. The directional parameter in the AmirX::swap function should be set to false
  2. The ss.destination parameter must be address(0)
  3. ss.oAmount could be set such that when burning from the origin stablecoin supply -> totalSupply < getMinLimit()
  4. ss.tAmount could be set such that when minting to the origin stablecoin supply -> totalSupply > getMaxLimit()

External pre-conditions

No response

Attack Path

  1. SWAPPER_ROLE calls the AmirX::swap function with ss.destination = address(0), directional = false, unsafe ss.oAmount and ss.tAmount
  2. _verifyStablecoinSwap() which checks the min and max supply gets skipped due to if (ss.destination != address(0))
  3. directional = false => the else block will get executed
  4. _stablecoinSwap function in the else block will get called unsafely burning or minting stablecoins potentially disrupting the total supply invariants

Impact

The explained issue could unsafely modify the stablecoins' total supply such that it exceeds the min and max limits.

PoC

No response

Mitigation

Adding a check would be sufficient

function swap(
        ...
    ) external payable onlyRole(SWAPPER_ROLE) whenNotPaused {
        if (directional) {
        ...
        } else {
-           _stablecoinSwap(wallet, ss);
+          if (ss.destination != address(0)) _stablecoinSwap(wallet, ss);
            if (defi.walletData.length != 0) _defiSwap(wallet, defi);
        }
    }
@sherlock-admin3 sherlock-admin3 changed the title Magic Daisy Bison - Insufficient checks in AmirX::swap will allow change of protocol's state without verification frndz0ne - Insufficient checks in AmirX::swap will allow change of protocol's state without verification Nov 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant