[WP-H3] S2S Transfer from the origin schain to another schain with automatic deploy disabled can cause funds to be frozen

[code423n4]

Lines of code

https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/schain/TokenManagers/TokenManagerERC20.sol#L289-L301

Vulnerability details

When moving tokens that are native on the origin schain, to another schain, TokenManagerERC20.sol#transferToSchainERC20() will be called, which calls _exit() -> _receiveERC20():

https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/schain/TokenManagers/TokenManagerERC20.sol#L289-L301

if (isMainChainToken) {
    data = _receiveERC20(
        chainHash,
        address(contractOnSchain),
        msg.sender,
        amount
    );
    _saveTransferredAmount(chainHash, address(contractOnSchain), amount);
    require(
        contractOnSchain.transferFrom(msg.sender, address(this), amount),
        "Transfer was failed"
    );
}

https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/schain/TokenManagers/TokenManagerERC20.sol#L351-L361

bool isERC20AddedToSchain = _schainToERC20[chainHash].contains(erc20OnMainChain);
if (!isERC20AddedToSchain) {
    _addERC20ForSchain(chainHash, erc20OnMainChain);
    data = Messages.encodeTransferErc20AndTokenInfoMessage(
        erc20OnMainChain,
        to,
        amount,
        _getErc20TotalSupply(erc20),
        _getErc20TokenInfo(erc20)
    );
}

However, on the target schain, while handling the inbound message with postMessage() -> _sendERC20(), when contractOnSchain is false, The transaction will fail with "Automatic deploy is disabled" when automaticDeploy == false:

https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/schain/TokenManagers/TokenManagerERC20.sol#L227-L235

 contractOnSchain = clonesErc20[fromChainHash][token];

if (address(contractOnSchain) == address(0)) {
    require(automaticDeploy, "Automatic deploy is disabled");
    contractOnSchain = new ERC20OnChain(message.tokenInfo.name, message.tokenInfo.symbol);
    clonesErc20[fromChainHash][token] = contractOnSchain;
    addedClones[contractOnSchain] = true;
    emit ERC20TokenCreated(fromChainHash, token, address(contractOnSchain));
}

As a result, any tokens that are locked in the origin schain by the user will be frozen in the contract.

Recommendation

Consider adding a mapping storage to cache whether automaticDeploy is enabled on a certain schain, the cache should be updated once the automaticDeploy is updated.

And only allows S2S transfer when automaticDeploy is enabled on the target schain.

To further avoid the edge case of: right after the user submitted the S2S transfer tx on the from schain, the target schain disabled automaticDeploy and the user's tokens can be frozen in the from schain. We can introduce a 24 hrs timelock for disabling automaticDeploy.

[cstrangedk]

Issue raised is acknowledged and work is assigned on the roadmap. SKALE Chain owners must ensure any mapped assets, either through manual or automatic mapping are compatible with their dApp(s). Manual mapping mode is the default mode for bridge operation.

[GalloDaSballo]

I agree with both sides of the argument, and because this is contingent on configuration and admin privilege, believe Medium Severity to be more appropriate