Skip to content
This repository has been archived by the owner on Nov 26, 2023. It is now read-only.

BugBusters - StableOracleWETH will return the wrong price for asset if underlying aggregator hits minAnswer #589

Closed
sherlock-admin opened this issue May 24, 2023 · 5 comments
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Escalation Resolved This issue's escalations have been approved/rejected Medium A valid Medium severity issue Reward A payout will be made for this issue

Comments

@sherlock-admin
Copy link
Contributor

sherlock-admin commented May 24, 2023

BugBusters

medium

StableOracleWETH will return the wrong price for asset if underlying aggregator hits minAnswer

Summary

Chainlink aggregators have a built in circuit breaker if the price of an asset goes outside of a predetermined price band. The result is that if an asset experiences a huge drop in value (i.e. LUNA crash) the price of the oracle will continue to return the minPrice instead of the actual price of the asset. This would allow user to continue borrowing with the asset but at the wrong price. This is exactly what happened to Venus on BSC when LUNA imploded.

Vulnerability Detail

StableOracleWETH uses the ChainlinkFeedRegistry to obtain the price of the requested tokens.

ChainlinkFeedRegistry#latestRoundData pulls the associated aggregator and requests round data from it. ChainlinkAggregators have minPrice and maxPrice circuit breakers built into them. This means that if the price of the asset drops below the minPrice, the protocol will continue to value the token at minPrice instead of it's actual value.

https://github.com/sherlock-audit/2023-05-USSD/blob/6d7a9fdfb1f1ed838632c25b6e1b01748d0bafda/ussd-contracts/contracts/oracles/StableOracleDAI.sol#L48

Example:
TokenA has a minPrice of $1. The price of TokenA drops to $0.10. The aggregator still returns $1 which is 10x it's actual value.

Note:
Chainlink oracles are used a just one piece of the OracleAggregator system and it is assumed that using a combination of other oracles, a scenario like this can be avoided. However this is not the case because the other oracles also have their flaws that can still allow this to be exploited. As an example if the chainlink oracle is being used with a UniswapV3Oracle which uses a long TWAP then this will be exploitable when the TWAP is near the minPrice on the way down. In a scenario like that it wouldn't matter what the third oracle was because it would be bypassed with the two matching oracles prices. If secondary oracles like Band are used a malicious user could DDOS relayers to prevent update pricing. Once the price becomes stale the chainlink oracle would be the only oracle left and it's price would be used.

Impact

In the event that an asset crashes (i.e. LUNA) the protocol can be manipulated to give out loans at an inflated price

Code Snippet

Tool used

Manual Review

Recommendation

StableOracleWETH should check the returned answer against the minPrice/maxPrice and revert if the answer is outside of the bounds:

Duplicate of #598

@github-actions github-actions bot closed this as completed Jun 5, 2023
@github-actions github-actions bot added Medium A valid Medium severity issue Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label labels Jun 5, 2023
@sherlock-admin sherlock-admin added the Reward A payout will be made for this issue label Jun 23, 2023
@Nabeel-javaid
Copy link

Escalate for 10 USDC

This is not a duplicate of #31 as it talks about price hitting minAnswer, but not stale price.

This escalation fully describes why issue is different from stale price
sherlock-audit/2023-02-blueberry-judging#18 (comment)

@sherlock-admin
Copy link
Contributor Author

Escalate for 10 USDC

This is not a duplicate of #31 as it talks about price hitting minAnswer, but not stale price.

This escalation fully describes why issue is different from stale price
sherlock-audit/2023-02-blueberry-judging#18 (comment)

You've created a valid escalation for 10 USDC!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

@sherlock-admin sherlock-admin added the Escalated This issue contains a pending escalation label Jun 24, 2023
@ctf-sec
Copy link
Collaborator

ctf-sec commented Jul 4, 2023

See comments on #197

@hrishibhat
Copy link
Contributor

Result:
Medium
Duplicate of #598

@sherlock-admin sherlock-admin removed the Escalated This issue contains a pending escalation label Jul 14, 2023
@sherlock-admin
Copy link
Contributor Author

Escalations have been resolved successfully!

Escalation status:

@sherlock-admin sherlock-admin added the Escalation Resolved This issue's escalations have been approved/rejected label Jul 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Escalation Resolved This issue's escalations have been approved/rejected Medium A valid Medium severity issue Reward A payout will be made for this issue
Projects
None yet
Development

No branches or pull requests

4 participants