code-423n4 / 2024-04-panoptic-findings

9 stars 4 forks source link

inadequate Handling of Receiver Logic in safeTransferFrom #538

Closed c4-bot-7 closed 7 months ago

c4-bot-7 commented 7 months ago

Lines of code

https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/tokens/ERC1155Minimal.sol#L112-L121

Vulnerability details

Impact

the bug is can disrupt user experience and contract integrations, where legitimate contracts might intend to receive tokens but haven't implemented the necessary receiver interface, thus causing all transfers to these contracts to fail.

root of the bug and summary

the contract checks if to is a contract inn this line to.code.length != 0 and then immediately calls onERC1155Received on to. The logical assumption here is that any contract address will correctly implement ERC1155Holder or, at least, a compliant onERC1155Received function if to is a contract but does not implement ERC1155Holder or the expected interface, this will cause a call to a non-existent function, leading to a failed transaction here :

if (to.code.length != 0) {
    if (
        ERC1155Holder(to).onERC1155Received(msg.sender, from, id, amount, data) !=
        ERC1155Holder.onERC1155Received.selector
    ) {
        revert UnsafeRecipient();
    }
}

the root of this vulnerability lies in the assumption that any contract address in the to field will properly implement the required ERC-1155 token receiver interface. There's no preliminary check to ensure that to indeed supports the interface, which could lead to transaction failures if to is a contract that doesn't support handling ERC-1155 tokens correctly.

Tools Used

manual review

Recommended Mitigation Steps

need that Before making the call to onERC1155Received, use ERC-165 standard checks to verify if the contract implements the ERC-1155 token receiver interface

Assessed type

Other

c4-judge commented 7 months ago

Picodes marked the issue as unsatisfactory: Invalid