sherlock-audit / 2024-04-titles-judging

1 stars 1 forks source link

BengalCatBalu - Malicious minter can steal all funds from Edition.sol due to a lack of data validation in the _collectMintFee function #365

Closed sherlock-admin4 closed 2 months ago

sherlock-admin4 commented 2 months ago

BengalCatBalu

medium

Malicious minter can steal all funds from Edition.sol due to a lack of data validation in the _collectMintFee function

Summary

The lack of data validation in the FeeManager::calculateMintFee and FeeManager::_calculateMintFee functions has been described in detail in this issue issue Title: There is no validation on the FeeManager::collectMintFee function, anyone can call it Here I would like to focus on a specific edge case, when address of edition is specified as the referer_

Note that at the end of every mint transaction in Edition.sol there is a call to the function Edition.sol::_refundExcess. This function transfers the remaining balance to the address of the mint that called the function. Thus, if funds arrive at the edition address before this function is called, the balance of Edition.sol will not be zero and we will be able to withdraw all funds from this contract.

Vulnerability Detail

I have described the process of paying commissions in minutes in detail in the other two issues first second first Title - In the FeeManager.sol::_splitProtocolFee function, the collectionReffererShare recipient is misspelled. second Title - There is no validation on the FeeManager::collectMintFee function, anyone can call it It can be seen from them that the specified refferer is not validated in any way and receives 50% of the protocolFee The refundExcess function is executed if msg.value > 0 && address(this).balance > 0. That is, if we apply even a very small amount of money when calling the mint function, and use an edge case to point to the refferer address edition, we will take the entire contract balance.

Impact

Edition.sol contract implementations do not imply that it will hold funds longer than as part of a mint transaction. It also does not have receive(). However, it can still receive eth by selfdestruct(). This eth can then be output by the attacker. Score: medium

Code Snippet

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

Tool used

Manual Review

Recommendation

Add appropriate validation to eliminate this edge case