Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Insufficient Validation in liboracle Contract's Price Data Check in line 60 before calling twapCircuitBreaker() #44

Closed
c4-bot-7 opened this issue Mar 27, 2024 · 11 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-164 edited-by-warden insufficient quality report This report is not of sufficient quality partial-50 Incomplete articulation of vulnerability; eligible for partial credit only (50%) 🤖_44_group AI based duplicate group recommendation

Comments

@c4-bot-7
Copy link
Contributor

c4-bot-7 commented Mar 27, 2024

Lines of code

https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/libraries/LibOracle.sol#L60

Vulnerability details

Impact

The assertion at line 60 of the liboracle contract is inaccurately stated and incomplete. The current check only reverts when the value of price is 0 which would have been ok if PRICE was uint256 due to its type being int256 it is not sufficient and timestamp at zero is not checked also . This insufficient validation can lead to potential vulnerabilities and incorrect price calculations, posing a risk to the contract's integrity and security.

Proof of Concept

Code Before Mitigation:

if (roundID == 0 || price == 0 || timeStamp > block.timestamp) revert Errors.InvalidPrice();

Using the code below are examples of others oracle validation checks also in the contract, using them as a reference

Checks used in function oracleCircuitBreaker and function baseOracleCircuitBreaker:

bool invalidFetchData = roundId == 0 || timeStamp == 0 || timeStamp > block.timestamp || chainlinkPrice <= 0
    || block.timestamp > 2 hours + timeStamp;
bool invalidFetchData = roundId == 0 || timeStamp == 0 || timeStamp > block.timestamp || chainlinkPrice <= 0
    || baseRoundId == 0 || baseTimeStamp == 0 || baseTimeStamp > block.timestamp || baseChainlinkPrice <= 0;

From the reference used, other oracles check all parameters with precisely thus we can assert that the check in line 60 is not suficient enough and

  • If both the price is negative and the timestamp is 0, the contract will not revert due to the current checks.

Alice is a user who interacts with the liboracle contract to obtain price data. She relies on the contract's price data for her financial transactions. Due to the insufficient validation in the contract's assertion, Alice may receive incorrect or manipulated price data, which could lead her to make financial decisions based on inaccurate information.

For example, if the price is less than 0 but other conditions are not met, Alice might receive a negative price, causing her to execute a transaction at an incorrect price, resulting in financial loss. Additionally, if the timeStamp is 0 due to insufficient validation, Alice may unknowingly use an unupdated or outdated or manipulated price data, leading to inaccurate financial transactions and potential financial losses.

Tools Used

Manual code analysis

Recommended Mitigation Steps

To mitigate the potential vulnerabilities and risks associated with the insufficient validation in the liboracle contract, the assertion at line 60 should be updated to include a comprehensive check for all relevant conditions affecting the price data.

Implement the following mitigation code to enhance the validation:

if (roundID == 0 || price <= 0 || timeStamp == 0 || timeStamp > block.timestamp) {
    revert Errors.InvalidPrice();
}

Assessed type

Invalid Validation

@c4-bot-7 c4-bot-7 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Mar 27, 2024
c4-bot-7 added a commit that referenced this issue Mar 27, 2024
@c4-bot-12 c4-bot-12 added the 🤖_44_group AI based duplicate group recommendation label Apr 5, 2024
@c4-pre-sort c4-pre-sort added the insufficient quality report This report is not of sufficient quality label Apr 6, 2024
@c4-pre-sort
Copy link

raymondfam marked the issue as insufficient quality report

@c4-pre-sort
Copy link

raymondfam marked the issue as duplicate of #4

@raymondfam
Copy link

See #8.

@c4-judge
Copy link
Contributor

hansfriese marked the issue as unsatisfactory:
Out of scope

@c4-judge c4-judge added the unsatisfactory does not satisfy C4 submission criteria; not eligible for awards label Apr 12, 2024
@lanrebayode
Copy link

Thank you for judging @hansfriese,

I believe this issue is not a duplicate of #4 OR #8, It speaks about a different part of the function.

@hansfriese
Copy link

Check #164 for details.

@c4-judge
Copy link
Contributor

hansfriese removed the grade

@c4-judge c4-judge removed the unsatisfactory does not satisfy C4 submission criteria; not eligible for awards label Apr 21, 2024
@c4-judge c4-judge reopened this Apr 21, 2024
@c4-judge
Copy link
Contributor

hansfriese marked the issue as not a duplicate

@c4-judge
Copy link
Contributor

hansfriese marked the issue as duplicate of #164

@c4-judge
Copy link
Contributor

hansfriese marked the issue as satisfactory

@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Apr 21, 2024
@c4-judge
Copy link
Contributor

hansfriese marked the issue as partial-50

@c4-judge c4-judge added partial-50 Incomplete articulation of vulnerability; eligible for partial credit only (50%) and removed satisfactory satisfies C4 submission criteria; eligible for awards labels Apr 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-164 edited-by-warden insufficient quality report This report is not of sufficient quality partial-50 Incomplete articulation of vulnerability; eligible for partial credit only (50%) 🤖_44_group AI based duplicate group recommendation
Projects
None yet
Development

No branches or pull requests

8 participants