search

sherlock-audit / 2024-04-titles-judging

9 stars 6 forks source link

nisedo - `Edition.mintBatch()` uses `msg.value` in a for loop, allowing users to mint tokens for free #254

Closed sherlock-admin3 closed 4 months ago

sherlock-admin3 commented 4 months ago

nisedo

high

Edition.mintBatch() uses msg.value in a for loop, allowing users to mint tokens for free

Summary

Edition.mintBatch() uses msg.value in a for loop which allows to mint tokens for free.

Vulnerability Detail

Using msg.value inside a loop is dangerous because it allows the sender to “re-use” the msg.value (cf RareSkills blog).

Edition.mintBatch() is used to mint multiple tokens at once, paying a mint fee for each token bought.

However, due to this issue, a malicious user can mint multiple tokens by paying only once for the MintFee.

    /// @notice Mint multiple tokens for the given works.
    /// @param to_ The address to mint the tokens to.
    /// @param tokenIds_ The IDs of the works to mint.
    /// @param amounts_ The amounts of each work to mint.
    /// @param data_ The data associated with the mint. Reserved for future use.
    function mintBatch(
        address to_,
        uint256[] calldata tokenIds_,
        uint256[] calldata amounts_,
        bytes calldata data_
    ) external payable {
        for (uint256 i = 0; i < tokenIds_.length; i++) {
            Work storage work = works[tokenIds_[i]];

❌          FEE_MANAGER.collectMintFee{value: msg.value}(
                this, tokenIds_[i], amounts_[i], msg.sender, address(0), work.strategy
            );

            _checkTime(work.opensAt, work.closesAt);
            _updateSupply(work, amounts_[i]);
        }

        _batchMint(to_, tokenIds_, amounts_, data_);
        _refundExcess();
    }

PoC

Here is the full commented PoC: https://gist.github.com/nisedo/f6ff2380f0142186a5ef80f06d8070e2

Paste it in wallflower-contract-v2/test/nisedoTest.t.sol and run it with forge test --mt test_nisedo_mintBatch -vvvv

Impact

Free minting of tokens.

Code Snippet

Tool used

Manual Review

Recommendation

Avoid using msg.value in a for loop.

Result: High Duplicate of #264

nisedo commented 3 months ago

Escalate

Dup of #264

sherlock-admin3 commented 3 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 3 months ago

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

Evert0x commented 3 months ago

Result: High Duplicate of #264

sherlock-admin4 commented 3 months ago

Escalations have been resolved successfully!

Escalation status: