sherlock-audit / 2023-12-ubiquity-judging

2 stars 2 forks source link

rvdemonk - updateChainLinkCollateralPrice() can return wrong price if underlying aggregator hits minAnswer or maxAnswer #168

Closed sherlock-admin2 closed 9 months ago

sherlock-admin2 commented 9 months ago

rvdemonk

medium

updateChainLinkCollateralPrice() can return wrong price if underlying aggregator hits minAnswer or maxAnswer

rvdemonk

medium

Summary

The updateChainLinkCollateralPrice function within the protocol fetches the latest price for a collateral token from a Chainlink V3 Aggregator and updates it in the UbiquityPoolStorage. It is assumed for this analysis that Chainlink Aggregators incorporate a circuit breaker mechanism that caps the reported price to a predefined upper or lower bound if the actual market price deviates significantly from these bounds. This mechanism, while designed to prevent extreme market manipulation, could potentially be exploited under certain conditions.

Vulnerability Detail

If the actual price of the collateral token goes beyond the permitted bounds set by Chainlink, the oracle would return the capped price (either the maximum or minimum bound) instead of the real market price. This discrepancy between the oracle-reported price and the actual market price could lead to inaccurate valuation of collateral within the protocol. Currently, the price feed answer is only checked to be positive and not stale.

Impact

If the price of the collateral asset is unstable and becomes depegged (as most planned collateral assets seem to be USD pegged stablecoins) the protocol can be manipulated and extorted

Code Snippet

https://github.com/sherlock-audit/2023-12-ubiquity/blob/main/ubiquity-dollar/packages/contracts/src/dollar/libraries/LibUbiquityPool.sol#L523C2-L553C1

Tool used

Manual Review

Recommendation

Insert a check into updateChainLinkCollateralPrice() to ensure the returned price answer is within accepted bounds, bounds that can be set by the admin.

Duplicate of #144

sherlock-admin2 commented 9 months ago

1 comment(s) were left on this issue during the judging contest.

auditsea commented:

The issue describes about not checking min/max value from Chainlink for data staleness, but this is not required

sherlock-admin2 commented 9 months ago

1 comment(s) were left on this issue during the judging contest.

auditsea commented:

The issue describes about not checking min/max value from Chainlink for data staleness, but this is not required