sherlock-audit / 2024-04-titles-judging

1 stars 1 forks source link

CodeWasp - TitlesCore does not refund excess creation fee payment #348

Closed sherlock-admin3 closed 2 months ago

sherlock-admin3 commented 2 months ago

CodeWasp

medium

TitlesCore does not refund excess creation fee payment

Summary

Funds sent to TitlesCore.createEdition() or TitlesCore.publish() exceeding the necessary creation fee are stuck in the contract.

Vulnerability Detail

TitlesCore does not refund excess payment on creation fees.

Impact

Excess fee payment is stuck in contract.

Code Snippet

https://github.com/sherlock-audit/2024-04-titles/blob/main/wallflower-contract-v2/src/TitlesCore.sol#L72-L74 https://github.com/sherlock-audit/2024-04-titles/blob/main/wallflower-contract-v2/src/TitlesCore.sol#L103-L105

Tool used

Manual Review

Recommendation

Refund excess fees in TitlesCore._publish().

thpani commented 1 month ago

Escalate

This is a dup of #30

sherlock-admin3 commented 1 month ago

Escalate

This is a dup of #30

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

Firstly, the escalation is asking to be a duplicate of #30 which is a duplicate itself. Secondly, we can consider duplicating it with #269 , but I believe this report is insufficient. From reading this report I see it as an opportunity loss and a user mistake to send excess ETH, cause the report doesn't mention that there is a functionality to refund it and why it doesn't work correctly.

Planning to reject the escalation and leave the issue as it is.

thpani commented 1 month ago

Firstly, the escalation is asking to be a duplicate of #30 which is a duplicate itself. Secondly, we can consider duplicating it with #269 , but I believe this report is insufficient.

@WangSecurity Please reconsider. #30 itself is incorrectly grouped with #269:

269 is about Edition.mint(), while #30 is about TitlesCore::_publish().

WangSecurity commented 1 month ago

Yes, you're correct, just noticed it myself. The #30 is planned to be invalidated as well due to the following reasons:

Excuse me, I believe this should remain invalid. Creation fee can only be changed by the owner, hence, there are two scenario's this issue may occur:

  1. User mistakenly sends more msg.value than creation fee -- user mistake
  2. Admin accidentally changes the fee right before the user creates the work -- admins are trusted, hence, this scenario is not viable.

While in situation with mint fee, it's changed with by the work creator, who is not trusted. Planning to reject this escalation cause doesn't effect reward distribution

Alireza-Razavi commented 1 month ago

@thpani @WangSecurity I believe this is dup of #36 which is already invalidated.

Evert0x commented 1 month ago

Result: Invalid Unique

sherlock-admin4 commented 1 month ago

Escalations have been resolved successfully!

Escalation status: