Skip to content
This repository has been archived by the owner on Oct 1, 2023. It is now read-only.

HonorLt - DOS queue minting of shares #315

Closed
sherlock-admin opened this issue Mar 27, 2023 · 0 comments
Closed

HonorLt - DOS queue minting of shares #315

sherlock-admin opened this issue Mar 27, 2023 · 0 comments
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label High A valid High severity issue Reward A payout will be made for this issue

Comments

@sherlock-admin
Copy link
Contributor

sherlock-admin commented Mar 27, 2023

HonorLt

high

DOS queue minting of shares

Summary

A queue can be blocked if any of the recipient is a smart contract that reverts on onERC1155Received.

Vulnerability Detail

When minting shares, it calls an internal _mint function:

 function _mintShares(
        address to,
        uint256 id,
        uint256 amount
    ) internal {
        _mint(to, id, amount, EMPTY);
        _mintEmissions(to, id, amount);
    }

This function requires that if the recipient is a smart contract, it implements the onERC1155Received:

    function _mint(
        address to,
        uint256 id,
        uint256 amount,
        bytes memory data
    ) internal virtual {
        balanceOf[to][id] += amount;

        emit TransferSingle(msg.sender, address(0), to, id, amount);

        require(
            to.code.length == 0
                ? to != address(0)
                : ERC1155TokenReceiver(to).onERC1155Received(msg.sender, address(0), id, amount, data) ==
                    ERC1155TokenReceiver.onERC1155Received.selector,
            "UNSAFE_RECIPIENT"
        );
    }

The _mintShares function is utilized in _deposit, mintDepositInQueue, and mintRollovers. The problem is that the latter 2 functions execute the minting in a loop, e.g.:

        while ((length - _operations) <= i) {
            // this loop impelements FILO (first in last out) stack to reduce gas cost and improve code readability
            // changing it to FIFO (first in first out) would require more code changes and would be more expensive
            _mintShares(
                queue[i].receiver,
                _epochId,
                queue[i].assets - relayerFee
            );
            emit Deposit(
                msg.sender,
                queue[i].receiver,
                _epochId,
                queue[i].assets - relayerFee
            );
            depositQueue.pop();

A malicious actor can write a custom recipient smart contract that does not revert on deposit but reverts when queue actions are executed. This way it will block the queue which is implemented in a FILO principle.

Impact

Anyone can block the queue with a custom smart contract that reverts.

Code Snippet

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

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

Tool used

Manual Review

Recommendation

There are many options, for example, allowing to skip certain items, performing the operation in a try/catch, or overriding the mint function to skip this check.

Duplicate of #468

@github-actions github-actions bot closed this as completed Apr 3, 2023
@github-actions github-actions bot added High A valid High severity issue Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label labels Apr 3, 2023
@sherlock-admin sherlock-admin added the Reward A payout will be made for this issue label Apr 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label High A valid High severity issue Reward A payout will be made for this issue
Projects
None yet
Development

No branches or pull requests

1 participant