hats-finance / Fenix--0x9d7765a7ebd5b6322a30797a44a5428531970d3d

0 stars 1 forks source link

Module's gas yield can never be claimed and all yield will be lost #34

Open hats-bug-reporter[bot] opened 2 months ago

hats-bug-reporter[bot] commented 2 months ago

Github username: @https://github.com/cholakovvv Twitter username: https://x.com/cholakovv Submission hash (on-chain): 0x2ffc92fd17e8ae60669f5b5ec4ee69c4556191664533c8ded95eb9078e9444c7 Severity: high

Description: Description\

This protocol is designed for deployment on the Blast network. Blast allows gas and Ether balances to accrue yield.

By default, the yield settings for both ETH and gas are set to "VOID." This means that unless we explicitly configure the yield mode to "claimable," we will not be able to receive any yield. Notably, the protocol never sets gas to claimable for the modules, and the governor contract (blastGovernor_) also lacks a function to enable this functionality.

Attack Scenario\

The __BaseManagedNFTStrategy__init() in BaseManagedNFTStrategyUpgradeable.sol set the blastGovernor_ to the __BlastGovernorClaimableSetup_init() here:

function __BaseManagedNFTStrategy__init(
        address blastGovernor_,
        address managedNFTManager_,
        string memory name_
    ) internal onlyInitializing {
        __BlastGovernorClaimableSetup_init(blastGovernor_);

Examining this contract, we can see the following code in BlastGovernorSetup.sol:

 function __BlastGovernorSetup_init(address gov_) internal {
        if (gov_ == address(0)) {
            revert AddressZero();
        }
        IBlast(0x4300000000000000000000000000000000000002).configureGovernor(gov_);
    }

As we can see , the blastGovernor_ is set, but we never set gas to claimable. Gas yield mode will be in its default mode which is VOID, the modules will not accue gas yields. Since these modules never set gas yield mode to claimable, the blastGovernor_ cannot claim any gas yield for either of the contracts.

Attachments

  1. Proof of Concept (PoC) File

BaseManagedNFTStrategyUpgradeable.sol:

function __BaseManagedNFTStrategy__init(
        address blastGovernor_,
        address managedNFTManager_,
        string memory name_
    ) internal onlyInitializing {
        __BlastGovernorClaimableSetup_init(blastGovernor_);

        _checkAddressZero(managedNFTManager_);

        managedNFTManager = managedNFTManager_;
        votingEscrow = IManagedNFTManager(managedNFTManager_).votingEscrow();
        voter = IManagedNFTManager(managedNFTManager_).voter();
        name = name_;
    }

BlastGovernorSetup.sol:

abstract contract BlastGovernorSetup {
    /// @dev Error thrown when an operation involves a zero address where a valid address is required.
    error AddressZero();

    /**
     * @dev Initializes the governor configuration for the Blast protocol.
     * This internal function is meant to be called in the initialization process
     * of a derived contract that sets up governance.
     *
     * @param gov_ The address of the governor to be configured in the Blast protocol.
     * Must be a non-zero address.
     */
    function __BlastGovernorSetup_init(address gov_) internal {
        if (gov_ == address(0)) {
            revert AddressZero();
        }
        IBlast(0x4300000000000000000000000000000000000002).configureGovernor(gov_);
    }
}
  1. Revised Code File (Optional)

Change the following in BlastGovernorSetup and IBlast contracts, this will set the gas yield of the modules to claimable in the constructor and allowing the auction house to claim gas yield:

BlastGovernorSetup.sol:

abstract contract BlastGovernorSetup {
    /// @dev Error thrown when an operation involves a zero address where a valid address is required.
    error AddressZero();

    /**
     * @dev Initializes the governor configuration for the Blast protocol.
     * This internal function is meant to be called in the initialization process
     * of a derived contract that sets up governance.
     *
     * @param gov_ The address of the governor to be configured in the Blast protocol.
     * Must be a non-zero address.
     */
    function __BlastGovernorSetup_init(address gov_) internal {
        if (gov_ == address(0)) {
            revert AddressZero();
        }

+ IBlast(0x4300000000000000000000000000000000000002).configureClaimableGas();
IBlast(0x4300000000000000000000000000000000000002).configureGovernor(gov_);
    }
}

Iblast.sol:

interface IBlast {
    function configureGovernor(address _governor) external;
+ function configureClaimableGas() external; 
}
ArnieSec commented 2 months ago

Duplicate #20