code-423n4 / 2022-10-inverse-findings

0 stars 0 forks source link

Wrong logic at Fed.resign() #555

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Fed.sol#L75-L78

Vulnerability details

Impact

The Fed contract has resign function which sets the chair address to address(0). The NATSPEC states that this function is useful for immediately removing chair powers in case of a wallet compromise. Since the chair wallet is a multisig wallet, in the event of a compromised wallet for the majority of the wallet admins, it's not common that a compromised wallet removing own powers.

And if the compromised wallet is the minority, calling this function will remove all the other chair wallet admin powers as well as leave the contract chairless. This will also cause redeployment expenses and loss of reputation while the team could resolve the issue silently.

Proof of Concept

    function resign() public {
        require(msg.sender == chair, "ONLY CHAIR");
        chair = address(0);
    }

Permalink

Tools Used

Manual Review

Recommended Mitigation Steps

Instead of removing all chair wallet admin powers, there might be a function that removes the compromised admin/admins from the admin list of the wallet. The team might also consider setting the wallets 3/5 rather than 2/5 as stated on the docs.

c4-judge commented 2 years ago

0xean marked the issue as unsatisfactory: Invalid