MaslarovK - Lack of Slippage Protection and Deadline Parameter in AmirX.sol Contract Swaps

[sherlock-admin4]

MaslarovK

Medium

Lack of Slippage Protection and Deadline Parameter in AmirX.sol Contract Swaps

Summary

In the AmirX.sol contract, the functions handling swaps, such as stablecoinSwap, defiToStablecoinSwap, and stablecoinToDefiSwap, lack a deadline parameter to mitigate slippage risk. Slippage refers to the potential variation in the token price between the time a transaction is submitted and when it is executed. Without slippage protection via a deadline, swaps are vulnerable to:

1. Price volatility during the delay between transaction submission and execution.
2. Block-stuffing attacks, where a malicious entity could delay the transaction until prices are unfavorable, potentially exploiting maximum extractable value (MEV).
3. Dynamic Variable Manipulation: If a transaction remains in the mempool for a prolonged period, variables like maxSupply and minSupply for XYZ tokens can be changed, affecting the swap's validity and leading to unexpected behavior.

The following code section in [_stablecoinSwap](https://github.com/sherlock-audit/2024-11-telcoin/blob/main/telcoin-audit/contracts/stablecoin/StablecoinHandler.sol#L144-L168) lacks deadline verification for slippage protection:

// 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
    );
}

// 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
    );
}

This lack of a deadline parameter creates an unnecessary risk in the protocol, particularly since the contract is deployed on Polygon, where MEV attacks are more common.

Root Cause

The primary cause of this issue is the omission of a deadline parameter in StablecoinSwap and DefiSwap structures, leading to the absence of time-based protections in swap operations. This omission affects the logic in _stablecoinSwap() and _defiSwap() and increases vulnerability to:

1. Market volatility, causing the swap to complete at an undesirable price.
2. Manipulation of dynamically changing constraints, such as maxSupply and minSupply, which could lead to reverted transactions if values shift before the transaction is mined.

Internal pre-conditions

• Swap transactions in AmirX.sol rely on values in StablecoinSwap and DefiSwap structures but do not include a deadline parameter.
• Swaps operate under the assumption that market conditions remain stable between submission and execution, which is not always valid.
• Constraints like maxSupply and minSupply could be modified while the transaction is pending in the mempool.

External pre-conditions

• An adversary could delay a swap transaction's execution by performing a block-stuffing attack, thereby controlling when the transaction is mined to exploit slippage.
• The absence of slippage protection in volatile market conditions could result in unfavorable swaps for users.
• An entity with the MAINTAINER_ROLE could modify maxSupply and minSupply values during the transaction’s mempool delay, potentially invalidating the transaction.

Attack Path

1. Transaction Submission:

   • A user submits a swap transaction, expecting it to complete with the parameters specified (e.g., origin and target amounts).

2. Block Stuffing (MEV Exploitation):

   • A malicious entity identifies the swap transaction and executes a block-stuffing attack, delaying its execution while monitoring the market.
   • The entity waits until the swap value becomes unfavorable to the user, allowing it to maximize MEV extraction.

3. Transaction Execution:

   • The swap is executed with an undesirable price due to the delay, leading to significant losses for the user.

4. Variable Manipulation:

   • If the transaction remains in the mempool for a prolonged time, the maxSupply and minSupply of XYZ tokens state variables could change before the transaction is mined. These variables directly affect the checks for _stablecoinSwap() function, potentially resulting transaction to revert.

Impact

The lack of a deadline parameter in StablecoinSwap and DefiSwap has several negative impacts:

• Financial Losses: Users could suffer significant losses if swaps execute at unfavorable prices due to slippage.

• Manipulated Reverts: Changes to maxSupply and minSupply variables could cause transactions to revert, making the protocol unreliable for users and damaging its reputation.

• Vulnerability to MEV Attacks: The contract is susceptible to block-stuffing and timing attacks, leading to losses for users and MEV gains for malicious actors, especially given the deployment on Polygon.

• Stablecoin -> stablecoin: Stablecoins will be minted or burned regularly to mirror the price of the off-chain asset they are pegged to. The value of X stablecoins may vary significantly from its level when the swap transaction was initially submitted.

• Stablecoin -> ERC20 and vice versa: The explanation above also applies here, but the risk is heightened due to potential swaps involving a highly volatile ERC20 token. According to the documentation, any ERC20 tokens can interact with the protocol.

Example Attack Scenario

Suppose a user initiates a stablecoin-to-ERC20 swap with the following details:

StablecoinSwap memory ss = StablecoinSwap({
    liquiditySafe: 0xSafeAddress,
    destination: 0xUserAddress,
    origin: 0xStablecoinAddress,
    oAmount: 1000,
    target: 0xERC20Address,
    tAmount: 500,
    stablecoinFeeCurrency: 0xFeeToken,
    stablecoinFeeSafe: 0xFeeSafe,
    feeAmount: 10
});

1. The swap transaction enters the mempool without a deadline parameter.
2. A block-stuffing attack delays the swap execution, and by the time it executes, the stablecoin has lost value due to market changes.
3. The user receives significantly less value in the ERC20 token, resulting in a financial loss.

PoC

No response

Mitigation

To address this vulnerability, add a deadline parameter to StablecoinSwap and DefiSwap structures. The deadline should specify a timestamp beyond which the transaction is invalid. This parameter will allow users to protect against slippage and MEV attacks by ensuring their transaction completes only within a specified time.

Proposed Solution

1. Add Deadline Parameter:
   Modify the StablecoinSwap and DefiSwap structures to include a deadline field, as shown below:

   struct StablecoinSwap {
       address liquiditySafe;
       address destination;
       address origin;
       uint256 oAmount;
       address target;
       uint256 tAmount;
       address stablecoinFeeCurrency;
       address stablecoinFeeSafe;
       uint256 feeAmount;
       uint256 deadline; // New parameter
   }
   
   struct DefiSwap {
       address defiSafe;
       address aggregator;
       ISimplePlugin plugin;
       ERC20 feeToken;
       address referrer;
       uint256 referralFee;
       bytes walletData;
       bytes swapData;
       uint256 deadline; // New parameter
   }

2. Implement Deadline Check in _stablecoinSwap() and _defiSwap():

   Add logic in _stablecoinSwap() and _defiSwap() to verify that the current block timestamp is less than or equal to the deadline. If the deadline is passed, revert the transaction:

   function _stablecoinSwap(
       address wallet,
       StablecoinSwap memory ss
   ) internal {
       // Check for deadline expiration
       require(block.timestamp <= ss.deadline, "AmirX: swap deadline expired");
   
       if (
           ss.stablecoinFeeCurrency != address(0) &&
           ss.stablecoinFeeSafe != address(0)
       )
           ERC20PermitUpgradeable(ss.stablecoinFeeCurrency).safeTransferFrom(
               wallet,
               ss.stablecoinFeeSafe,
               ss.feeAmount
           );
   
       // Handle the transfer or burning of the origin currency:
       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 (isXYZ(ss.target)) {
           Stablecoin(ss.target).mintTo(ss.destination, ss.tAmount);
       } else {
           ERC20PermitUpgradeable(ss.target).safeTransferFrom(
               ss.liquiditySafe,
               ss.destination,
               ss.tAmount
           );
       }
   }

3. Update Other Swap Functions:
   Update all other swap functions in AmirX.sol (e.g., defiToStablecoinSwap, stablecoinToDefiSwap, defiSwap) to include the deadline parameter and perform deadline checks as shown in _stablecoinSwap().

Summary

By adding a deadline parameter and checking it within swap functions, the protocol can significantly reduce risks associated with slippage and MEV attacks. This update provides a safeguard against volatile market changes and ensures users’ transactions complete within their desired time frame, enhancing the reliability and security of the Telcoin Protocol.