Protocol is not ERC-1155 compliant

[c4-bot-10]

Lines of code

https://github.com/code-423n4/2023-11-shellprotocol/blob/main/src/ocean/Ocean.sol#L317-L366

Vulnerability details

Impact

The Ocean contract is not fully compliant with the ERC-1155 standard. Specifically, the onERC1155Received and onERC1155BatchReceived functions do not revert when they reject a transfer. According to the ERC-1155 standard, these functions MUST revert if they reject a transfer.
This may have been a deliberate choice by the authors given the comments in the functions' documentation, but ERC-1155 adherence is specifically listed as an invariant and this behaviour represents strict non-conformity.

Proof of Concept

This could lead to a potential loss of funds if an external IERC1155 contract is improperly implemented and doesn't check for the return value of onERC1155Received. In such a case, a user could unknowingly transfer tokens to the Ocean contract without a proper interaction. Since the Ocean contract doesn't revert the transaction, the tokens would be transferred successfully, but without the required interaction to credit them to the user in the ocean. This could lead to the tokens being stuck in the Ocean contract, resulting in a loss of funds for the user.

Tools Used

Manual inspection

Recommended Mitigation Steps

To ensure full compliance with the ERC-1155 standard, modify the onERC1155Received and onERC1155BatchReceived functions to revert when a transfer is rejected. This can be achieved by replacing the return 0; statement with a revert statement.

Assessed type

Other

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #218

[c4-judge]

0xA5DF marked the issue as not a duplicate

[c4-judge]

0xA5DF marked the issue as primary issue

[c4-judge]

0xA5DF changed the severity to QA (Quality Assurance)

[0xA5DF]

This could lead to a potential loss of funds if an external IERC1155 contract is improperly implemented and doesn't check for the return value of onERC1155Received. In such a case, a user could unknowingly transfer tokens to the Ocean contract without a proper interaction.

This is a loss of funds due to a mistake on the user's side.
Marking as low

[0xA5DF]

Moved to #336

[c4-judge]

0xA5DF marked the issue as grade-c

[c4-judge]

0xA5DF marked the issue as grade-a