Add detectors for incorrect use of Oracle API #2289
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Improve checks and detection
New features and checks
|
Hello people, From my side, the PR is ready for review. The checks were improved, and the code was also tested on several contracts to improve overall detection and variations of Oracle usage. So far, the detection is supporting just Chainlink. The capabilities of detection of other providers can be extended in the future 😃 I am looking forward to hearing your feedback. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
slither/detectors/oracles/supported_oracles/chainlink_oracle.py
Outdated
Show resolved
Hide resolved
slither/detectors/oracles/supported_oracles/chainlink_oracle.py
Outdated
Show resolved
Hide resolved
...tors/snapshots/detectors__detector_OracleDataCheck_0_8_20_oracle_timestamp_in_var_sol__0.txt
Show resolved
Hide resolved
...snapshots/detectors__detector_OracleDataCheck_0_8_20_oracle_check_out_of_function_sol__0.txt
Show resolved
Hide resolved
...snapshots/detectors__detector_OracleDataCheck_0_8_20_oracle_check_out_of_function_sol__0.txt
Show resolved
Hide resolved
tests/e2e/detectors/test_data/oracle-data-validation/0.8.20/oracle_timestamp_in_var.sol
Outdated
Show resolved
Hide resolved
tests/e2e/detectors/test_data/oracle-data-validation/0.8.20/oracle_check_out_of_function.sol
Show resolved
Hide resolved
...detectors/test_data/oracle-data-validation/0.8.20/oracle_data_check_price_in_internal_fc.sol
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
Out of diff range and nitpick comments (1)
slither/detectors/oracles/supported_oracles/chainlink_oracle.py (1)
27-113: ClassChainlinkOracleis well-structured. Consider adding more comments to complex methods to improve readability and maintainability.
...rs/test_data/oracle-data-validation/0.8.20/oracle_data_check_price_in_double_internal_fc.sol
Show resolved
Hide resolved
...rs/test_data/oracle-data-validation/0.8.20/oracle_data_check_price_in_double_internal_fc.sol
Show resolved
Hide resolved
slither/detectors/oracles/supported_oracles/chainlink_oracle.py
Outdated
Show resolved
Hide resolved
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I am creating this pull request based on the following issue: #2283.
The code here presents basic foundations for detecting improper usage of Oracle API's. So far, the detector supports just Chainlink, but I plan to add other providers such as Tellor or Chronicle.
The PR contains three detectors:
oracle-data-validation- Staleness price, check for price != 0deprecated-chainlink-call- Check if the smart contract uses deprecated chainlink API functionsoracle-sequencer- Recommendation to the user to check the liveness of sequence if planning to use chainlink oracle on L2.The checks that indicate improper data validation, such as staleness price, are still under development. I will improve these checks and overall detection in the following weeks.
The reason behind this PR is to get feedback for the approach I have chosen to detect an oracle and if the checks for data validation make sense from your side.
I have one question: Should support for other oracles and improvements of checks be included in different pull requests, or should I add it to this one?
Talfao
Summary by CodeRabbit
New Features
actions/setup-nodeaction in the workflow file.Dockerfile.README.md.Tests
OracleDataCheckandDeprecatedChainlinkCalldetectors using various Solidity files.