code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-03-rolla/blob/main/quant-protocol/contracts/pricing/oracle/ProviderOracleManager.sol#L25

Vulnerability details

Impact

Missing non-zero address check for _oracle parameter in ProviderOracleManager.addAssetOracle(). In the comments for function addAssetOracle() (see here) it states "Once an oracle is added for an asset it can't be changed!".

/// @notice Add an asset to the oracle manager with its corresponding oracle address
/// @dev Once this is set for an asset, it can't be changed or removed  @audit-info <-- here
/// @param _asset the address of the asset token we are adding the oracle for
/// @param _oracle the address of the oracle

Adding a zero-address oracle would break core functionality and the oracle for an asset could not be changed again.

Proof of Concept

pricing/oracle/ProviderOracleManager.sol

Adding zero-address for parameter _oracle will break usage of protocol as it can not be reverted.

Tools Used

slither and manual review

Recommended mitigation steps

Add non-zero address check for _oracle in addAssetOracle():

 require(
    _oracle != address(0),
    "ProviderOracleManager: Oracle is zero address"
);

quantizations commented 2 years ago

Very low risk to the point was going to dispute this. Adding oracle is behind governance and even if the zero address was added then someone would have to create and mint options with that address.

alcueca commented 2 years ago

I'm not a fan of zero address checks in governance functions, but given the lasting damage to the protocol in this case is warranted as it would lead to a data integrity issue. There would be no immediate risk of lost funds even then, so downgraded to QA issue.

JeeberC4 commented 2 years ago

include with QA Report #61

code-423n4 / 2022-03-rolla-findings