sherlock-audit / 2024-04-titles-judging

1 stars 1 forks source link

maushish - `_refundExcess()` current implementation could get transactions reverted. #362

Closed sherlock-admin3 closed 2 months ago

sherlock-admin3 commented 2 months ago

maushish

high

_refundExcess() current implementation could get transactions reverted.

Summary

The current implementation of the _refundExcess method could make some users spend more funds in the minting process while others can get more funds.

Vulnerability Detail

In the current implementaion we are using address(this).balance there could be a scenarios when gas fees is more than the current contract's balance especially on the ETH MAINNET, due to which this function will simply revert the txn. https://github.com/sherlock-audit/2024-04-titles/blob/main/wallflower-contract-v2/src/editions/Edition.sol#L512

    /// @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);
    }
}

Impact

Some user can lose funds while other user can get more funds in return.

Code Snippet

Consider these Scenarios Scenario A:

Scenario B:

Manual Review

Recommendation

Implement a new mechanism that emits an event whenever user excess funds are stuck in the protocol so that later either ADMIN or some new role can refund this and also have an internal accounting system for such excess funds in the Edition contract.