nobody2018 - relayer can never successfully execute mintDepositInQueue and mintRollovers due to ERC1155's _doSafeTransferAcceptanceCheck

[sherlock-admin]

nobody2018

high

relayer can never successfully execute mintDepositInQueue and mintRollovers due to ERC1155's _doSafeTransferAcceptanceCheck

Summary

As we all know, the ERC1155 token of OpenZeppelin will call _doSafeTransferAcceptanceCheck internally when token transferring and minting. If to is a contract, _doSafeTransferAcceptanceCheck will callback to 'to contract'. Carousel is inherited from ERC1155. When a user deposits asset into Carousel, Carousel mints the equivalent amount of ERC1155 tokens to the user. Before a new round of epoch is started, off-chain relayer calls mintDepositInQueue to process users' pre-deposited assets, and mints the corresponding ERC1155 tokens to these users. Meanwhile, off-chain relayer also calls mintRollovers to burn the ERC1155 locked by the user in the previous round of epoch, and mints the ERC1155 token to these users in the next round. If there are malicious receiver contracts in the depositQueue or rolloverQueue, when a callback is triggered, the malicious contract can either directly revert or make a infinite loop to exhaust gas, leading to DOS. Therefore, relayer can never successfully call these two functions.

Vulnerability Detail

Let's begin DOS attack step by step. Here are two scenarios. Let's first look at the first one:

1. attacker deploys a contract A and calls Carousel.deposit(0, relayFee+1, A).
2. time goes by, new round of epoch is coming. The depositQueue already has 100 QueueItem.
3. relayer calls mintDepositInQueue(newRoundId, 50), and the QueueItem of contract A is one of the 50 items to be processed this time.
4. when mintDepositInQueue mints token for A, it callback to A. A's code triggers DOS. This transaction revert.
5. relayer tries again to go back to step 3.

In summary, it is never possible for relayer to empty depositQueue.

Let's take a look at the second one, assuming that the current epoch is E1 and the next round is E2:

1. attacker deploys a contract A and calls Carousel.deposit(E1, 3 * relayFee, A), _balances[E1][A] = 2*relayFee.
2. attacker calls Carousel.enlistInRollover(E1, 2 * relayFee, A).
3. time goes by, round E1 finished, round E2 is coming. The rolloverQueue already has 100 QueueItem.
4. relayer calls mintRollovers(E2, 50), and the QueueItem of contract A is one of the 50 items to be processed this time.
5. when mintRollovers mints token for A, it callback to A. A's code triggers DOS. This transaction revert.
6. relayer tries again to go back to step 4.

In summary, it is never possible for relayer to handle rolloverQueue .

Impact

Due to DOS attack, depositQueue or rolloverQueue  can't be processed in time. This will destroy the normal process of the protocol. In fact, this issue can also cause the relayer to pay a large gas fee, which does not exceed the gas limit for one block.

Code Snippet

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC1155/ERC1155.sol#L447-L466

https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/Carousel/Carousel.sol#L334-L338

https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/Carousel/Carousel.sol#L437

Tool used

Manual Review

Recommendation

There are two ways worth recommending:

• remove the code of ERC1155._doSafeTransferAcceptanceCheck to prevent calls to untrusted code.
• Inside Carousel._mintShares, special handling is performed for the case where to is a contract.

Duplicate of #468