027.md

Best Ceramic Yak

High

MasterPriceOracle Lacks Proper Proxy Implementation

Summary

The MasterPriceOracle.sol contract was intended to be deployed as a proxy but lacks the required inheritance from UUPSUpgradeable. This design flaw prevents the contract from functioning as a UUPS proxy, potentially leading to an inability to upgrade the contract and leaving the protocol in a non-upgradable state. This flaw can result in severe operational limitations and security risks for the protocol and its users.

Root Cause

The contract does not inherit from UUPSUpgradeable, which is required for implementing a UUPS proxy. Additionally, the constructor is used for initialization, which is incompatible with proxy deployments.

contract MasterPriceOracle is Initializable, BasePriceOracle { 
    constructor(address[] memory underlyings, address[] memory _oracles) {
        // Initialization logic here
    }
}

Internal pre-conditions

1. The development team deploys MasterPriceOracle with the intention of using it as a UUPS proxy.
2. The contract’s constructor is used to initialize state variables instead of the initializer modifier, which is required for proxy patterns.

External pre-conditions

1. A UUPS-compatible proxy contract is deployed to upgrade the MasterPriceOracle.
2. The protocol attempts to use upgrade functionality, which will fail due to the missing UUPSUpgradeable inheritance and initializer.

Attack Path

1. The protocol deploys MasterPriceOracle assuming it is compatible with the UUPS proxy pattern.
2. At a later stage, the protocol attempts to upgrade the contract to add new functionality or fix vulnerabilities.
3. The upgrade fails due to the missing UUPSUpgradeable inheritance and proxy-compatible implementation, leaving the protocol locked into the current state of the contract.

Impact

• Operational Impact: The protocol cannot upgrade the MasterPriceOracle contract, leaving it unable to adapt to new requirements or fix critical vulnerabilities.
• Security Risks: If vulnerabilities are discovered in the deployed contract, the inability to upgrade leaves the protocol exposed to attacks.
• Reputational and Financial Damage: Failure to implement proxy compatibility could undermine user confidence and result in financial losses due to stalled operations or security breaches.

PoC

No response

Mitigation

To address this issue, the MasterPriceOracle contract must be updated to inherit from UUPSUpgradeable and use an initializer function for state variable initialization. The revised implementation should resemble the following:

+ import "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";
+ import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";

- contract MasterPriceOracle is Initializable, BasePriceOracle {
+ contract MasterPriceOracle is Initializable, IBasePriceOracle, UUPSUpgradeable, OwnableUpgradeable {


-    constructor(
-        address[] memory underlyings,
-        address[] memory _oracles
-    ) {

+    function initialize(
+        address[] memory underlyings,
+        address[] memory _oracles
+    ) public initializer {

+        // Initialize the owner of the contract
+        __Ownable_init(msg.sender);
+        // Initialize the proxy
+        __UUPSUpgradeable_init();

        // Get the total number of collateral addresses
        uint16 noOfUnderlyings = uint16(underlyings.length);

        // Check the number of pricefeed addresses and collateral addresses are same
        if (noOfUnderlyings != _oracles.length)
            revert Oracle_CollateralAddressesAndPriceFeedIdsMustBeSameLength();

        // Loop through the number of collateral address
        for (uint256 i = 0; i < noOfUnderlyings; i++) {
            // Assign the value(pricefeed address) for a key(collateral address)
            oracles[underlyings[i]] = _oracles[i];
            // Assign the value(collateral address) for a key(collateral name ENUM)
            assetAddress[IBorrowing.AssetName(i + 1)] = underlyings[i];
        }
    }




+    function _authorizeUpgrade(
+        address newImplementation
+    ) internal override onlyOwner{}

}

also better to rename the BasePriceOracle to IBasePriceOracle

Blockchian\contracts\oracles\BasePriceOracle.sol -> Blockchian\contracts\oracles\IBasePriceOracle.sol

// SPDX-License-Identifier: MIT
pragma solidity 0.8.22;

- interface BasePriceOracle {
+ interface IBasePriceOracle {
  error Oracle_CollateralAddressesAndPriceFeedIdsMustBeSameLength();

  function price(address underlying) external view returns (uint128, uint128);
}