sherlock-audit / 2024-04-titles-judging

1 stars 1 forks source link

trachev - No funds can be refunded #333

Closed sherlock-admin4 closed 2 months ago

sherlock-admin4 commented 2 months ago

trachev

medium

No funds can be refunded

Summary

When a work token is minted, the caller supplies ETH to pay for the mint fees. Any excess funds are intended to be refunded to the msg.sender through the _refundExcess function. The issue is that no funds will ever be refunded.

Vulnerability Detail

_refundExcess sends any remaining ETH in the Edition.sol smart contract to the caller, aiming to refund any of the msg.value, that has not been used to cover the mint fees. The issue is that when FEE_MANAGER.collectMintFee{value: msg.value} is called, all of the msg.value is transferred to the FEE_MANAGER, and any ETH, that is left over after the fees are paid, is not sent back to the Edition.sol contract, but remains stuck in the FeeManager.sol contract:

FEE_MANAGER.collectMintFee{value: msg.value}(
    this, tokenId_, amount_, msg.sender, referrer_, works[tokenId_].strategy
);

Consequently, when _refundExcess is called only ETH left in the Edition.sol contract balance is sent to the initial caller, which in most cases will be close to or equal to 0.

Furthermore, as the remaining msg.value (that was not used for fee payments) is stuck in the FeeManager.sol contract, any other user can send it to the protocolFeeReceiver(if it is sufficient enough) by calling collectCreationFee. This will prevent the admin from getting the stuck refund through withdraw, and sending it back to the initial caller.

Impact

Loss of funds for minters.

Code Snippet

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

Tool used

Manual Review

Recommendation

Unused funds in FEE_MANAGER.collectMintFee should be sent back from the FeeManager.sol contract to the Edition.sol contract, in order for the refund functionality to work. Another soution would be to calculate the amount of fees that the mint is going to cost and provide only that amount in FEE_MANAGER.collectMintFee, instead of the entire msg.value.

Duplicate of #269

trachevgeorgi commented 1 month ago

Escalate This is definitely a duplicate of issue #269. Please recheck.

sherlock-admin3 commented 1 month ago

Escalate This is definitely a duplicate of issue #269. Please recheck.

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

Agree with the escalation, planning to accept and duplicate with #269

Evert0x commented 1 month ago

Result: High Duplicate of #269

sherlock-admin2 commented 1 month ago

Escalations have been resolved successfully!

Escalation status: