No revert on zero address breaks ERC1155 compatibility

[c4-bot-1]

Lines of code

https://github.com/code-423n4/2023-11-panoptic/blob/f75d07c345fd795f907385868c39bafcd6a56624/contracts/tokens/ERC1155Minimal.sol#L90-L118 https://github.com/code-423n4/2023-11-panoptic/blob/f75d07c345fd795f907385868c39bafcd6a56624/contracts/tokens/ERC1155Minimal.sol#L128-L171

Vulnerability details

Impact

ERC1155Minimal does not revert on a transfer where _to is the zero address. According to the EIP1155 specification this must hold true: MUST revert if _to is the zero address

Additionally, in the contest description ERC1155 compliance is explicitly mentioned as a requirement.

One implication is that TransferSingle events will be emitted that are specified as burns (to == address(0)), although no actual burn has happened (while unusable, the token still exists).

Proof of Concept

Looking at ERC1155Minimal.safeTransferFrom and its callchain from afterTokenTransfer -> registerTokenTransfer it can be seen that no revert will happen specifically for the case to == address(0)

function safeTransferFrom(
    address from,
    address to,
    uint256 id,
    uint256 amount,
    bytes calldata data
) public {
    if (!(msg.sender == from || isApprovedForAll[from][msg.sender])) revert NotAuthorized();

    balanceOf[from][id] -= amount;

    unchecked {
        balanceOf[to][id] += amount;
    }

    afterTokenTransfer(from, to, id, amount);

    emit TransferSingle(msg.sender, from, to, id, amount);

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

Tools Used

Manual Review

Recommended Mitigation Steps

Add a check to revert on to == address(0) in safeTransferFrom and safeBatchTransferFrom

if (to == address(0)) revert();

Assessed type

Other

[c4-judge]

Picodes marked the issue as duplicate of #81

[c4-judge]

Picodes changed the severity to QA (Quality Assurance)