Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
sherlock-admin3 opened this issue Apr 26, 2024 · 8 comments
Closed
Labels
Escalation Resolved This issue's escalations have been approved/rejected Excluded Excluded by the judge without consulting the protocol or the senior Non-Reward This issue will not receive a payout

Comments

@sherlock-admin3
Copy link
Contributor

sherlock-admin3 commented Apr 26, 2024

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().

@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 Zany Golden Cat - TitlesCore does not refund excess creation fee payment CodeWasp - TitlesCore does not refund excess creation fee payment May 12, 2024
@sherlock-admin2 sherlock-admin2 added the Non-Reward This issue will not receive a payout label May 12, 2024
@thpani
Copy link

thpani commented May 13, 2024

Escalate

This is a dup of #30

@sherlock-admin3
Copy link
Contributor Author

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

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

thpani commented May 17, 2024

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

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

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

@Evert0x
Copy link

Evert0x commented May 20, 2024

Result:
Invalid
Unique

@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

Escalations have been resolved successfully!

Escalation status:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Escalation Resolved This issue's escalations have been approved/rejected Excluded Excluded by the judge without consulting the protocol or the senior Non-Reward This issue will not receive a payout
Projects
None yet
Development

No branches or pull requests

7 participants