hrmneffdii - Unintended protocol behavior when doing swap more than two users

[sherlock-admin2]

hrmneffdii

High

Unintended protocol behavior when doing swap more than two users

Summary

Unintended protocol behavior when doing swap lead to an unexpected revert. This revert comes from unordered transactions where this accident could happen at anytime, causes inconvenience for users when doing swap.

Root Cause

Let's see code below

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

Because of the AmirX creates a transfer all of funds with telcoin.balanceOf(address(this)), it automatically affect to move all of funds into address defi.defiSafe. For single transaction let's say just Alice transaction, it acceptable because just Alice token that would be move to address defi.defiSafe. But in another case, it doesn't acceptable.

Assume first, we go through normal case. We have two users that is Alice and Bob. They would swap their assets from eUSD to eMXN with fee token telcoin directly . The scenario looks like:

• Alice sends telcoin token to the AmirX as a referral fees and Alice creates a swap
• Also Bob sends telcoin token to the AmirX as a referral fees and Bob creates a swap
• The transactions will run normal

Assume second, we go through unnormal case. We have two users that is Alice and Bob. And also they would swap their assets from eUSD to eMXN with fee token with telcoin directly. This unnormal case can occur when:

• Alice sends telcoin token to the AmirX as a referral fees. And then Bob also sends telcoin token to the AmirX as a referral fees
• The AmirX protocol contains all of referral fees from Alice and Bob
• Alice creates a swap and then theAmirX moves all of funds (both referral fees of Alice and Bob) into defi.defiSafe
• Afterthat, Bob creates a swap and the Bob's transaction will revert because the referral fee of Bob already move first when Alice creates a swap.

Impact

When we see the unnormal case above, the impact will affect to Bob that the protocol will revert his transaction and losing his referral fee. To avoid it, Bob must retransfer again to the AmirX and make Bob pays the referral fees twice (with notes that his second transaction in normal case).

PoC

it("Unnormal case when doing swap", async () => {
            const stablecoinInputsAlice = {
                liquiditySafe: deployer,
                destination: holder,
                origin: eUSD,
                oAmount: 100,
                target: eMXN,
                tAmount: 1000,
                stablecoinFeeCurrency: eUSD,
                stablecoinFeeSafe: safe.address,
                feeAmount: 5
            }

            const defiInputsAlice = {
                defiSafe: deployer,
                aggregator: aggregator,
                plugin: plugin,
                feeToken:  await telcoin.getAddress(),
                referrer: deployer,
                referralFee: 5,
                walletData: await wallet.getTestSelector(),
                swapData: await aggregator.getSwapSelector(),
            }

            const stablecoinInputsBob = {
                liquiditySafe: deployer,
                destination: holder,
                origin: eUSD,
                oAmount: 200,
                target: eMXN,
                tAmount: 2000,
                stablecoinFeeCurrency: eUSD,
                stablecoinFeeSafe: safe.address,
                feeAmount: 10
            }

            const defiInputsBob = {
                defiSafe: deployer,
                aggregator: aggregator,
                plugin: plugin,
                feeToken:  await telcoin.getAddress(),
                referrer: deployer,
                referralFee: 10,
                walletData: await wallet.getTestSelector(),
                swapData: await aggregator.getSwapSelector(),
            }

            await telcoin.mintTo(Alice,5);
            await telcoin.mintTo(Bob, 10);                

            await eUSD.mintTo(Alice, 105);                
            await eUSD.connect(Alice).approve(AmirX, 105);

            await eUSD.mintTo(Bob, 210);                
            await eUSD.connect(Bob).approve(AmirX, 210);

            await telcoin.connect(Alice).transfer(AmirX, 5);
            await telcoin.connect(Bob).transfer(AmirX, 10);

            await expect(AmirX.stablecoinToDefiSwap(Alice, stablecoinInputsAlice, defiInputsAlice)).to.not.be.reverted;
            // Bob transaction will revert
@>          await expect(AmirX.stablecoinToDefiSwap(Bob, stablecoinInputsBob, defiInputsBob)).to.be.reverted; 
            });

  AmirX
    Swaps
      two steps
        ✔ Unnormal case when doing swap (51ms)
  1 passing (1s)

Mitigation

Instead of using telcoin.balanceOf(address(this)) as a value, recommend to use defi.referralFee as a value exatcly to avoid moving all of funds into address defi.defiSafe.

        if (TELCOIN.balanceOf(address(this)) > 0) 
               TELCOIN.safeTransfer( 
                     defi.defiSafe, 
-                    TELCOIN.balanceOf(address(this)), 
+                    defi.referralFee
              );