hats-finance / ether-fi-0x36c3b77853dec9c4a237a692623293223d4b9bc4

Smart Contracts for Ether Fi dapp
1 stars 1 forks source link

Owner can renounce ownership of Treasury leaving it unusable #20

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

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

Github username: @jonsey Submission hash (on-chain): 0x857092751a5e08c02cb35af394eae4e3c296dbbfc3cc52e9ae6768a7e510c8e9 Severity: medium

Description: Description\ Treasury inherits from Openzeppelin Ownable which allows the owner to renounceOwnership of the contract.

Attack Scenario\ Calling this renounceOwnership will leave the contract without an owner, preventing any further administrative operations. Specifically withdraw will not be able to be called, locking any depositied funds in the contract.

Risk level\ Likelihood - 1 Impact - 5 Overall: Medium

Attachments

  1. Proof of Concept (PoC) File

    function renounceOwnership() public virtual onlyOwner {
        _transferOwnership(address(0));
    }
  2. Revised Code File (Optional) It is recommended that the owner should not be able to renounce ownership without transfering the ownership first. The functionality can be disabled by adding the following code to the Treasury.

function renounceOwnership() public override onlyOwner {
    revert("Cannot renounce ownership");
}
seongyun-ko commented 11 months ago

We use gnosis multi signature for any txn of onlyOwner function execution, not to have this kind of scenario but TY

0xRizwan commented 11 months ago

@bunbuntigery This does not seem to be an issue as described by @seongyun-ko

Jonsey commented 11 months ago

If you search Solodit, there are many instances of this being reported and accepted as an issue. Whether it's from a multisig or single account is not really relevant. As pointed out in the report, the likely hood of this happening is low, but the ramifications are high.