sherlock-audit / 2024-04-titles-judging

9 stars 6 forks source link

xiaoming90 - Minting can be DOSed by any of the fee recipients #261

Open sherlock-admin4 opened 4 months ago

sherlock-admin4 commented 4 months ago

xiaoming90

high

Minting can be DOSed by any of the fee recipients

Summary

The minting process might be DOS by malicious fee recipients, breaking the protocol's core functionality. This would also result in the loss of fee for the rest of the innocent fee recipients as the minting cannot proceed, resulting in the minting fee not being collected.

Vulnerability Detail

When minting a new token for a given work, the minting fee will be collected as per Line 236 below.

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

File: Edition.sol
222:     /// @notice Mint a new token for the given work.
223:     /// @param to_ The address to mint the token to.
224:     /// @param tokenId_ The ID of the work to mint.
225:     /// @param amount_ The amount of tokens to mint.
226:     /// @param referrer_ The address of the referrer.
227:     /// @param data_ The data associated with the mint. Reserved for future use.
228:     function mint(
229:         address to_,
230:         uint256 tokenId_,
231:         uint256 amount_,
232:         address referrer_,
233:         bytes calldata data_
234:     ) external payable override {
235:         // wake-disable-next-line reentrancy
236:         FEE_MANAGER.collectMintFee{value: msg.value}(
237:             this, tokenId_, amount_, msg.sender, referrer_, works[tokenId_].strategy
238:         );
239: 
240:         _issue(to_, tokenId_, amount_, data_);
241:         _refundExcess();
242:     }

The collected minting fee will be routed or transferred to one or more of the following parties directly (Using a push mechanism):

  1. Edition's creator (Using 0xSplit wallet)
  2. Edition's attributions (no limit on the number of attributions in the current setup) (Using 0xSplit wallet)
  3. Collection referrer
  4. Minting referrer
  5. Protocol fee receiver

This approach introduced the risk of DOS to the minting process. As long as any of the parties above intentionally revert upon receiving the ETH minting fee, they could DOS the entire minting process of work.

Note: The parties using the 0xSplit wallet are not an issue because the wallet requires the recipients to claim the fees from the 0xSplit wallet manually (Using the pull mechanism instead)

Impact

The minting process might be DOS by malicious fee recipients, breaking the protocol's core functionality. This would also result in the loss of fee for the rest of the innocent fee recipients as the minting cannot proceed, resulting in the minting fee not being collected.

Code Snippet

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

Tool used

Manual Review

Recommendation

Consider adopting a pull mechanism for the fee recipients to claim their accrued fee instead of transferring the fee directly to them during the minting process.

xiaoming9090 commented 3 months ago

Escalate

The two (2) actors mentioned in this report, the "Collection referrer" and "Minting referrer," are external parties of the protocol that are not fully trusted. When these two external actors behave maliciously, they could negatively impact the other innocent parties (Edition's creator, Edition's attributions, Protocol fee receiver), including the protocol, by preventing them from receiving their fee, resulting in a loss of assets for the victims.

As such, this issue should not be overlooked due to its risks and should be considered valid.

sherlock-admin3 commented 3 months ago

Escalate

The two (2) actors mentioned in this report, the "Collection referrer" and "Minting referrer," are external parties of the protocol that are not fully trusted. When these two external actors behave maliciously, they could negatively impact the other innocent parties (Edition's creator, Edition's attributions, Protocol fee receiver), including the protocol, by preventing them from receiving their fee, resulting in a loss of assets for the victims.

As such, this issue should not be overlooked due to its risks and should be considered valid.

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, even though it's pure griefing and the referrers lose fees themselves, the protocol also loses fee. Planning to accept and validate the report.

Evert0x commented 3 months ago

Result: Medium Has Duplicates

sherlock-admin4 commented 3 months ago

Escalations have been resolved successfully!

Escalation status: