Skip to content
This repository has been archived by the owner on Oct 27, 2024. It is now read-only.

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

Open
sherlock-admin4 opened this issue Apr 26, 2024 · 5 comments
Open
Labels
Escalation Resolved This issue's escalations have been approved/rejected Has Duplicates A valid issue with 1+ other issues describing the same vulnerability Medium A valid Medium severity issue Reward A payout will be made for this issue Sponsor Disputed The sponsor disputed this issue's validity Won't Fix The sponsor confirmed this issue will not be fixed

Comments

@sherlock-admin4
Copy link
Contributor

sherlock-admin4 commented Apr 26, 2024

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.

@github-actions github-actions bot closed this as completed May 6, 2024
@github-actions github-actions bot added the Excluded Excluded by the judge without consulting the protocol or the senior label May 6, 2024
@sherlock-admin2 sherlock-admin2 changed the title Winning Scarlet Yeti - Minting can be DOSed by any of the fee recipients xiaoming90 - Minting can be DOSed by any of the fee recipients May 12, 2024
@sherlock-admin2 sherlock-admin2 added the Non-Reward This issue will not receive a payout label May 12, 2024
@xiaoming9090
Copy link
Collaborator

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
Copy link
Contributor

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.

@sherlock-admin4 sherlock-admin4 added the Escalated This issue contains a pending escalation label May 15, 2024
@WangSecurity
Copy link
Collaborator

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 Evert0x reopened this May 20, 2024
@Evert0x Evert0x added the Medium A valid Medium severity issue label May 20, 2024
@sherlock-admin2 sherlock-admin2 added Reward A payout will be made for this issue and removed Non-Reward This issue will not receive a payout labels May 20, 2024
@Evert0x
Copy link

Evert0x commented May 20, 2024

Result:
Medium
Has Duplicates

@sherlock-admin2 sherlock-admin2 removed the Escalated This issue contains a pending escalation label May 20, 2024
@sherlock-admin3 sherlock-admin3 added the Escalation Resolved This issue's escalations have been approved/rejected label May 20, 2024
@sherlock-admin4
Copy link
Contributor Author

Escalations have been resolved successfully!

Escalation status:

@sherlock-admin3 sherlock-admin3 added Sponsor Disputed The sponsor disputed this issue's validity Won't Fix The sponsor confirmed this issue will not be fixed labels May 21, 2024
@sherlock-admin2 sherlock-admin2 added Has Duplicates A valid issue with 1+ other issues describing the same vulnerability and removed Excluded Excluded by the judge without consulting the protocol or the senior labels May 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Escalation Resolved This issue's escalations have been approved/rejected Has Duplicates A valid issue with 1+ other issues describing the same vulnerability Medium A valid Medium severity issue Reward A payout will be made for this issue Sponsor Disputed The sponsor disputed this issue's validity Won't Fix The sponsor confirmed this issue will not be fixed
Projects
None yet
Development

No branches or pull requests

6 participants