search

sherlock-audit / 2023-12-ubiquity-judging

2 stars 2 forks source link

evmboi32 - The same (incorrect) `heartbeat` is used for multiple price feeds. #140

Closed sherlock-admin2 closed 7 months ago

sherlock-admin2 commented 8 months ago

evmboi32

high

The same (incorrect) heartbeat is used for multiple price feeds.

Summary

The same (incorrect) heartbeat is used for multiple price feeds.

Vulnerability Detail

The collateralPriceFeedStalenessThresholds is set to 24 hours for every collateral by default.

poolStorage.collateralPriceFeedStalenessThresholds.push(1 days);

The problem is that different pairs have different heartbeats. For example, the LUSD/USD price should be stale after not being updated for 3600s, but since the heartbeat is set to 24 hours it will consider a price valid even if it wasn't updated for up to 24 hours which is incorrect.

require(
    block.timestamp - updatedAt <
        poolStorage.collateralPriceFeedStalenessThresholds[
            collateralIndex
        ],
    "Stale data"
);

Impact

Using the same heartbeat for all price feeds is not correct because the freshness validation would be useless for some pairs which can return stale data.

Code Snippet

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

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

Tool used

Manual Review

Recommendation

Use the heartbeat values from the official chainlink docs for each price feed separately https://docs.chain.link/data-feeds/price-feeds/addresses?network=ethereum&page=1&search=lusd

Duplicate of #130

sherlock-admin2 commented 7 months ago

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

auditsea commented:

This issue describes about retreiving zero index by default when collateralAddress is unknown, but the function is called by admin so no need to be considered

sherlock-admin2 commented 7 months ago

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

auditsea commented:

This issue describes about retreiving zero index by default when collateralAddress is unknown, but the function is called by admin so no need to be considered