sherlock-audit / 2024-04-titles-judging

10 stars 7 forks source link

oxchryston - No mechanism to refund excess `msg.value` in the function `collectCreationFee()` #347

Closed sherlock-admin4 closed 5 months ago

sherlock-admin4 commented 5 months ago

oxchryston

medium

No mechanism to refund excess msg.value in the function collectCreationFee()

Summary

If users, pass an amount greater than the creationFee as msg.value, then there is no mechanism to return the excess amount back to the user.

Vulnerability Detail

User calls publish() which is payable to publish a new work in an edition. the function then calls the invokes the FeeManager, calling the function collectCreationFee() in order to collect the creation fee for publishing the work. The collectCreationFeefunction calls thegetCreationFeefunction, which returns theasset(ETH)and theamount(protocolCreationFee). The issue here is that in a situation where the user passes a valuegreaterthan theprotocolCreationFee`, the excess amount will not be refunded as there is no mechanism for refunding the excess.

Impact

Users will be unable to get their assets back.

Code Snippet

function collectCreationFee(IEdition edition_, uint256 tokenId_, address feePayer_)
        external
        payable
    {
        Fee memory fee = getCreationFee();
        if (fee.amount == 0) return;

        _route(fee, Target({target: protocolFeeReceiver, chainId: block.chainid}), feePayer_);
        emit FeeCollected(address(edition_), tokenId_, ETH_ADDRESS, fee.amount, 0);
    }

https://github.com/sherlock-audit/2024-04-titles/blob/main/wallflower-contract-v2/src/fees/FeeManager.sol#L166

Tool used

Manual Review

Recommendation

implement the refundExcess() funtion used in the Edition.sol here so that users can automatically get their excess amount back.

function _refundExcess() internal {
        if (msg.value > 0 && address(this).balance > 0) {
            msg.sender.safeTransferETH(address(this).balance);
        }
    }