code-423n4 / 2024-04-renzo-validation

2 stars 2 forks source link

Title: Lack of Multisig Protection and Timelock for `ORACLE_ADMIN` Role #7

Closed c4-bot-10 closed 5 months ago

c4-bot-10 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/1c7cc4e632564349b204b4b5e5f494c9b0bc631d/contracts/Permissions/RoleManager.sol#L51

Vulnerability details

Impact

The absence of multisig protection and a timelock mechanism for the ORACLE_ADMIN role poses significant risks, including the potential for price feed manipulation, rapid and unpredictable changes to price feeds, loss of trust and participation, and difficulty in recovery from errors or malicious actions.

Proof of Concept

The RoleManager contract in the Renzo protocol lacks multisig protection and a timelock mechanism for the ORACLE_ADMIN role at RoleManager::isOracleAdmin function. This role is critical for setting price feed oracles for ERC20 tokens, which are fundamental to the protocol's operations, including trading, liquidity provision, and other financial activities.

The issue was identified in the RoleManager contract, specifically at this line: RoleManager.sol#L51. This line of code does not include any multisig protection or timelock mechanism for the ORACLE_ADMIN role, which allows the admin to set price feed oracles for ERC20 tokens without any checks or delays.

    function isOracleAdmin(address potentialAddress) external view returns (bool) {
@>        return hasRole(ORACLE_ADMIN, potentialAddress);
    }

A scenario illustrating the impact could be a situation where a malicious ORACLE_ADMIN manipulates the price feed for a popular ERC20 token, leading to incorrect pricing information. This manipulation could disrupt trading, affect liquidity provision, and potentially lead to financial losses for users and participants in the protocol.

Tools Used

Manual review of the contract code using VSCode.

Recommended Mitigation Steps

To mitigate the risk of a single compromised or malicious ORACLE_ADMIN, implement multisig protection. This requires defining a new role, such as MULTISIG_APPROVER_ROLE, and modifying the contract to require approval from multiple addresses before allowing changes to the ORACLE_ADMIN role.

To prevent rapid and unchecked changes to the ORACLE_ADMIN role, implement a timelock mechanism. This requires integrating with an external timelock contract that allows scheduling actions to be executed after a certain delay.

Assessed type

Access Control

0xJuancito commented 5 months ago

@howlbot reject

0xJuancito commented 5 months ago

Known Risks: Administrative centralization (protocol is becoming decentralized over time)