search

sherlock-audit / 2024-11-telcoin-judging

0 stars 0 forks source link

Hacek00 - Strict minting & burning functionality may stop execution of StablecoinSwap #141

Open sherlock-admin2 opened 1 week ago

sherlock-admin2 commented 1 week ago

Hacek00

High

Strict minting & burning functionality may stop execution of StablecoinSwap

Summary

See the root cause section.

Root Cause

Strict access control in Stablecoin::mintTo() & Stablecoin::burnFrom() may lead to issue. We can see that the mintTo() is only callable by MINTER_ROLE & burnFrom() only callable by BURNER_ROLE. In StablecoinHandler(), during StablecoinSwap, minTo() & burnFrom() is called from _stablecoinSwap(). From this we can understand that MINTER_ROLE & BURNER_ROLE must be assigned to the AmirX contract [ since StablecoinHandler is an abstract function & AmirX was inherited from it ]. If AmirX has those 2 role then no other address can call mintTo() & burnFrom(). This can create problem because in stablecoinSwap origin token is burned & target token is minted. A token can only be burned if it was minted before. As the mintTo() only callable by AmirX & AmirX calls the minTo() only during swap, after burning another token, it falls into a broken phase like the to be burned token has not minted yet. In a nutshell we can say that the minting process is only initiate after burning an another token & limited only that place. So if the very first swap is StablecoinSwap [ by calling stablecoinSwap() ] then it may revert.

Example: Alice want to swap 100 eMXN to 100 eUSD where both are stablecoin in terms of Telcoin protocol i.e both are deployed using Stablecoin contract. Now to have 100 eMXN in Alice's account that amount of eMXN must be minted to Alice. The issue is here, it is not possible to mint eMXN externally to Alice, only 1 way to mint is calling stablecoinSwap(), but this won't work because this function will attempt to burn 100 eMXN from Alice's account but Alice do not have eMXN yet. We can say the minting process is locked in stablecoinSwap().

The tests in StablecoinHandler.test.ts are passing because the tokens are minted externally to the wallet:

it("stablecoinSwap", async () => {
                await eUSD.mintTo(holder, 15);      // @audit origin token were minted to wallet externally
                const inputs = {
                    liquiditySafe: deployer,
                    destination: holder,
                    origin: eUSD,
                    oAmount: 10,
                    target: eMXN,
                    tAmount: 100,
                    stablecoinFeeCurrency: eUSD,
                    stablecoinFeeSafe: safe.address,
                    feeAmount: 5
                }

                await eUSD.connect(holder).approve(stablecoinHandler, 15);
                await expect(stablecoinHandler.stablecoinSwap(holder, inputs)).to.not.be.reverted;
                expect(await eUSD.totalSupply()).to.equal(5);
                expect(await eUSD.balanceOf(safe)).to.equal(5);
                expect(await eMXN.balanceOf(holder)).to.equal(100);
            });

But in production it is not possible to mint externally due to role restriction.

Internal pre-conditions

None.

External pre-conditions

None.

Attack Path

None.

Impact

stablecoinSwap will not work.

PoC

Mitigation

Do not limit the minting only in stablecoinSwap().