turvec - Address initiating the swap can manipulate transfers to their favor due to overriding oAmount without adjusting tAmount

[sherlock-admin4]

turvec

high

Address initiating the swap can manipulate transfers to their favor due to overriding oAmount without adjusting tAmount

Summary

Address initiating the swap can manipulate transfers to their favor due to overriding oAmount without adjusting tAmount

Vulnerability Detail

When converting to stablecoin (EXYZ) using the stablecoinSwap() function, they agree to transfer an amount (oAmount) and get minted an amount of the target stablecoin (tAmount).

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

convertToEXYZ

function convertToEXYZ(
        address wallet,
        address safe,
        StablecoinSwap memory ss
    ) public virtual whenNotPaused nonZero(ss) onlyRole(SWAPPER_ROLE) {
        if (
            Stablecoin(ss.target).totalSupply() + ss.tAmount >
            getMaxLimit(ss.target)
        ) revert InvalidMintBurnBoundry(ss.target);

        ERC20PermitUpgradeable(ss.origin).safeTransferFrom(
            wallet,
            safe,
            ss.oAmount
        );
        Stablecoin(ss.target).mintTo(ss.destination, ss.tAmount);
    }

However, notice that the oAmount can be entirely overridden if there is a slight change in the contract balance after a defiSwap which makes an external call to the address initiating the swap:

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

defiSwap

 function defiSwap(
        address wallet,
        address safe,
        DefiSwap memory defi
    ) public payable onlyRole(SWAPPER_ROLE) {
  @>    (bool walletResult, ) = wallet.call{value: 0}(defi.walletData);
        require(walletResult, "AmirX: wallet transaction failed");

        _feeDispersal(safe, defi);
    }

The issue here is that while this oAmount gets totally overridden to whatever was the balance difference, the tAmount to be minted for the address initiating the swap remains the same regardless. This means the address initiating the swap can deliberately transfer just 1 wei worth of amount to the contract during the external call which will end up being the only amount they would have to transfer and get minted the same tAmount, causing them to manipulate the transfer and the stablecoin accounting and minting in general

Impact

Address initiating the swap can manipulate transfers to their favor and also affect the stablecoin accounting and minting in general

Code Snippet

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

Tool used

Manual Review

Recommendation

Consider if tAmount should be adjusted as well if oAmount gets overridden or if it's to sum the balance difference to the already agreed upon oAmount rather than totally overriding it.

[sherlock-admin2]

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

WangAudit commented:

iBalance and fBalance are checked right before and after the defiSwap; therefore; I can't see a way that those values can be manipulated within the function execution

takarez commented:

invalid