Spot Price detector

[talfao]

Hello everyone,

This detector is continuing the work of my thesis for possible vulnerabilities regarding the utilization of oracles in smart contracts. (related to another PR #2289).

A detector is named an oracle spot price detector (oracle-spot-price). It checks the following methods for indicating spot price data in connection with Uniswap Liquidity pools:

• getReserves
• slot0
• balanceOf pattern -> certain patterns could indicate a functionality to obtain price data.

Then it detects arithmetic operations or functions which could indicate a spot price calculation.

I am looking forward to any feedback and am willing to improve the code based on it.

Talfao

Summary by CodeRabbit

• New Features

  • Introduced SpotPriceDetector to enhance detection of spot price usage in smart contracts, including specific oracle calls and Uniswap interactions.

• Tests

  • Added extensive tests for the new SpotPriceDetector across various scenarios and Solidity files to ensure robust functionality.

[coderabbitai]

Walkthrough

Walkthrough

The updates involve the introduction of a new SpotPriceDetector in Slither's detection suite, aimed at identifying the use of spot prices in smart contracts. Changes include the addition of this detector, updates to the conversion functions in Slither, and new test cases to ensure the functionality of the newly added features.

Changes

File Path	Change Summary
slither/detectors/oracles/spot_price.py	Introduced SpotPriceDetector for detecting spot price usage in contracts.
slither/detectors/all_detectors.py	Added import of SpotPriceDetector.
slither/slithir/convert.py	Added new conditional block in convert_to_library_or_top_level function.
tests/e2e/detectors/.../detectors__detector_SpotPriceDetector_...	New tests for detecting spot price usage in Uniswap V2 contracts and other scenarios.
tests/e2e/detectors/test_data/oracle-spot-price/...	Introduced new Solidity smart contracts for testing spot price related functionalities.
tests/e2e/detectors/test_detectors.py	Added tests for SpotPriceDetector with various Solidity files and versions.

Possibly related issues

• Detectors related to improper usage of Oracle APIs #2283: This issue's focus on developing detectors for identifying improper usage of Oracle APIs in smart contracts aligns with the introduction of SpotPriceDetector, which could be part of a broader initiative to enhance security by scrutinizing Oracle data usage.

---

Recent Review Details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between ded705d and f2c9409.

Files ignored due to path filters (3)

• tests/e2e/detectors/test_data/oracle-spot-price/0.6.12/spot_price_getReserves.sol-0.6.12.zip is excluded by !**/*.zip
• tests/e2e/detectors/test_data/oracle-spot-price/0.8.20/spot_price_balanceOf.sol-0.8.20.zip is excluded by !**/*.zip
• tests/e2e/detectors/test_data/oracle-spot-price/0.8.20/spot_price_getAmountOut.sol-0.8.20.zip is excluded by !**/*.zip

Files selected for processing (10)

• slither/detectors/all_detectors.py (1 hunks)
• slither/detectors/oracles/spot_price.py (1 hunks)
• slither/slithir/convert.py (1 hunks)
• tests/e2e/detectors/snapshots/detectors__detector_SpotPriceDetector_0_6_12_spot_price_getReserves_sol__0.txt (1 hunks)
• tests/e2e/detectors/snapshots/detectors__detector_SpotPriceDetector_0_8_20_spot_price_balanceOf_sol__0.txt (1 hunks)
• tests/e2e/detectors/snapshots/detectors__detector_SpotPriceDetector_0_8_20_spot_price_getAmountOut_sol__0.txt (1 hunks)
• tests/e2e/detectors/test_data/oracle-spot-price/0.6.12/spot_price_getReserves.sol (1 hunks)
• tests/e2e/detectors/test_data/oracle-spot-price/0.8.20/spot_price_balanceOf.sol (1 hunks)
• tests/e2e/detectors/test_data/oracle-spot-price/0.8.20/spot_price_getAmountOut.sol (1 hunks)
• tests/e2e/detectors/test_detectors.py (1 hunks)

Files not reviewed due to errors (1)

• tests/e2e/detectors/test_detectors.py (no review received)

Additional Context Used

LanguageTool (17)

tests/e2e/detectors/snapshots/detectors__detector_SpotPriceDetector_0_6_12_spot_price_getReserves_sol__0.txt (4)

Near line 1: Possible spelling mistake found.
Context: ...ata/oracle-spot-price/0.6.12/spot_price_getReserves.sol#200 Method which could indicate usa...

---

Near line 2: Possible spelling mistake found.
Context: ...ate usage of spot price was detected in Uniswap V2 at tests/e2e/detectors/test_data/ora...

---

Near line 2: Possible spelling mistake found.
Context: ...ata/oracle-spot-price/0.6.12/spot_price_getReserves.sol#197 EXPRESSION (_yaxReserves) = yax...

---

Near line 3: Possible spelling mistake found.
Context: ..._price_getReserves.sol#197 EXPRESSION (_yaxReserves) = yaxisEthUniswapV2Pair.getReserves() ...

tests/e2e/detectors/snapshots/detectors__detector_SpotPriceDetector_0_8_20_spot_price_balanceOf_sol__0.txt (8)

Near line 1: Possible spelling mistake found.
Context: ...ata/oracle-spot-price/0.8.20/spot_price_balanceOf.sol#41 but the function is not used any...

---

Near line 1: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...price/0.8.20/spot_price_balanceOf.sol#41 but the function is not used anywhere in th...

---

Near line 2: Possible spelling mistake found.
Context: ...ata/oracle-spot-price/0.8.20/spot_price_balanceOf.sol#38 and tests/e2e/detectors/test_dat...

---

Near line 2: Possible spelling mistake found.
Context: ...ata/oracle-spot-price/0.8.20/spot_price_balanceOf.sol#39. NEW VARIABLE usdcBalance = IERC...

---

Near line 3: Possible spelling mistake found.
Context: ...ot_price_balanceOf.sol#39. NEW VARIABLE usdcBalance = IERC20(USDCAddress).balanceOf(pool) N...

---

Near line 3: Possible spelling mistake found.
Context: ...l#39. NEW VARIABLE usdcBalance = IERC20(USDCAddress).balanceOf(pool) NEW VARIABLE ethBalanc...

---

Near line 4: Possible spelling mistake found.
Context: ...DCAddress).balanceOf(pool) NEW VARIABLE ethBalance = IERC20(weth).balanceOf(pool)

---

Near line 4: Possible spelling mistake found.
Context: ...(pool) NEW VARIABLE ethBalance = IERC20(weth).balanceOf(pool)

tests/e2e/detectors/snapshots/detectors__detector_SpotPriceDetector_0_8_20_spot_price_getAmountOut_sol__0.txt (5)

Near line 1: Possible spelling mistake found.
Context: ...ata/oracle-spot-price/0.8.20/spot_price_getAmountOut.sol#66-71 but the function is not used ...

---

Near line 1: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...0.8.20/spot_price_getAmountOut.sol#66-71 but the function is not used anywhere in th...

---

Near line 2: Possible spelling mistake found.
Context: ...ate usage of spot price was detected in Uniswap V2 at tests/e2e/detectors/test_data/ora...

---

Near line 2: Possible spelling mistake found.
Context: ...ata/oracle-spot-price/0.8.20/spot_price_getAmountOut.sol#59 EXPRESSION (left,right) = pair.g...

---

Near line 3: Put a space after the comma.
Context: ...ice_getAmountOut.sol#59 EXPRESSION (left,right) = pair.getReserves()

Ruff (100)

slither/detectors/all_detectors.py (96)

2-2: .examples.backdoor.Backdoor imported but unused

---

3-3: .variables.uninitialized_state_variables.UninitializedStateVarsDetection imported but unused

---

4-4: .variables.uninitialized_storage_variables.UninitializedStorageVars imported but unused

---

5-5: .variables.uninitialized_local_variables.UninitializedLocalVars imported but unused

---

6-6: .variables.var_read_using_this.VarReadUsingThis imported but unused

---

7-7: .attributes.constant_pragma.ConstantPragma imported but unused

---

8-8: .attributes.incorrect_solc.IncorrectSolc imported but unused

---

9-9: .attributes.locked_ether.LockedEther imported but unused

---

10-10: .functions.arbitrary_send_eth.ArbitrarySendEth imported but unused

---

11-11: .erc.erc20.arbitrary_send_erc20_no_permit.ArbitrarySendErc20NoPermit imported but unused

---

12-12: .erc.erc20.arbitrary_send_erc20_permit.ArbitrarySendErc20Permit imported but unused

---

13-13: .functions.suicidal.Suicidal imported but unused

---

16-16: .reentrancy.reentrancy_benign.ReentrancyBenign imported but unused

---

17-17: .reentrancy.reentrancy_read_before_write.ReentrancyReadBeforeWritten imported but unused

---

18-18: .reentrancy.reentrancy_eth.ReentrancyEth imported but unused

---

19-19: .reentrancy.reentrancy_no_gas.ReentrancyNoGas imported but unused

---

20-20: .reentrancy.reentrancy_events.ReentrancyEvent imported but unused

---

21-21: .variables.unused_state_variables.UnusedStateVars imported but unused

---

22-22: .variables.could_be_constant.CouldBeConstant imported but unused

---

23-23: .variables.could_be_immutable.CouldBeImmutable imported but unused

---

24-24: .statements.tx_origin.TxOrigin imported but unused

---

25-25: .statements.assembly.Assembly imported but unused

---

26-26: .operations.low_level_calls.LowLevelCalls imported but unused

---

27-27: .operations.unused_return_values.UnusedReturnValues imported but unused

---

28-28: .operations.unchecked_transfer.UncheckedTransfer imported but unused

---

29-29: .naming_convention.naming_convention.NamingConvention imported but unused

---

30-30: .functions.external_function.ExternalFunction imported but unused

---

31-31: .statements.controlled_delegatecall.ControlledDelegateCall imported but unused

---

32-32: .attributes.const_functions_asm.ConstantFunctionsAsm imported but unused

---

33-33: .attributes.const_functions_state.ConstantFunctionsState imported but unused

---

34-34: .shadowing.abstract.ShadowingAbstractDetection imported but unused

---

35-35: .shadowing.state.StateShadowing imported but unused

---

36-36: .shadowing.local.LocalShadowing imported but unused

---

37-37: .shadowing.builtin_symbols.BuiltinSymbolShadowing imported but unused

---

38-38: .operations.block_timestamp.Timestamp imported but unused

---

39-39: .statements.calls_in_loop.MultipleCallsInLoop imported but unused

---

40-40: .statements.incorrect_strict_equality.IncorrectStrictEquality imported but unused

---

41-41: .erc.erc20.incorrect_erc20_interface.IncorrectERC20InterfaceDetection imported but unused

---

42-42: .erc.incorrect_erc721_interface.IncorrectERC721InterfaceDetection imported but unused

---

43-43: .erc.unindexed_event_parameters.UnindexedERC20EventParameters imported but unused

---

44-44: .statements.deprecated_calls.DeprecatedStandards imported but unused

---

45-45: .source.rtlo.RightToLeftOverride imported but unused

---

46-46: .statements.too_many_digits.TooManyDigits imported but unused

---

47-47: .operations.unchecked_low_level_return_values.UncheckedLowLevel imported but unused

---

48-48: .operations.unchecked_send_return_value.UncheckedSend imported but unused

---

49-49: .operations.void_constructor.VoidConstructor imported but unused

---

50-50: .statements.type_based_tautology.TypeBasedTautology imported but unused

---

51-51: .statements.boolean_constant_equality.BooleanEquality imported but unused

---

52-52: .statements.boolean_constant_misuse.BooleanConstantMisuse imported but unused

---

53-53: .statements.divide_before_multiply.DivideBeforeMultiply imported but unused

---

54-54: .statements.unprotected_upgradeable.UnprotectedUpgradeable imported but unused

---

55-55: .slither.name_reused.NameReused imported but unused

---

57-57: .functions.unimplemented.UnimplementedFunctionDetection imported but unused

---

58-58: .statements.mapping_deletion.MappingDeletionDetection imported but unused

---

59-59: .statements.array_length_assignment.ArrayLengthAssignment imported but unused

---

60-60: .variables.similar_variables.SimilarVarsDetection imported but unused

---

61-61: .variables.function_init_state_variables.FunctionInitializedState imported but unused

---

62-62: .statements.redundant_statements.RedundantStatements imported but unused

---

63-63: .operations.bad_prng.BadPRNG imported but unused

---

64-64: .statements.costly_operations_in_loop.CostlyOperationsInLoop imported but unused

---

65-65: .statements.assert_state_change.AssertStateChange imported but unused

---

66-66: .attributes.unimplemented_interface.MissingInheritance imported but unused

---

67-67: .assembly.shift_parameter_mixup.ShiftParameterMixup imported but unused

---

68-68: .compiler_bugs.storage_signed_integer_array.StorageSignedIntegerArray imported but unused

---

70-70: .compiler_bugs.uninitialized_function_ptr_in_constructor.UninitializedFunctionPtrsConstructor imported but unused

---

72-72: .compiler_bugs.storage_ABIEncoderV2_array.ABIEncoderV2Array imported but unused

---

73-73: .compiler_bugs.array_by_reference.ArrayByReference imported but unused

---

74-74: .compiler_bugs.enum_conversion.EnumConversion imported but unused

---

75-75: .compiler_bugs.multiple_constructor_schemes.MultipleConstructorSchemes imported but unused

---

76-76: .compiler_bugs.public_mapping_nested.PublicMappingNested imported but unused

---

77-77: .compiler_bugs.reused_base_constructor.ReusedBaseConstructor imported but unused

---

78-78: .operations.missing_events_access_control.MissingEventsAccessControl imported but unused

---

79-79: .operations.missing_events_arithmetic.MissingEventsArithmetic imported but unused

---

80-80: .functions.modifier.ModifierDefaultDetection imported but unused

---

81-81: .variables.predeclaration_usage_local.PredeclarationUsageLocal imported but unused

---

82-82: .statements.unary.IncorrectUnaryExpressionDetection imported but unused

---

83-83: .operations.missing_zero_address_validation.MissingZeroAddressValidation imported but unused

---

84-84: .functions.dead_code.DeadCode imported but unused

---

85-85: .statements.write_after_write.WriteAfterWrite imported but unused

---

86-86: .statements.msg_value_in_loop.MsgValueInLoop imported but unused

---

87-87: .statements.delegatecall_in_loop.DelegatecallInLoop imported but unused

---

88-88: .functions.protected_variable.ProtectedVariables imported but unused

---

89-89: .functions.permit_domain_signature_collision.DomainSeparatorCollision imported but unused

---

90-90: .functions.codex.Codex imported but unused

---

91-91: .functions.cyclomatic_complexity.CyclomaticComplexity imported but unused

---

92-92: .operations.cache_array_length.CacheArrayLength imported but unused

---

93-93: .statements.incorrect_using_for.IncorrectUsingFor imported but unused

---

94-94: .operations.encode_packed.EncodePackedCollision imported but unused

---

95-95: .assembly.incorrect_return.IncorrectReturn imported but unused

---

96-96: .assembly.return_instead_of_leave.ReturnInsteadOfLeave imported but unused

---

97-97: .operations.incorrect_exp.IncorrectOperatorExponentiation imported but unused

---

98-98: .statements.tautological_compare.TautologicalCompare imported but unused

---

99-99: .statements.return_bomb.ReturnBomb imported but unused

---

100-100: .functions.out_of_order_retryable.OutOfOrderRetryable imported but unused

---

101-101: .statements.unused_import.UnusedImport imported but unused

---

102-102: .oracles.spot_price.SpotPriceDetector imported but unused

slither/slithir/convert.py (4)

354-354: Test for membership should be not in

---

1594-1594: Undefined name contract

---

1905-1905: Test for membership should be not in

---

1915-1915: Test for membership should be not in

Additional comments not posted (13)

tests/e2e/detectors/snapshots/detectors__detector_SpotPriceDetector_0_6_12_spot_price_getReserves_sol__0.txt (3)

1-1: Ensure the referenced line number in spot_price_getReserves.sol#200 is accurate and relevant to the spot price data calculations.

---

2-2: Confirm the method detection at spot_price_getReserves.sol#197 is correctly identifying spot price usage.

---

3-3: Check the correctness of the expression _yaxReserves = yaxisEthUniswapV2Pair.getReserves() in the context of detecting spot price usage.

tests/e2e/detectors/snapshots/detectors__detector_SpotPriceDetector_0_8_20_spot_price_getAmountOut_sol__0.txt (3)

1-1: Ensure the referenced line number in spot_price_getAmountOut.sol#66-71 is accurate and relevant to the spot price data calculations.

---

2-2: Confirm the method detection at spot_price_getAmountOut.sol#59 is correctly identifying spot price usage.

---

3-3: Check the correctness of the expression (left,right) = pair.getReserves() in the context of detecting spot price usage.

tests/e2e/detectors/snapshots/detectors__detector_SpotPriceDetector_0_8_20_spot_price_balanceOf_sol__0.txt (3)

1-1: Ensure the referenced line number in spot_price_balanceOf.sol#41 is accurate and relevant to the spot price data calculations.

---

2-2: Confirm the method detection at spot_price_balanceOf.sol#38 and spot_price_balanceOf.sol#39 is correctly identifying spot price usage.

---

3-4: Check the correctness of the expressions usdcBalance = IERC20(USDCAddress).balanceOf(pool) and ethBalance = IERC20(weth).balanceOf(pool) in the context of detecting spot price usage.

slither/detectors/all_detectors.py (1)

102-102: Ensure the SpotPriceDetector is correctly imported and utilized within the Slither framework. Verify that it is integrated into the detection suite and that there are no import errors.

slither/detectors/oracles/spot_price.py (3)

66-85: Verify the logic in instance_of_call to ensure it correctly identifies instances of calls that are relevant to spot price detection. Check for any potential logical errors or improvements that could be made.

---

113-177: Ensure the detect_oracle_call method accurately identifies oracle calls related to spot prices. Verify that the method handles different scenarios correctly and efficiently.

---

180-393: Review the detect_spot_price_usage and _detect methods for efficiency and correctness. Ensure that they integrate well with the rest of the Slither framework and provide accurate detection of spot price usage.

---

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

• X
• Mastodon
• Reddit
• LinkedIn

Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai generate interesting stats about this repository and render them as a table.
  • @coderabbitai show all the console.log statements in this repository.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[coderabbitai]

Actionable comments posted: 4