diff --git a/CODEOWNERS b/CODEOWNERS index c92f0d79d..496da0c30 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -1,5 +1,4 @@ -* @montyly @0xalpharush @smonicas -/slither/tools/read_storage/ @0xalpharush +* @montyly @smonicas /slither/tools/doctor/ @elopez /slither/slithir/ @montyly /slither/analyses/ @montyly diff --git a/slither/detectors/all_detectors.py b/slither/detectors/all_detectors.py index 0b93b9d75..067a2b0a1 100644 --- a/slither/detectors/all_detectors.py +++ b/slither/detectors/all_detectors.py @@ -98,5 +98,7 @@ from .statements.return_bomb import ReturnBomb from .functions.out_of_order_retryable import OutOfOrderRetryable from .functions.chainlink_feed_registry import ChainlinkFeedRegistry +from .functions.pyth_deprecated_functions import PythDeprecatedFunctions +from .functions.optimism_deprecation import OptimismDeprecation # from .statements.unused_import import UnusedImport diff --git a/slither/detectors/functions/optimism_deprecation.py b/slither/detectors/functions/optimism_deprecation.py new file mode 100644 index 000000000..752e8bb2d --- /dev/null +++ b/slither/detectors/functions/optimism_deprecation.py @@ -0,0 +1,92 @@ +from typing import List + +from slither.detectors.abstract_detector import ( + AbstractDetector, + DetectorClassification, + DETECTOR_INFO, +) +from slither.core.cfg.node import Node +from slither.core.variables.variable import Variable +from slither.core.expressions import TypeConversion, Literal +from slither.utils.output import Output + + +class OptimismDeprecation(AbstractDetector): + + ARGUMENT = "optimism-deprecation" + HELP = "Detect when deprecated Optimism predeploy or function is used." + IMPACT = DetectorClassification.LOW + CONFIDENCE = DetectorClassification.HIGH + + WIKI = "https://github.com/crytic/slither/wiki/Detector-Documentation#optimism-deprecation" + + WIKI_TITLE = "Optimism deprecated predeploy or function" + WIKI_DESCRIPTION = "Detect when deprecated Optimism predeploy or function is used." + + # region wiki_exploit_scenario + WIKI_EXPLOIT_SCENARIO = """ +```solidity +interface GasPriceOracle { + function scalar() external view returns (uint256); +} + +contract Test { + GasPriceOracle constant OPT_GAS = GasPriceOracle(0x420000000000000000000000000000000000000F); + + function a() public { + OPT_GAS.scalar(); + } +} +``` +The call to the `scalar` function of the Optimism GasPriceOracle predeploy always revert. +""" + # endregion wiki_exploit_scenario + + WIKI_RECOMMENDATION = "Do not use the deprecated components." + + def _detect(self) -> List[Output]: + results = [] + + deprecated_predeploys = [ + "0x4200000000000000000000000000000000000000", # LegacyMessagePasser + "0x4200000000000000000000000000000000000001", # L1MessageSender + "0x4200000000000000000000000000000000000002", # DeployerWhitelist + "0x4200000000000000000000000000000000000013", # L1BlockNumber + ] + + for contract in self.compilation_unit.contracts_derived: + use_deprecated: List[Node] = [] + + for _, ir in contract.all_high_level_calls: + # To avoid FPs we assume predeploy contracts are always assigned to a constant and typecasted to an interface + # and we check the target address of a high level call. + if ( + isinstance(ir.destination, Variable) + and isinstance(ir.destination.expression, TypeConversion) + and isinstance(ir.destination.expression.expression, Literal) + ): + if ir.destination.expression.expression.value in deprecated_predeploys: + use_deprecated.append(ir.node) + + if ( + ir.destination.expression.expression.value + == "0x420000000000000000000000000000000000000F" + and ir.function_name in ("overhead", "scalar", "getL1GasUsed") + ): + use_deprecated.append(ir.node) + # Sort so output is deterministic + use_deprecated.sort(key=lambda x: (x.node_id, x.function.full_name)) + if len(use_deprecated) > 0: + info: DETECTOR_INFO = [ + "A deprecated Optimism predeploy or function is used in the ", + contract.name, + " contract.\n", + ] + + for node in use_deprecated: + info.extend(["\t - ", node, "\n"]) + + res = self.generate_result(info) + results.append(res) + + return results diff --git a/slither/detectors/functions/pyth_deprecated_functions.py b/slither/detectors/functions/pyth_deprecated_functions.py new file mode 100644 index 000000000..87cff9181 --- /dev/null +++ b/slither/detectors/functions/pyth_deprecated_functions.py @@ -0,0 +1,73 @@ +from typing import List + +from slither.detectors.abstract_detector import ( + AbstractDetector, + DetectorClassification, + DETECTOR_INFO, +) +from slither.utils.output import Output + + +class PythDeprecatedFunctions(AbstractDetector): + """ + Documentation: This detector finds deprecated Pyth function calls + """ + + ARGUMENT = "pyth-deprecated-functions" + HELP = "Detect Pyth deprecated functions" + IMPACT = DetectorClassification.MEDIUM + CONFIDENCE = DetectorClassification.HIGH + + WIKI = "https://github.com/crytic/slither/wiki/Detector-Documentation#pyth-deprecated-functions" + WIKI_TITLE = "Pyth deprecated functions" + WIKI_DESCRIPTION = "Detect when a Pyth deprecated function is used" + WIKI_RECOMMENDATION = ( + "Do not use deprecated Pyth functions. Visit https://api-reference.pyth.network/." + ) + + WIKI_EXPLOIT_SCENARIO = """ +```solidity +import "@pythnetwork/pyth-sdk-solidity/IPyth.sol"; +import "@pythnetwork/pyth-sdk-solidity/PythStructs.sol"; + +contract C { + + IPyth pyth; + + constructor(IPyth _pyth) { + pyth = _pyth; + } + + function A(bytes32 priceId) public { + PythStructs.Price memory price = pyth.getPrice(priceId); + ... + } +} +``` +The function `A` uses the deprecated `getPrice` Pyth function. +""" + + def _detect(self): + DEPRECATED_PYTH_FUNCTIONS = [ + "getValidTimePeriod", + "getEmaPrice", + "getPrice", + ] + results: List[Output] = [] + + for contract in self.compilation_unit.contracts_derived: + for target_contract, ir in contract.all_high_level_calls: + if ( + target_contract.name == "IPyth" + and ir.function_name in DEPRECATED_PYTH_FUNCTIONS + ): + info: DETECTOR_INFO = [ + "The following Pyth deprecated function is used\n\t- ", + ir.node, + "\n", + ] + + res = self.generate_result(info) + results.append(res) + + return results diff --git a/tests/e2e/detectors/snapshots/detectors__detector_OptimismDeprecation_0_8_20_optimism_deprecation_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_OptimismDeprecation_0_8_20_optimism_deprecation_sol__0.txt new file mode 100644 index 000000000..f6f4dccba --- /dev/null +++ b/tests/e2e/detectors/snapshots/detectors__detector_OptimismDeprecation_0_8_20_optimism_deprecation_sol__0.txt @@ -0,0 +1,4 @@ +A deprecated Optimism predeploy or function is used in the Test contract. + - OPT_GAS.scalar() (tests/e2e/detectors/test_data/optimism-deprecation/0.8.20/optimism_deprecation.sol#15) + - L1_BLOCK_NUMBER.q() (tests/e2e/detectors/test_data/optimism-deprecation/0.8.20/optimism_deprecation.sol#19) + diff --git a/tests/e2e/detectors/snapshots/detectors__detector_PythDeprecatedFunctions_0_8_20_pyth_deprecated_functions_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_PythDeprecatedFunctions_0_8_20_pyth_deprecated_functions_sol__0.txt new file mode 100644 index 000000000..4cc23d213 --- /dev/null +++ b/tests/e2e/detectors/snapshots/detectors__detector_PythDeprecatedFunctions_0_8_20_pyth_deprecated_functions_sol__0.txt @@ -0,0 +1,3 @@ +The following Pyth deprecated function is used + - price = pyth.getPrice(priceId) (tests/e2e/detectors/test_data/pyth-deprecated-functions/0.8.20/pyth_deprecated_functions.sol#23) + diff --git a/tests/e2e/detectors/test_data/optimism-deprecation/0.8.20/optimism_deprecation.sol b/tests/e2e/detectors/test_data/optimism-deprecation/0.8.20/optimism_deprecation.sol new file mode 100644 index 000000000..7ad55f3dd --- /dev/null +++ b/tests/e2e/detectors/test_data/optimism-deprecation/0.8.20/optimism_deprecation.sol @@ -0,0 +1,27 @@ +interface GasPriceOracle { + function scalar() external view returns (uint256); + function baseFee() external view returns (uint256); +} + +interface L1BlockNumber { + function q() external view returns (uint256); +} + +contract Test { + GasPriceOracle constant OPT_GAS = GasPriceOracle(0x420000000000000000000000000000000000000F); + L1BlockNumber constant L1_BLOCK_NUMBER = L1BlockNumber(0x4200000000000000000000000000000000000013); + + function bad() public { + OPT_GAS.scalar(); + } + + function bad2() public { + L1_BLOCK_NUMBER.q(); + } + + function good() public { + OPT_GAS.baseFee(); + } + + +} diff --git a/tests/e2e/detectors/test_data/optimism-deprecation/0.8.20/optimism_deprecation.sol-0.8.20.zip b/tests/e2e/detectors/test_data/optimism-deprecation/0.8.20/optimism_deprecation.sol-0.8.20.zip new file mode 100644 index 000000000..de18d4a0d Binary files /dev/null and b/tests/e2e/detectors/test_data/optimism-deprecation/0.8.20/optimism_deprecation.sol-0.8.20.zip differ diff --git a/tests/e2e/detectors/test_data/pyth-deprecated-functions/0.8.20/pyth_deprecated_functions.sol b/tests/e2e/detectors/test_data/pyth-deprecated-functions/0.8.20/pyth_deprecated_functions.sol new file mode 100644 index 000000000..dc8130db5 --- /dev/null +++ b/tests/e2e/detectors/test_data/pyth-deprecated-functions/0.8.20/pyth_deprecated_functions.sol @@ -0,0 +1,35 @@ + +// Fake Pyth interface +interface IPyth { + function getPrice(bytes32 id) external returns (uint256 price); + function notDeprecated(bytes32 id) external returns (uint256 price); +} + +interface INotPyth { + function getPrice(bytes32 id) external returns (uint256 price); +} + +contract C { + + IPyth pyth; + INotPyth notPyth; + + constructor(IPyth _pyth, INotPyth _notPyth) { + pyth = _pyth; + notPyth = _notPyth; + } + + function Deprecated(bytes32 priceId) public { + uint256 price = pyth.getPrice(priceId); + } + + function notDeprecated(bytes32 priceId) public { + uint256 price = pyth.notDeprecated(priceId); + } + + function notPythCall(bytes32 priceId) public { + uint256 price = notPyth.getPrice(priceId); + } + + +} diff --git a/tests/e2e/detectors/test_data/pyth-deprecated-functions/0.8.20/pyth_deprecated_functions.sol-0.8.20.zip b/tests/e2e/detectors/test_data/pyth-deprecated-functions/0.8.20/pyth_deprecated_functions.sol-0.8.20.zip new file mode 100644 index 000000000..258a28c93 Binary files /dev/null and b/tests/e2e/detectors/test_data/pyth-deprecated-functions/0.8.20/pyth_deprecated_functions.sol-0.8.20.zip differ diff --git a/tests/e2e/detectors/test_detectors.py b/tests/e2e/detectors/test_detectors.py index 371553d70..32cbbb491 100644 --- a/tests/e2e/detectors/test_detectors.py +++ b/tests/e2e/detectors/test_detectors.py @@ -1719,6 +1719,16 @@ def id_test(test_item: Test): "chainlink_feed_registry.sol", "0.8.20", ), + Test( + all_detectors.PythDeprecatedFunctions, + "pyth_deprecated_functions.sol", + "0.8.20", + ), + Test( + all_detectors.OptimismDeprecation, + "optimism_deprecation.sol", + "0.8.20", + ), # Test( # all_detectors.UnusedImport, # "ConstantContractLevelUsedInContractTest.sol",