search

code-423n4 / 2022-02-skale-findings

0 stars 0 forks source link

There is a possibility of Token transfer getting stuck when using Erc1155BatchMessage #62

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/Messages.sol#L515-L529

Vulnerability details

Impact

In the event of user error while making the calldata for encodeTransferErc1155BatchMessage, where the size of the arrays of ids and amounts donot match, the message will get encoded due to no input validation, however the transfer will fail at the other end of the bridge, resulting in the tokens getting stuck in the protocol.

Applies also to encodeTransferErc1155BatchAndTokenInfoMessage

These underlying functions are called in below contracts

Proof of Concept

Contract : Messages.sol Line : 515 - There is no check for array length match for ids and amounts.

   function encodeTransferErc1155BatchMessage(
        address token,
        address receiver,
        uint256[] memory ids,
        uint256[] memory amounts
    ) internal pure returns (bytes memory) {
        TransferErc1155BatchMessage memory message = TransferErc1155BatchMessage(
            BaseMessage(MessageType.TRANSFER_ERC1155_BATCH),
            token,
            receiver,
            ids,
            amounts
        );
        return abi.encode(message);
    }

Recommended Mitigation Steps

Check if the array length of ids and amounts are same during encoding and decoding of the Erc1155BatchMessage.

cstrangedk commented 2 years ago

Issue is correct, but

This is a known issue was explicitly stated in the README under https://github.com/code-423n4/2022-02-skale#known-issues

GalloDaSballo commented 2 years ago

I fail to understand why this would actually cause issues, if the call reverts no state changes would happen.

Additionally, as the issue was marked out of scope in the readme, am going to mark this finding invalid