sherlock-audit / 2024-04-titles-judging

9 stars 6 forks source link

nisedo - `Edition._refundExcess()` allows a malicious user to steal the entire Edition contract balance by only paying the minimum fee #255

Closed sherlock-admin4 closed 4 months ago

sherlock-admin4 commented 4 months ago

nisedo

high

Edition._refundExcess() allows a malicious user to steal the entire Edition contract balance by only paying the minimum fee

Summary

Edition._refundExcess() allows for stealing the entire balance by only paying for the minimum fee.

Vulnerability Detail

Edition._refundExcess() should be used only to "Refund any excess ETH sent to the contract".

However, a malicious user can monitor the contract balance, and call Edition.mint(), minting a fee-free token, every time the contract balance is greater than the minimum minting fee, allowing to steal the excess balance.

    /// @notice Mint a new token for the given work.
    /// @param to_ The address to mint the token to.
    /// @param tokenId_ The ID of the work to mint.
    /// @param amount_ The amount of tokens to mint.
    /// @param referrer_ The address of the referrer.
    /// @param data_ The data associated with the mint. Reserved for future use.
    function mint(
        address to_,
        uint256 tokenId_,
        uint256 amount_,
        address referrer_,
        bytes calldata data_
    ) external payable override {
        FEE_MANAGER.collectMintFee{value: msg.value}(
            this, tokenId_, amount_, msg.sender, referrer_, works[tokenId_].strategy
        );

        _issue(to_, tokenId_, amount_, data_);
❌      _refundExcess();
    }
    /// @notice Refund any excess ETH sent to the contract.
    /// @dev This function is called after minting tokens to refund any ETH left in the contract after all fees have been collected.
    function _refundExcess() internal {
        if (msg.value > 0 && address(this).balance > 0) {
❌          msg.sender.safeTransferETH(address(this).balance);
        }
    }

This attack is particularly feasible on Layer 2 (L2) blockchains where gas fees are low, making frequent monitoring and transactions economically viable.

Scenario

  1. Increased Contract Balance: Legitimate users or admins perform transactions that include a higher msg.value than required for operations like TitlesCore.createEdition(), TitlesCore.publish(), Edition.grantRoles(), or Edition.revokeRoles(). This increases the ether balance of the Edition contract, for instance, up to 0.1 ether.

  2. Minimum Protocol Fee: The minimum protocol fee required to mint a token is 0.0006 ether for tokens with mintFee = 0.

  3. Exploitation by Malicious User: A malicious user, let's call him Bob, continuously monitors the Edition contract's ether balance. Whenever the balance exceeds 0.0006 ether, Bob initiates a transaction to Edition.mint() with exactly 0.0006 ether. Because of how _refundExcess() is implemented, he not only mints the token but also receives the excess ether above 0.0006 ether as a refund.

Impact

Theft of funds.

Code Snippet

Tool used

Manual Review

Recommendation

Remove the payable modifier of Edition.grantRoles() and Edition.revokeRoles() and add a call to Edition._refundExcess() at the end of TitlesCore.createEdition() and TitlesCore.publish() to ensure that the Edition contract balance never accumulates.