sherlock-audit / 2023-03-Y2K-judging

7 stars 1 forks source link

mstpr-brainbot - Rollover Queue Stuck Issue in ERC1155 Contracts #470

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

mstpr-brainbot

high

Rollover Queue Stuck Issue in ERC1155 Contracts

Summary

The rollover queue functionality in ERC1155 contracts can become stuck when encountering a contract with a specific onErc1155Received implementation, such as Alice's contract example. This critical issue impacts the primary purpose of the rollover queue.

Vulnerability Detail

The rollover queue, similar to the deposit queue, does not verify if the recipient is a contract with a properly implemented onErc1155Received hook.

Example: Consider a situation where Alice's contract has an onErc1155Received function as shown below:

uint256 count = 0;
function onERC1155Received(
        address operator,
        address from,
        uint256 id,
        uint256 value,
        bytes calldata data
    ) external returns (bytes4) {
    if (count == 0) {
        return bytes4("Success");
    }
    revert("Malicious"); 
}

In this case, Alice's contract allows minting only once. During the first deposit phase, Alice's contract will successfully receive the tokens. However, when Alice is in the rollover queue, her contract's if block will cause the function to fail. This will result in the queue becoming stuck for users following Alice. To access their funds, users after Alice must remove themselves from the rollover queue, as Alice's queue position will prevent the rollovers from proceeding.

Impact

Rollovers will become stuck, preventing the rollover queue from achieving its intended purpose. However, users in the rollover queue can delist themselves to exit their positions. This is a critical finding since it thoroughly disrupts the rollover queue functionality. Nonetheless, it can be considered a medium severity issue since users have a way to escape the situation.

Code Snippet

https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/Carousel/Carousel.sol#L437 will fail in case of the Alice contracts

Tool used

Manual Review

Recommendation

Do not allow smart contracts without proper onERC1155Received function

Duplicate of #468