User can game the protocol and mint multiple NFTs for the cost of 1

Summary

Edition::mintBatch doesn't correctly calculate the total price/fee of minting a NFT to multiple addresses.

Vulnerability Detail

This is the Edition::mintBatch function:

    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
        );

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

        _refundExcess();
    }

As we can see, it's correctly collecting the fee for 1 mint, but then it proceeds to mint the NFT to multiple addresses. A user can call this function with an array of multiple addresses (which he can own them all) and mint a NFT to each of them for the total price of 1.

Impact

A user can mint multiple NFTs for the price of 1, essentially inflating the NFT and making it worthless.

Proof of Code

Paste the following code in the Edition.t.sol file:

    address[] receivers = [makeAddr("bob"), makeAddr("alice"), makeAddr("etc"), makeAddr("infinite")];

    function test_mint_batch_infinite_mint() public {
        // validate that minting one "work" costs 0.0106 ether
        edition.mint{value: 0.0106 ether}(address(1), 1, 1, address(0), new bytes(0));
        // fees are distributed correctly
        assertEq(address(1).balance, 0.01 ether);
        assertEq(address(0xc0ffee).balance, 0.0006 ether);

        // mint 4 NFTs, 1 to each of our 4 addresses for the cost of 1
        edition.mintBatch{value: 0.0106 ether}(receivers, 1, 1, new bytes(0));
        // fees are updated correctly (our previous mint of 1 + this)
        assertEq(address(1).balance, 0.02 ether);
        assertEq(address(0xc0ffee).balance, 0.0012 ether);
    }

Code Snippet

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

Tool used

Manual Review

Recommendation

My recommendation is to correctly calculate the whole fee:

    function mintBatch(address[] calldata receivers_, uint256 tokenId_, uint256 amount_, bytes calldata data_) external payable {
+       uint256 fee = mintFee(tokenId_, amount_) * receivers_.length;
+       FEE_MANAGER.collectMintFee{value: fee}(
-       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();
    }

This makes the transaction revert if the user hasn't sent enough ETH.

sherlock-audit / 2024-04-titles-judging

Kalogerone - User can game the protocol and mint multiple NFTs for the cost of 1 #288