joestakey - Lack of CEI in mintRollovers can be exploited by users two inflate their shares amount

[sherlock-admin]

joestakey

high

Lack of CEI in mintRollovers can be exploited by users two inflate their shares amount

Summary

Because storage is updated after minting the shares, a malicious attacker can enter delistRollover using onERC1155Received to inflate the shares amount of another user.

Vulnerability Detail

minting of shares in rollover is done before updating storage

Earthquake/src/v2/Carousel/Carousel.sol
437: _mintShares(queue[index].receiver, _epochId, assetsToMint); //@audit reenter delistInRollover
438:                     emit Deposit(
439:                         msg.sender,
440:                         queue[index].receiver,
441:                         _epochId,
442:                         assetsToMint
443:                     );
444:                     rolloverQueue[index].assets = assetsToMint;
445:                     rolloverQueue[index].epochId = _epochId;

_mintShares() mints shares to the depositor, in the form of an ERC1155 token. ERC1155._mint() has a safety hook to ensure the receiver handles this type of tokens:

File: Earthquake/lib/openzeppelin-contracts/contracts/token/ERC1155/ERC1155.sol
285: _doSafeTransferAcceptanceCheck(operator, address(0), to, id, amount, data);
...
467: function _doSafeTransferAcceptanceCheck(
468:         address operator,
469:         address from,
470:         address to,
471:         uint256 id,
472:         uint256 amount,
473:         bytes memory data
474:     ) private {
475:         if (to.isContract()) {
476:             try IERC1155Receiver(to).onERC1155Received(operator, from, id, amount, data) returns (bytes4 response) {
477:                 if (response != IERC1155Receiver.onERC1155Received.selector) {
478:                     revert("ERC1155: ERC1155Receiver rejected tokens");
479:                 }
480:             } catch Error(string memory reason) {
481:                 revert(reason);
482:             } catch {
483:                 revert("ERC1155: transfer to non ERC1155Receiver implementer");
484:             }
485:         }
486:     }

This can be exploited by a depositor:
if they use a smart contract to deposit, and have the following logic in onERC1155Received: call delistRollover

This will delete them from the queue, and place the last user at their index

File: Earthquake/src/v2/Carousel/Carousel.sol
292: } else {
293:             // overwrite the item to be removed with the last item in the queue
294:             rolloverQueue[index] = rolloverQueue[length - 1]; //@audit key part of the attack
295:             // remove the last item in the queue
296:             rolloverQueue.pop();
297:             // update the index of prev last user ( mapping index is allways array index + 1)
298:             ownerToRollOverQueueIndex[rolloverQueue[index].receiver] =
299:                 index +
300:                 1;
301:             // remove receiver from index mapping
302:             delete ownerToRollOverQueueIndex[_owner];
303:         }

Which will then be updated at the end of mintRollovers.

File: Earthquake/src/v2/Carousel/Carousel.sol
444: rolloverQueue[index].assets = assetsToMint;
445:                     rolloverQueue[index].epochId = _epochId;

Meaning that "new" user at index index has inflated their shares for free (if assetToMint is greater than what they had previously)

Impact

The attacker can inflate shares of another users.
Note that they do not lose any shares themselves, as they can still withdraw() in this new epoch.

Code Snippet

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

Tool used

Manual Review

Recommendation

Earthquake/src/v2/Carousel/Carousel.sol
-437: _mintShares(queue[index].receiver, _epochId, assetsToMint); //@audit reenter delistInRollover
438:                     emit Deposit(
439:                         msg.sender,
440:                         queue[index].receiver,
441:                         _epochId,
442:                         assetsToMint
443:                     );
444:                     rolloverQueue[index].assets = assetsToMint;
445:                     rolloverQueue[index].epochId = _epochId;
+ _mintShares(queue[index].receiver, _epochId, assetsToMint); //@audit reenter delistInRollover

Duplicate of #468