Skip to content

Latest commit

 

History

History
138 lines (99 loc) · 5.3 KB

064.md

File metadata and controls

138 lines (99 loc) · 5.3 KB

Salty Mauve Puma

Medium

Oversight in defiToStablecoinSwap will cost the Wallet Stablefee eventhough ss.oAmount is Zero

Summary

A vulnerability exists in the defiToStablecoinSwap function, allowing fees to be incurred even when no swap occurs due to ss.oAmount being zero. This issue arises when ss.oAmount is not adequately validated, causing a potential loss of funds for the wallet. In the current implementation, if ss.oAmount is zero, a fee is still transferred without any actual stablecoin transfer, mint, or burn operations taking place.

Root Cause

In the defiToStablecoinSwap function, the flow follows several steps:

  1. Defi Verification: _verifyDefiSwap checks for any issues with the defi swap parameters.
  2. Stablecoin Verification: _verifyStablecoinSwap validates the stability of the swap before execution.
  3. Defi Swap Execution: _defiSwap processes the defi swap.
  4. Stablecoin Swap Execution: _stablecoinSwap initiates the stablecoin transaction.

The vulnerability is rooted in the assignment of ss.oAmount. In the swap flow:

    uint256 iBalance = ERC20(ss.origin).balanceOf(wallet);
    _defiSwap(wallet, defi);
    uint256 fBalance = ERC20(ss.origin).balanceOf(wallet);
      //change balance to reflect change

@audit>>> 0 not avoided >>>    ss.oAmount = fBalance - iBalance;

Here, ss.oAmount is set to fBalance - iBalance, but there’s no check to ensure that ss.oAmount is not zero. If this calculation results in zero, _stablecoinSwap will proceed with a transaction where no actual stablecoin value is exchanged, leading to fees being charged despite the lack of an effective swap. This leaves the wallet exposed to unnecessary fee transfers without any beneficial outcome. Looking at the same Implementation in the SWAP function

      // 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

@audit>>> not 0 >>>      if (fBalance - iBalance != 0) ss.oAmount = fBalance - iBalance;
             
             _stablecoinSwap(wallet, ss);

https://github.com/sherlock-audit/2024-11-telcoin/blob/main/telcoin-audit/contracts/swap/AmirX.sol#L92-L94

https://github.com/sherlock-audit/2024-11-telcoin/blob/main/telcoin-audit/contracts/swap/AmirX.sol#L111-L127

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

Since ss.oamount is zero we can see below that the wallet still pays the s.feeAmount. instead of the entire process reverting.

 function _stablecoinSwap(
        address wallet,
        StablecoinSwap memory ss
    ) internal {
  
@audit>>>      if (
            ss.stablecoinFeeCurrency != address(0) &&
            ss.stablecoinFeeSafe != address(0)
        )
@audit>>>            ERC20PermitUpgradeable(ss.stablecoinFeeCurrency).safeTransferFrom(
                wallet,
                ss.stablecoinFeeSafe,
@audit>>>                ss.feeAmount
            );

        // Handle the transfer or burning of the origin currency:
        // If the origin is a recognized stablecoin (XYZ), burn the specified amount from the wallet.
        if (isXYZ(ss.origin)) {
            Stablecoin(ss.origin).burnFrom(wallet, ss.oAmount);
        } else {
            ERC20PermitUpgradeable(ss.origin).safeTransferFrom(
                wallet,
                ss.liquiditySafe,
                ss.oAmount
            );
        }

        // Handle the minting or transferring of the target currency:
        // If the target is a recognized stablecoin (XYZ), mint the required amount to the destination address.
        if (isXYZ(ss.target)) {
            Stablecoin(ss.target).mintTo(ss.destination, ss.tAmount);
        } else {
            ERC20PermitUpgradeable(ss.target).safeTransferFrom(
                ss.liquiditySafe,
                ss.destination,
                ss.tAmount
            );
        }
    }

Impact

The primary impact of this vulnerability is a potential loss of funds:

  • Fee Transfer with Zero Swap: When ss.oAmount is zero, fees are transferred without minting, burning, or transferring new stablecoins.

PoC

No response

Mitigation

To mitigate this vulnerability, a check can be implemented to ensure ss.oAmount is not zero before executing _stablecoinSwap. Specifically:

  1. Relocate Stablecoin Swap Verification: Move the _verifyStablecoinSwap function call to after the assignment of ss.oAmount, ensuring ss.oAmount has been calculated correctly.

    Code Update Example:

     uint256 iBalance = ERC20(ss.origin).balanceOf(wallet);
     _defiSwap(wallet, defi);
     uint256 fBalance = ERC20(ss.origin).balanceOf(wallet);
     ss.oAmount = fBalance - iBalance;
     
     // Verify stablecoin swap after updating ss.oAmount
     _verifyStablecoinSwap(wallet, ss);
    
     // Execute stablecoin swap only if ss.oAmount is non-zero
     if (ss.oAmount != 0) {
         _stablecoinSwap(wallet, ss);
     }
  2. Add Validation Logic: Explicitly check ss.oAmount to prevent unintended execution when zero has it is done in SWAP function.