KupiaSec - Improper handling of `msg.value` in the `Edition::mintBatch` function

[sherlock-admin4]

KupiaSec

high

Improper handling of msg.value in the Edition::mintBatch function

Summary

The Edition::mintBatch function is designed to mint multiple tokens for the given works and minter should send required amount of Ether as msg.value. However msg.value is used within a loop and it will try to pull much more Ether from Edition than sent from minter.

Vulnerability Detail

Users are able to use the Edition::mintBatch function to mint multiple tokens for the given works. The current implementation of the Edition::mintBatch function is as follows:

Edition.sol
277: function mintBatch(
278:         address to_,
279:         uint256[] calldata tokenIds_,
280:         uint256[] calldata amounts_,
281:         bytes calldata data_
282:     ) external payable {
283:         for (uint256 i = 0; i < tokenIds_.length; i++) {
284:             Work storage work = works[tokenIds_[i]];
285: 
286:             // wake-disable-next-line reentrancy
287:             FEE_MANAGER.collectMintFee{value: msg.value}(// @audit msg.value within a loop
288:                 this, tokenIds_[i], amounts_[i], msg.sender, address(0), work.strategy
289:             );
290: 
291:             _checkTime(work.opensAt, work.closesAt);
292:             _updateSupply(work, amounts_[i]);
293:         }
294: 
295:         _batchMint(to_, tokenIds_, amounts_, data_);
296:         _refundExcess();
297:     }

As evident from the provided code, at L287 the msg.value is sent to the FeeManager contract multiple times within a loop. This implementation flaw could result in the unintended drainage of Ether from the Edition contract.

If the Edition contract has a sufficient Ether balance, more Eth is transfered to FEE_MANAGER. However, if the Edition contract's Ether balance is depleted, the mintBatch function will always revert due to insufficient funds.

Impact

There are two potential impacts in the Edition::mintBatch function implementation:

• If the Edition contract has a sufficient Ether balance, minters may be able to drain the contract's Ether by calling the mintBatch function, effectively obtaining multiple items without paying the required amount of Ether.

• In the event the Edition contract does not have a sufficient Ether balance, any attempt to call the mintBatch function will always result in a revert due to insufficient funds.

Code Snippet

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

Tool used

Manual Review

Recommendation

It is recommended to refactor the implementation of the Edition::mintBatch function to avoid the use of msg.value within a loop.

Duplicate of #280

[KupiaSecAdmin]

Escalate

This is a duplicate of #280

[sherlock-admin3]

Escalate

This is a duplicate 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]

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

[Evert0x]

Result: Medium Duplicate of #280

[sherlock-admin4]

Escalations have been resolved successfully!

Escalation status:

• KupiaSecAdmin: accepted