Skip to content

Latest commit

 

History

History
138 lines (113 loc) · 5.52 KB

024.md

File metadata and controls

138 lines (113 loc) · 5.52 KB

Rich Chrome Rattlesnake

High

The min burn limit of XYZ tokens can be bypassed.

Summary

The min burn limit of XYZ token is set by the admin. However, AmirX contract contains a logical error that allows users to burn tokens below the min burn limit.

Root Cause

The StablecoinHandler._verifyStablecoinSwap() function is following.

    function _verifyStablecoinSwap(
        address wallet,
        StablecoinSwap memory ss
    ) internal view nonZero(ss) {
        // Ensure the wallet address is not zero.
        if (wallet == address(0)) revert ZeroValueInput("WALLET");

        // For the origin currency:
        if (isXYZ(ss.origin)) {
            // Ensure the total supply does not drop below the minimum limit after burning the specified amount.
            if (
205:            Stablecoin(ss.origin).totalSupply() - ss.oAmount <
                getMinLimit(ss.origin)
            ) revert InvalidMintBurnBoundry(ss.origin, ss.oAmount);
        } else if (ss.liquiditySafe == address(0)) {
            // Ensure the liquidity safe is provided for ERC20 origin tokens.
            revert ZeroValueInput("LIQUIDITY SAFE");
        }

        ... SKIP ...
    }

As can be seen, the _verifyStablecoinSwap() function prevents users from burning XYZ tokens below the min burn limit.

Meanwhile, the AmirX.swap() function is following.

    function swap(
        address wallet,
        bool directional,
        StablecoinSwap memory ss,
        DefiSwap memory defi
    ) external payable onlyRole(SWAPPER_ROLE) whenNotPaused {
        // checks if it will fail
80:     if (ss.destination != address(0)) _verifyStablecoinSwap(wallet, ss);
        if (defi.walletData.length != 0) _verifyDefiSwap(wallet, defi);

        if (directional) {
            // if only defi swap
            if (ss.destination == address(0)) _defiSwap(wallet, defi);
            else {
                // if defi then stablecoin swap
                //check balance to adust second swap
                uint256 iBalance = ERC20(ss.origin).balanceOf(wallet);
                if (defi.walletData.length != 0) _defiSwap(wallet, defi);
                uint256 fBalance = ERC20(ss.origin).balanceOf(wallet);
                //change balance to reflect change
93:             if (fBalance - iBalance != 0) ss.oAmount = fBalance - iBalance;
                _stablecoinSwap(wallet, ss);
            }
        } else {
            // if stablecoin swap
            _stablecoinSwap(wallet, ss);
            // if only stablecoin swap
            if (defi.walletData.length != 0) _defiSwap(wallet, defi);
        }
    }

As can be seen, the above function updates the burn amount based on the result of Defi swap in L93. Due to potential slippage, the swapper role can't precisely predict the burn amount beforehand, causing ss.oAmount to change. If ss.oAmount increases, it bypasses the condition of L205. The issue also exists in the AmirX.defiToStablecoinSwap() function.

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

The min burn limit of XYZ tokens can be bypassed. If the total supply of XYZ tokens becomes too low, it could negatively impact the token's price, which is a significant issue since XYZ tokens are stablecoins.

PoC

  1. Assume that the min burn limit of a XYZ token is 100 ether and the total supply is 101 ether.
  2. If ss.oAmount is 1 ether, it will pass the condition check in the _verifyStablecoinSwap() function at L205.
  3. As a result of Defi swap, ss.oAmount is updated to 2 ether in L93 of the swap() function.
  4. Afther the stable swap completes, the total supply of XYZ token will decrease to 99 ether, which is below the min burn limit.

Mitigation

Modify the AmirX.swap() function as follows.

    function swap(
        address wallet,
        bool directional,
        StablecoinSwap memory ss,
        DefiSwap memory defi
    ) external payable onlyRole(SWAPPER_ROLE) whenNotPaused {
        // checks if it will fail
--      if (ss.destination != address(0)) _verifyStablecoinSwap(wallet, ss);
        if (defi.walletData.length != 0) _verifyDefiSwap(wallet, defi);

        if (directional) {
            // if only defi swap
            if (ss.destination == address(0)) _defiSwap(wallet, defi);
            else {
                // if defi then stablecoin swap
                //check balance to adjust second swap
                uint256 iBalance = ERC20(ss.origin).balanceOf(wallet);
                if (defi.walletData.length != 0) _defiSwap(wallet, defi);
                uint256 fBalance = ERC20(ss.origin).balanceOf(wallet);
                //change balance to reflect change
--              if (fBalance - iBalance != 0) ss.oAmount = fBalance - iBalance;
++              if (fBalance - iBalance != 0) {
++                  ss.oAmount = fBalance - iBalance;
++                  _verifyStablecoinSwap(wallet, ss);
++              }
                _stablecoinSwap(wallet, ss);
            }
        } else {
++          if (ss.destination != address(0)) _verifyStablecoinSwap(wallet, ss);
            // if stablecoin swap
            _stablecoinSwap(wallet, ss);
            // if only stablecoin swap
            if (defi.walletData.length != 0) _defiSwap(wallet, defi);
        }
    }