sherlock-audit / 2024-04-titles-judging

9 stars 6 forks source link

AlexCzm - Users can `mintBatch` to mint out but pay for one token only #199

Closed sherlock-admin4 closed 4 months ago

sherlock-admin4 commented 4 months ago

AlexCzm

high

Users can mintBatch to mint out but pay for one token only

Summary

Edition.mintBatch() lack a critical check that allows users to pay for one token and mint more than one token.

Vulnerability Detail

mintBatch allow users to mint a token to a set of receivers. The msg.sender pays the mint fee for amount_ tokens. Next, in the for loop the amount_ of tokenId is sent to each receiver. Total tokens minted is receivers_.length * amount_ but the mint fee collected is for amount_ tokens.

https://github.com/sherlock-audit/2024-04-titles/blob/d7f60952df22da00b772db5d3a8272a988546089/wallflower-contract-v2/src/editions/Edition.sol#L299-L320

    /// @notice Mint a token to a set of receivers for the given work.
    /// @param receivers_ The addresses to mint the tokens to.
    /// @param tokenId_ The ID of the work to mint.
    /// @param amount_ The amount of tokens to mint.
    /// @param data_ The data associated with the mint. Reserved for future use.
    function mintBatch(
        address[] calldata receivers_,
        uint256 tokenId_,
        uint256 amount_,
        bytes calldata data_
    ) external payable {
        // wake-disable-next-line reentrancy
        FEE_MANAGER.collectMintFee{value: msg.value}(
            this, tokenId_, amount_, msg.sender, address(0), works[tokenId_].strategy //@audit pays the mint fee for `amount_` tokens 
        );

        for (uint256 i = 0; i < receivers_.length; i++) {
            _issue(receivers_[i], tokenId_, amount_, data_);//@audit mint same `amount_` of tokenId multiplet times
        }

        _refundExcess();
    }

Impact

Users can mint more tokens than they paid for. Protocol, creator, etc collects (almost) no fees.

Code Snippet

Provided above

Tool used

Manual Review

Recommendation

Consider updating 'mintBatchfunction and require that the msg.sender to pay foramount * receivers.length` tokens:

    /// @notice Mint a token to a set of receivers for the given work.
    /// @param receivers_ The addresses to mint the tokens to.
    /// @param tokenId_ The ID of the work to mint.
    /// @param amount_ The amount of tokens to mint.
    /// @param data_ The data associated with the mint. Reserved for future use.
    function mintBatch(
        address[] calldata receivers_,
        uint256 tokenId_,
        uint256 amount_,
        bytes calldata data_
    ) external payable {

+        uint256 totalAmount = amount_ * receivers_.length;
+        uint256 totalMintFee = FEE_MANAGER.getMintFee(works[tokenId_].strategy, totalAmount).amount;
+        if (msg.value < totalMintFee) revert NotEnoughEther();

        // wake-disable-next-line reentrancy
        FEE_MANAGER.collectMintFee{value: msg.value}(
            this, tokenId_, amount_, msg.sender, address(0), works[tokenId_].strategy
        );

        for (uint256 i = 0; i < receivers_.length; i++) {
            _issue(receivers_[i], tokenId_, amount_, data_);
        }

        _refundExcess();
    }

Duplicate of #264

sammy-tm commented 4 months ago

Escalate

Dup of #264

sherlock-admin3 commented 4 months ago

Escalate

Dup of #264

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

WangSecurity commented 4 months ago

Agree with the escalation, planning to accept and duplicate with #264

Evert0x commented 4 months ago

Result: High Duplicate of #264

sherlock-admin4 commented 4 months ago

Escalations have been resolved successfully!

Escalation status: