sherlock-audit / 2024-04-titles-judging

1 stars 1 forks source link

alexzoid - Incorrect Fee Handling in Batch Minting #427

Closed sherlock-admin4 closed 1 month ago

sherlock-admin4 commented 2 months ago

alexzoid

medium

Incorrect Fee Handling in Batch Minting

Summary

The mintBatch function in the Edition contract incorrectly handles Ether transactions when minting multiple tokens in a loop, leading to insufficient fee coverage.

Vulnerability Detail

In the mintBatch function, msg.value is used directly in the collectMintFee function for each iteration of the loop. As a result, the total Ether sent will either be insufficient to cover all fees.

Impact

This will lead to transactions failing due to insufficient fees for each minting operation.

Code Snippet

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

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

            // wake-disable-next-line reentrancy
            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();
    }

Tool used

Manual Review

Recommendation

Modify the mintBatch function to calculate the required fee for each mint operation individually and then sum these to determine the total fee required for the batch. Only this amount should be sent to the FEE_MANAGER. Any excess Ether should remain in the contract balance to be refunded by _refundExcess.

Duplicate of #280

alexzoid-eth commented 1 month ago

Escalate

This issue is a valid excluded dup of #280

sherlock-admin3 commented 1 month ago

Escalate

This issue is a valid excluded dup of #280

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 1 month ago

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

Evert0x commented 1 month ago

Result: Medium Duplicate of #280

sherlock-admin2 commented 1 month ago

Escalations have been resolved successfully!

Escalation status: