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

3 stars 1 forks source link

petro1912 - `convertToEXYZ` method in `StablecoinHandler` doesn't check if `ss.origin` is external XYZ token, so can mint destination XYZ tokens without burning origin XYZ tokens. #15

Closed sherlock-admin4 closed 8 months ago

sherlock-admin4 commented 8 months ago

petro1912

medium

convertToEXYZ method in StablecoinHandler doesn't check if ss.origin is external XYZ token, so can mint destination XYZ tokens without burning origin XYZ tokens.

Summary

While swapping between eXYZ tokens, swapper can swap without burning origin token.

Vulnerability Detail

stablecoinSwap function of AmirX contract swaps tokens by burning origin eXYZ token and minting target eXYZ token if origin and destination are all eXYZ in StablecoinHandlerStorage.

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

However there is public convertToEXYZ method that doesn't check if origin is external XYZ token.

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

So swapper can call convertToEXYZ method directly instead of stablecoinSwap, then origin XYZ token can't be burned.

Impact

It affects swapping between eXYZ tokens breaking the swap logic.

Code Snippet

https://github.com/sherlock-audit/2024-02-telcoin-platform-audit-update/blob/main/telcoin-contracts/contracts/swap/AmirX.sol#L76-L79 https://github.com/sherlock-audit/2024-02-telcoin-platform-audit-update/blob/main/telcoin-contracts/contracts/stablecoin/StablecoinHandler.sol#L141-L157

Tool used

Manual Review

Recommendation

++   error OriginIsEXYZ();
...

    function convertToEXYZ(
        address wallet,
        address safe,
        StablecoinSwap memory ss
    ) public virtual whenNotPaused nonZero(ss) onlyRole(SWAPPER_ROLE) {
++      if (isXYZ(ss.origin))
++         revert OriginIsEXYZ();

        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);
    }
sherlock-admin3 commented 8 months ago

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

WangAudit commented:

swapper role is trusted and controlled by telcoin team

takarez commented:

invalid