sherlock-audit / 2024-04-titles-judging

9 stars 6 forks source link

ComposableSecurity - The `mintBatch` function with multiple tokenIds always reverts #418

Closed sherlock-admin3 closed 4 months ago

sherlock-admin3 commented 5 months ago

ComposableSecurity

medium

The mintBatch function with multiple tokenIds always reverts

Summary

The mintBatch function is not functional as it would always revert when user tries to mint more than 1 token, because the whole msg.value is sent to the fee manager in each for loop iteration.

Vulnerability Detail

The mintBatch function allows to mint tokens for multiple works in one call.

In each iteration of the for loop the whole msg.value is sent to the fee manager. However, in the second iteration the edition has insufficient balance, because the whole value has been sent in the previous iteration.

That said, any call to mintBatch function with multiple tokens will revert.

PoC

    function test_mintBatch() public {
        uint256[] memory tokenIds = new uint256[](2);
        tokenIds[0] = 1;
        tokenIds[1] = 1;

        uint256[] memory amounts = new uint256[](2);
        amounts[0] = 1;
        amounts[1] = 1;

        edition.mintBatch{value: 0.0212 ether}(address(1), tokenIds, amounts, new bytes(0));
    }
  [152304] EditionTest::test_mintBatch()
    ├─ [138878] Edition::mintBatch{value: 21200000000000000}(0x0000000000000000000000000000000000000001, [1, 1], [1, 1], 0x)
    │   ├─ [86455] FeeManager::collectMintFee{value: 21200000000000000}(Edition: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], 1, 1, EditionTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496], 0x0000000000000000000000000000000000000000, Strategy({ asset: 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE, mintFee: 10000000000000000 [1e16], revshareBps: 2500, royaltyBps: 250 }))
    │   │   ├─ [3000] 0x0000000000000000000000000000000000000001::fallback{value: 10000000000000000}()
    │   │   │   └─ ← [Return] 
    │   │   ├─ [0] 0x0000000000000000000000000000000000C0FFEE::fallback{value: 600000000000000}()
    │   │   │   └─ ← [Stop] 
    │   │   ├─ emit FeeCollected(edition: Edition: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], work: 1, asset: 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE, fee: 10600000000000000 [1.06e16], referrerShare: 0)
    │   │   └─ ← [Stop] 
    │   ├─ [0] FeeManager::collectMintFee{value: 21200000000000000}(Edition: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], 1, 1, EditionTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496], 0x0000000000000000000000000000000000000000, Strategy({ asset: 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE, mintFee: 10000000000000000 [1e16], revshareBps: 2500, royaltyBps: 250 }))
    │   │   └─ ← [OutOfFunds] EvmError: OutOfFunds
    │   └─ ← [Revert] EvmError: Revert
    └─ ← [Revert] EvmError: Revert

Impact

The mintBatch function is not functional as it would always revert when user tries to mint more than 1 token.

Code Snippet

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

Tool used

Manual Review

Recommendation

Send the only required portion of the msg.value in each iteration.

Duplicate of #280

ComposableSecurityTeam commented 4 months ago

Escalate

This is a valid issue. The similar issue was accepted and is rewarded here: https://github.com/sherlock-audit/2024-04-titles-judging/issues/280

sherlock-admin3 commented 4 months ago

Escalate

This is a valid issue. The similar issue was accepted and is rewarded here: https://github.com/sherlock-audit/2024-04-titles-judging/issues/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 4 months ago

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

Evert0x commented 4 months ago

Result: Medium Duplicate of #280

sherlock-admin4 commented 4 months ago

Escalations have been resolved successfully!

Escalation status: