code-423n4 / 2023-07-moonwell-findings

1 stars 0 forks source link

Single-step process for ownership transfer #285

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-07-moonwell/blob/main/src/core/Oracles/ChainlinkOracle.sol#L173

Vulnerability details

Impact

The accidental usage of address in the setAdmin (lines 175-181 in ChainlinkOracle.sol) in ChainlinkOracle.sol function would prevent the use of onlyAdmin() in the functions setUnderlyingPrice, setDirectPrice and setFeed. This would lead to a critical impact on the system if any of the functions setDirectPrice or setUnderlyingPrice must be executed at times of critical economical events or oracle malfunction, or a particular Chainlink feed address becomes obsolete or malfunctions.

    function setAdmin(address newAdmin) external onlyAdmin {
        address oldAdmin = admin;
        admin = newAdmin;

        emit NewAdmin(oldAdmin, newAdmin);
    }

Proof of Concept

Having a functions like setUnderlyingPrice and setDirectPrice supposes a critical economical and technical events to be mitigated with manually setting the price of an assets by the admin. Consequently setting the admin must executed with great precision with the help of a two-step battle-tested process.

Tools Used

Manual VS code

Recommended Mitigation Steps

Integrate two-step transferring ownership process.

Assessed type

Governance

0xSorryNotSorry commented 1 year ago

OOS --> [L‑16] Consider implementing two-step procedure for updating protocol addresses

c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as low quality report

c4-judge commented 1 year ago

alcueca marked the issue as unsatisfactory: Invalid