sherlock-audit / 2024-04-titles-judging

1 stars 1 forks source link

ZdravkoHr. - `Edition._refundExcess()` will not refund user funds #318

Closed sherlock-admin3 closed 2 months ago

sherlock-admin3 commented 2 months ago

ZdravkoHr.

medium

Edition._refundExcess() will not refund user funds

Summary

Edition._refundExcess() must refund any amount not used up by the minting fees to the caller. It does not work because the whole msg.value is being forwarded to the FeeManager contract and there is nothing to be refunded.

Vulnerability Detail

All functions that mint with fee - mint(), mintWithComment() and mintBatch - send the whole msg.value to the FeeManager contract and then call _refundExcess assuming the excess will be in the current contract, but it will actually be in the FeeManager contract.

    function mint(
        address to_,
        uint256 tokenId_,
        uint256 amount_,
        address referrer_,
        bytes calldata data_
    ) external payable override {
        // wake-disable-next-line reentrancy
        FEE_MANAGER.collectMintFee{value: msg.value}(
            this, tokenId_, amount_, msg.sender, referrer_, works[tokenId_].strategy
        );

        _issue(to_, tokenId_, amount_, data_);
        _refundExcess();
    }

Impact

Refunding functionality does not work, even though it should be supported. Users will lose their funds.

Code Snippet

https://github.com/sherlock-audit/2024-04-titles/blob/d7f60952df22da00b772db5d3a8272a988546089/wallflower-contract-v2/src/editions/Edition.sol#L510-L517

Tool used

Manual Review

Recommendation

Either add a refund mechanism to FeeManager as well or don't send the whole msg.value, but just a portion of it.

Duplicate of #269