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

3 stars 1 forks source link

Aamirusmani1552 - `AmirX::stablecoinSwap(...)` can cause de-peg of the StableCoin. #75

Closed sherlock-admin4 closed 8 months ago

sherlock-admin4 commented 8 months ago

Aamirusmani1552

high

AmirX::stablecoinSwap(...) can cause de-peg of the StableCoin.

Summary

Amir::stablecoinSwap(...) allows conversion of token for any amount of origin token amount which can lead to de-peg of the stablecoin.

Vulnerability Detail

When call to AmirX::defiSwap(...) is made in the AmirX::stablecoinSwap(...), it is not know how many tokens will be returned from the swap since it happens on the wallet. That mean it could be any value.

    function stablecoinSwap(
        address wallet,
        address safe,
        StablecoinSwap memory ss,
        DefiSwap memory defi
    ) external payable onlyRole(SWAPPER_ROLE) {
        // checks if it will fail
        _verifyStablecoin(wallet, safe, ss, defi);

        //eXYZ ot eXYZ
        if (isXYZ(ss.origin) && isXYZ(ss.target)) {
            swapAndSend(wallet, ss);
            return;
        }

        //stablecoin swap
        if (isXYZ(ss.origin) && !isXYZ(ss.target))
            convertFromEXYZ(wallet, safe, ss);

        //defi swap
        uint256 iBalance = ERC20(ss.origin).balanceOf(wallet);
@>        if (defi.walletData.length != 0) defiSwap(wallet, safe, defi);
        uint256 fBalance = ERC20(ss.origin).balanceOf(wallet);
        // //stablecoin swap
        if (!isXYZ(ss.origin) && isXYZ(ss.target)) {
 @>           if (fBalance - iBalance != 0) ss.oAmount = fBalance - iBalance;
            convertToEXYZ(wallet, safe, ss);
        }
    }

But the token to mint to the user will remain same when call to convertToEXYZ(...) is called. That mean for any value of one token, user will get the same amount of target token. If the returned value is less that what was expected to be than more tokens will be minted to the user for less corresponding amount.

Note that only condition that will affect the ss.oAmount is when fbalance - ibalance is greater than zero. That mean even if there is very little change in the amount of originToken in the wallet, ss.oAmount will be updated.

Impact

De-peg of the Stablecoin.

Code Snippet

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

Tool used

Manual Review

Recommendation

It is recommended to add the another state value to the DefiSwap struct for the expected amount of oAmount.

    struct DefiSwap {
        // Address of the swap aggregator or router
        address aggregator;
        // Plugin for handling referral fees
        ISimplePlugin plugin;
        // Token collected as fees
        ERC20 feeToken;
        // Address to receive referral fees
        address referrer;
        // Amount of referral fee
        uint256 referralFee;
        // Data for wallet interaction, if any
        bytes walletData;
        // Data for performing the swap, if any
        bytes swapData;

+     uint256 minOAmount;
    }

Then compare this with the actual amount changed after the swap:

    function stablecoinSwap(
        address wallet,
        address safe,
        StablecoinSwap memory ss,
        DefiSwap memory defi
    ) external payable onlyRole(SWAPPER_ROLE) {
        // checks if it will fail
        _verifyStablecoin(wallet, safe, ss, defi);

        //eXYZ ot eXYZ
        if (isXYZ(ss.origin) && isXYZ(ss.target)) {
            swapAndSend(wallet, ss);
            return;
        }

        //stablecoin swap
        if (isXYZ(ss.origin) && !isXYZ(ss.target))
            convertFromEXYZ(wallet, safe, ss);

        //defi swap
        uint256 iBalance = ERC20(ss.origin).balanceOf(wallet);
@>        if (defi.walletData.length != 0) defiSwap(wallet, safe, defi);
        uint256 fBalance = ERC20(ss.origin).balanceOf(wallet);
        // //stablecoin swap
        if (!isXYZ(ss.origin) && isXYZ(ss.target)) {
 @>           if (fBalance - iBalance != 0) ss.oAmount = fBalance - iBalance;
+          if(ss.oAmount < ss.minOAmount) revert();
            convertToEXYZ(wallet, safe, ss);
        }
    }
sherlock-admin3 commented 8 months ago

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

WangAudit commented:

seems to be working like it has to be; can't see where the depeg will take place here

takarez commented:

invalid