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

3 stars 1 forks source link

turvec - The `convertToEXYZ` and `convertFromEXYZ` functions doesn't check if the stablecoin is indeed still a valid eXYZ token #74

Closed sherlock-admin2 closed 8 months ago

sherlock-admin2 commented 8 months ago

turvec

medium

The convertToEXYZ and convertFromEXYZ functions doesn't check if the stablecoin is indeed still a valid eXYZ token

Summary

The convertToEXYZ and convertFromEXYZ functions doesn't check if the stablecoin is indeed still a valid eXYZ token

Vulnerability Detail

The protocol has external XYZ tokens which are added and maintained by the MAINTAINER_ROLE using the UpdateXYZ. external XYZ tokens struct has a validity status which is a boolean indicating whether the token should be considered valid or not. Which the MAINTAINER_ROLE handles as is crucial for maintaining the operational parameters of external XYZ tokens within the system. However, this is not checked in the convertToEXYZ and convertFromEXYZ meaning even if the MAINTAINER_ROLE updates the validity of an XYZ token, it would still remain valid and pass: The convertToEXYZ function

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

The convertFromEXYZ function

function convertFromEXYZ(
        address wallet,
        address safe,
        StablecoinSwap memory ss
    ) public virtual whenNotPaused nonZero(ss) onlyRole(SWAPPER_ROLE) {
        if (
            Stablecoin(ss.origin).totalSupply() - ss.oAmount <
            getMinLimit(ss.origin)
        ) revert InvalidMintBurnBoundry(ss.origin);

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

As you can see, none of these check if the stablecoin is indeed still a valid eXYZ token

Impact

If the MAINTAINER_ROLE decides to invalidate an XYZ token, it would still remain valid and pass.

Code Snippet

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

Call the isXYZ() on the stablecoins for both functions and check that they return true.

sherlock-admin2 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