Closed sherlock-admin4 closed 2 months ago
Escalate
Judge Sahab! This finding is a valid dup of #269, In fact this Watson added a coded PoC as well. Thanks!
Escalate
Judge Sahab! This finding is a valid dup of #269, In fact this Watson added a coded PoC as well. Thanks!
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.
Agree with the escalation, planning to accept and duplicate with #269
Result: High Duplicate of #269
Shaheen
medium
_refundExcess()
Functionality will Never Work and Users will Loose Ethers as the Funds will be Stuck in the FeeManager ContractSummary
_refundExcess()
will never work and users will loose ethers as the funds will be stuck in theFeeManager
contract (temporarily)Vulnerability Detail
The
refundExcess()
function is called after minting tokens to refund any ETH left in the contract after all fees have been collected. As we can see, this function transfers theEdition
contract's balance to the user (of course when expected):To understand the vulnerablity, we need to look at one of the minting functions. There are four minting functions, all utilizing
refundexcess()
mint()
,mintWithComment()
& bothmintBatch()
functions. Let's take onlymintWithComment()
to undertand the issue:As we can see, the
mintWithComment()
function, first calls theFeeManager
'scollectMintFee()
function, which calculates and takes the fee from the user), then it calls the_issue()
function, which mints an NFT to the user and then the_refundExcess()
function will called to return the excess amount to the user.Issue
The problem is, that when the
mintWithcomment()
function calls thecollectMintFee()
, it gives all themsg.value
to the FeeManager contract. But theFeeManager
contract never returns it back to theEdition
contract, which means all the excess fee will be stuck in the FeeManager contract and users will never get any excess amount back as the therefundExcess()
function only checks and transfersEditions
Contracts balance.Proof-of-Concept
Impact
Code Snippet
https://github.com/sherlock-audit/2024-04-titles/blob/d7f60952df22da00b772db5d3a8272a988546089/wallflower-contract-v2/src/editions/Edition.sol#L514 https://github.com/sherlock-audit/2024-04-titles/blob/d7f60952df22da00b772db5d3a8272a988546089/wallflower-contract-v2/src/editions/Edition.sol#L236
Tool used
Eyes
Recommendation
Make sure that the
FeeManager
contracts returns the excess fee amount to theEdition
contract.Duplicate of #269