hats-finance / Inverter-Network-0xe47e52c4fea05e555920f1dcdcc6fb8eca103eeb

Fork of the Inverter Smart Contracts Repository
GNU Lesser General Public License v3.0
0 stars 3 forks source link

Governor_v1.sol - Once the contract becomes an owner of another contract, he can't `transferOwnership` to another address #129

Open hats-bug-reporter[bot] opened 1 month ago

hats-bug-reporter[bot] commented 1 month ago

Github username: -- Twitter username: @EgisSec Submission hash (on-chain): 0x02e7a6dc883d4ca52bbc0eaa0f6a7929cb18cb3592218668abcbd5d48ec41e42 Severity: medium

Description: Description\ Governor_v1 implements a function called acceptOwnership. The function calls Ownable2Step.acceptOwnership on the adr to accept the ownership of the contract and become the new owner.

function acceptOwnership(address adr)
        external
        onlyCommunityOrTeamMultisig
    {
        if (adr.code.length == 0) {
            revert Governor__CallToTargetContractFailed();
        }

        (bool success,) =
            adr.call(abi.encodeCall(Ownable2Step.acceptOwnership, ()));

        // if the call is not a success
        if (!success) {
            revert Governor__CallToTargetContractFailed();
        }
        emit OwnershipAccepted(adr);
    }

One contract that the Governor_v1 is built aroun to be the owner is FeeManager_v1. Governor_v1 has multiple functions that control the fees and the treasuries of the FeeManager_v1.

The issue here is that, after Governor_v1 accepts ownership, he can't transfer it to another address as the contract doesn't implement a function that calls Ownable2Step.transferOwnershipon a specified address.

If the protocol wants to change ownership to an EOA for example, they have to deploy a new fee manager with the new owner.

Attack Scenario\

Attachments

  1. Proof of Concept (PoC) File

  2. Revised Code File (Optional)

Add a function that calls Ownable2Step.transferOwnership on a target address.

FHieser commented 1 month ago

For now we didnt intend to switch the ownership over contracts from the governor side. The acceptance of ownership was in case we would have to bring in another beacon contract