Lines of code

Vulnerability details

The comment @ Ocean L348 states:

> The Ocean never initiates ERC1155 Batch Transfers.

This is untrue, note the following callstack:

Ocean.doMultipleInteractions | Ocean.forwardedDoMultipleInteractions > Ocean._doMultipleInteractions (>> calls _mintBatch @ L560 <<) > OceanERC1155._mintBatch > OceanERC1155._doSafeBatchTransferAcceptanceCheck (will revert) > Ocean.onERC1155BatchReceived() (returns 0 if target is Ocean contract)

Additionally, the comments of the spec for onERC1155BatchReceived() prescribe: (docs)

> Return of any other value than the prescribed keccak256 generated value MUST result in the transaction being reverted by the caller.

Since a revert is inevitable, it makes sense to decide weather or not the Ocean contract should accept these transfers or not, and if not just revert() instead of returning 0.

Recommended Mitigation Steps

The protocol team should determine if they want batch transfers to be accepted or rejected.

If accepted: Ideally this should return the proper selector and allow the transfer of multiple tokens.

If rejected: It would make sense for this function to revert() instead of returning 0 as any inproper selector MUST be reverted by the caller anyway. It makes sense to just revert in onERC1155Received() and onERC1155BatchReceived() here to save the end user gas and ensure compliance with the standard as any rejected transfer MUST revert.

Assessed type

Token-Transfer

The text was updated successfully, but these errors were encountered:

All reactions