sherlock-audit / 2024-02-telcoin-platform-audit-update-judging

3 stars 1 forks source link

Anubis - Insufficient Token Allowance Check Leading to Potential Transfer Failures in stablecoinSwap Function #21

Closed sherlock-admin4 closed 8 months ago

sherlock-admin4 commented 8 months ago

Anubis

high

Insufficient Token Allowance Check Leading to Potential Transfer Failures in stablecoinSwap Function

Summary

The stablecoinSwap function in the provided code snippet potentially allows for token transfers without ensuring that the necessary token allowances have been set by the token owners. This lack of explicit allowance checks before executing token transfer operations such as swapAndSend, convertFromEXYZ, defiSwap, and convertToEXYZ could lead to failed transactions and the potential freezing of user funds or the contract's inability to fulfill its intended token swap operations.

Vulnerability Detail

The root cause of the vulnerability "Insufficient Token Allowance Check Leading to Potential Transfer Failures in stablecoinSwap Function" in the provided code is that there is no check to ensure that the user has approved enough token allowance for the smart contract to transfer tokens on their behalf.

In lines 86 to 88, the code checks the balance of the user's wallet before and after a potential token swap operation. However, it does not check if the smart contract has been approved to transfer tokens from the user's wallet. This can lead to potential transfer failures if the user has not approved a sufficient token allowance for the smart contract to perform the transfer.

The vulnerability in the code lies in the insufficient token allowance check before transferring tokens in the stablecoinSwap function. This can potentially lead to transfer failures if the user does not have enough token allowance set for the contract to transfer tokens on their behalf.

To exploit this vulnerability, an attacker can perform the following steps:

  1. Deploy a malicious contract that interacts with the vulnerable contract's stablecoinSwap function.
  2. Call the stablecoinSwap function with the attacker's wallet address, a safe address controlled by the attacker, and crafted StablecoinSwap and DefiSwap parameters.
  3. Craft the parameters in such a way that the token transfer will fail due to insufficient allowance set for the contract.
  4. The attacker can then manipulate the contract state or cause unexpected behavior by exploiting the transfer failure.

Proof of Concept (PoC) code:

// Malicious contract to exploit the vulnerability
pragma solidity ^0.8.0;

interface VulnerableContract {
    function stablecoinSwap(address wallet, address safe, StablecoinSwap memory ss, DefiSwap memory defi) external payable;
}

contract Exploit {
    address public vulnerableContractAddress = 0x123; // Address of the vulnerable contract
    address public attackerWallet = msg.sender;
    address public attackerSafe = address(this); // Attacker controlled safe address

    struct StablecoinSwap {
        address origin;
        address target;
        uint256 oAmount;
        uint256 tAmount;
    }

    struct DefiSwap {
        bytes walletData;
    }

    function exploit() public {
        VulnerableContract vulnerableContract = VulnerableContract(vulnerableContractAddress);

        StablecoinSwap memory ss = StablecoinSwap({
            origin: address(0x456), // Token address with insufficient allowance
            target: address(0x789),
            oAmount: 100,
            tAmount: 0
        });

        DefiSwap memory defi = DefiSwap({
            walletData: ""
        });

        vulnerableContract.stablecoinSwap(attackerWallet, attackerSafe, ss, defi);
    }
}

In this PoC code, the Exploit contract interacts with the vulnerable contract's stablecoinSwap function by passing parameters that will lead to a transfer failure due to insufficient token allowance. The attacker can then exploit this vulnerability to manipulate the contract state or cause unexpected behavior.

Impact

The impact of this vulnerability could be significant, as it might result in the loss of funds or trust in the contract's reliability, particularly if the contract interacts with DeFi protocols or performs automated swaps that require pre-authorized token allowances.

Code Snippet

https://github.com/sherlock-audit/2024-02-telcoin-platform-audit-update/blob/main/telcoin-contracts/contracts/swap/AmirX.sol#L66-L94

Tool used

Manual Review

Recommendation

The vulnerability in the code is that there is no check for the token allowance before transferring tokens in the stablecoinSwap function. This can lead to potential transfer failures if the user has not approved the contract to spend their tokens.

To fix this issue, we need to add a check to ensure that the contract has been approved to spend the required amount of tokens before attempting the transfer. This can be done by using the allowance function of the ERC20 token contract to check the allowance granted by the user to the contract.

Here is an example of how the code can be patched to include the allowance check:

66       function stablecoinSwap(
67           address wallet,
68           address safe,
69           StablecoinSwap memory ss,
70           DefiSwap memory defi
71       ) external payable onlyRole(SWAPPER_ROLE) {
72           // checks if it will fail
73           _verifyStablecoin(wallet, safe, ss, defi);
74   
75           // Check token allowance
76           require(ERC20(ss.origin).allowance(wallet, address(this)) >= ss.oAmount, "Insufficient allowance");
77   
78           //eXYZ ot eXYZ
79           if (isXYZ(ss.origin) && isXYZ(ss.target)) {
80               swapAndSend(wallet, ss);
81               return;
82           }
83   
84           //stablecoin swap
85           if (isXYZ(ss.origin) && !isXYZ(ss.target))
86               convertFromEXYZ(wallet, safe, ss);
87   
88           //defi swap
89           uint256 iBalance = ERC20(ss.origin).balanceOf(wallet);
90           if (defi.walletData.length != 0) defiSwap(wallet, safe, defi);
91           uint256 fBalance = ERC20(ss.origin).balanceOf(wallet);
92           // //stablecoin swap
93           if (!isXYZ(ss.origin) && isXYZ(ss.target)) {
94               if (fBalance - iBalance != 0) ss.oAmount = fBalance - iBalance;
95               convertToEXYZ(wallet, safe, ss);
96           }
97       }

By adding the require statement on line 76, we ensure that the contract has been approved to spend the required amount of tokens before proceeding with the transfer. This helps prevent potential transfer failures due to insufficient token allowance.

sherlock-admin3 commented 8 months ago

1 comment(s) were left on this issue during the judging contest.

WangAudit commented:

user mistake validation -> invalid under Sherlock's rules