code-423n4 / 2024-05-predy-validation

0 stars 0 forks source link

Missing access control in external `AddPairLogic::updatePriceOracle` function #615

Closed c4-bot-2 closed 3 months ago

c4-bot-2 commented 3 months ago

Lines of code

https://github.com/code-423n4/2024-05-predy/blob/a9246db5f874a91fb71c296aac6a66902289306a/src/libraries/logic/AddPairLogic.sol#L112

Vulnerability details

Impact

Allowing anyone to update the price oracle can lead to severe security vulnerabilities, including:

  1. Price Manipulation: An attacker could set the price feed to a malicious or incorrect oracle, resulting in incorrect pricing information being used in the system. This could be exploited to manipulate asset prices for personal gain.

  2. Financial Loss: Incorrect prices can cause mispriced trades, improper liquidations, or inaccurate valuations of assets, leading to significant financial losses for users. Users may buy or sell assets at unfair prices, or face liquidations due to manipulated price feeds, directly causing them to lose funds.

  3. System Integrity: The integrity and reliability of the entire protocol can be compromised, as the price oracle is a critical component for many decentralized finance (DeFi) applications.

Given these potential consequences, the lack of access control on this function poses a high risk to the protocol and its users.

Proof of Concept


function exploitUpdatePriceOracle(DataType.PairStatus storage _pairStatus) external {
    address maliciousOracle = 0x...; // Attacker's malicious oracle address
    _pairStatus.priceFeed = maliciousOracle;
    emit PriceOracleUpdated(_pairStatus.id, maliciousOracle);
}

Tools Used

none

Recommended Mitigation Steps

1 . Add struct :


    // Define access control storage layout
    struct AccessControl {
        address operator;
        mapping(uint256 => address) poolOwners;
    }

2 . Add modifier :


    modifier onlyPoolOwner(AccessControl storage accessControl, uint256 pairId) {
        require(msg.sender == accessControl.poolOwners[pairId], "Caller is not the pool owner");
        _;
    }

3 . included to the function :


function updatePriceOracle(
    DataType.PairStatus storage _pairStatus,
    address _priceOracle,
    AccessControl storage accessControl
) external onlyPoolOwner(accessControl, _pairStatus.id) {
    _pairStatus.priceFeed = _priceOracle;
    emit PriceOracleUpdated(_pairStatus.id, _priceOracle);
}

Assessed type

Access Control