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/core/variables/state_variable.py b/slither/core/variables/state_variable.py index f2a2d6ee3..404cf74ba 100644 --- a/slither/core/variables/state_variable.py +++ b/slither/core/variables/state_variable.py @@ -12,6 +12,7 @@ class StateVariable(ContractLevel, Variable): def __init__(self) -> None: super().__init__() self._node_initialization: Optional["Node"] = None + self._location: Optional[str] = None def is_declared_by(self, contract: "Contract") -> bool: """ @@ -21,6 +22,19 @@ class StateVariable(ContractLevel, Variable): """ return self.contract == contract + def set_location(self, loc: str) -> None: + self._location = loc + + @property + def location(self) -> Optional[str]: + """ + Variable Location + Can be default or transient + Returns: + (str) + """ + return self._location + # endregion ################################################################################### ################################################################################### diff --git a/slither/detectors/all_detectors.py b/slither/detectors/all_detectors.py index 9bfdc5cd4..437222346 100644 --- a/slither/detectors/all_detectors.py +++ b/slither/detectors/all_detectors.py @@ -97,6 +97,13 @@ from .operations.incorrect_exp import IncorrectOperatorExponentiation from .statements.tautological_compare import TautologicalCompare from .statements.return_bomb import ReturnBomb from .functions.out_of_order_retryable import OutOfOrderRetryable +from .functions.gelato_unprotected_randomness import GelatoUnprotectedRandomness +from .statements.chronicle_unchecked_price import ChronicleUncheckedPrice +from .statements.pyth_unchecked_confidence import PythUncheckedConfidence +from .statements.pyth_unchecked_publishtime import PythUncheckedPublishTime +from .functions.chainlink_feed_registry import ChainlinkFeedRegistry +from .functions.pyth_deprecated_functions import PythDeprecatedFunctions +from .functions.optimism_deprecation import OptimismDeprecation from .statements.unused_custom_errors import UnusedCustomErrors # from .statements.unused_import import UnusedImport diff --git a/slither/detectors/functions/chainlink_feed_registry.py b/slither/detectors/functions/chainlink_feed_registry.py new file mode 100644 index 000000000..82ab17424 --- /dev/null +++ b/slither/detectors/functions/chainlink_feed_registry.py @@ -0,0 +1,102 @@ +from typing import List + +from slither.detectors.abstract_detector import ( + AbstractDetector, + DetectorClassification, + DETECTOR_INFO, +) +from slither.utils.output import Output + + +class ChainlinkFeedRegistry(AbstractDetector): + + ARGUMENT = "chainlink-feed-registry" + HELP = "Detect when chainlink feed registry is used" + IMPACT = DetectorClassification.LOW + CONFIDENCE = DetectorClassification.HIGH + + WIKI = "https://github.com/crytic/slither/wiki/Detector-Documentation#chainlink-feed-registry" + + WIKI_TITLE = "Chainlink Feed Registry usage" + WIKI_DESCRIPTION = "Detect when Chainlink Feed Registry is used. At the moment is only available on Ethereum Mainnet." + + # region wiki_exploit_scenario + WIKI_EXPLOIT_SCENARIO = """ +```solidity +import "chainlink/contracts/src/v0.8/interfaces/FeedRegistryInteface.sol" + +contract A { + FeedRegistryInterface public immutable registry; + + constructor(address _registry) { + registry = _registry; + } + + function getPrice(address base, address quote) public return(uint256) { + (, int256 price,,,) = registry.latestRoundData(base, quote); + // Do price validation + return uint256(price); + } +} +``` +If the contract is deployed on a different chain than Ethereum Mainnet the `getPrice` function will revert. +""" + # endregion wiki_exploit_scenario + + WIKI_RECOMMENDATION = "Do not use Chainlink Feed Registry outside of Ethereum Mainnet." + + def _detect(self) -> List[Output]: + # https://github.com/smartcontractkit/chainlink/blob/8ca41fc8f722accfccccb4b1778db2df8fef5437/contracts/src/v0.8/interfaces/FeedRegistryInterface.sol + registry_functions = [ + "decimals", + "description", + "versiom", + "latestRoundData", + "getRoundData", + "latestAnswer", + "latestTimestamp", + "latestRound", + "getAnswer", + "getTimestamp", + "getFeed", + "getPhaseFeed", + "isFeedEnabled", + "getPhase", + "getRoundFeed", + "getPhaseRange", + "getPreviousRoundId", + "getNextRoundId", + "proposeFeed", + "confirmFeed", + "getProposedFeed", + "proposedGetRoundData", + "proposedLatestRoundData", + "getCurrentPhaseId", + ] + results = [] + + for contract in self.compilation_unit.contracts_derived: + nodes = [] + for target, ir in contract.all_high_level_calls: + if ( + target.name == "FeedRegistryInterface" + and ir.function_name in registry_functions + ): + nodes.append(ir.node) + # Sort so output is deterministic + nodes.sort(key=lambda x: (x.node_id, x.function.full_name)) + + if len(nodes) > 0: + info: DETECTOR_INFO = [ + "The Chainlink Feed Registry is used in the ", + contract.name, + " contract. It's only available on Ethereum Mainnet, consider to not use it if the contract needs to be deployed on other chains.\n", + ] + + for node in nodes: + info.extend(["\t - ", node, "\n"]) + + res = self.generate_result(info) + results.append(res) + + return results diff --git a/slither/detectors/functions/gelato_unprotected_randomness.py b/slither/detectors/functions/gelato_unprotected_randomness.py new file mode 100644 index 000000000..bdc3a6fb0 --- /dev/null +++ b/slither/detectors/functions/gelato_unprotected_randomness.py @@ -0,0 +1,78 @@ +from typing import List + +from slither.slithir.operations.internal_call import InternalCall +from slither.detectors.abstract_detector import ( + AbstractDetector, + DetectorClassification, + DETECTOR_INFO, +) +from slither.utils.output import Output + + +class GelatoUnprotectedRandomness(AbstractDetector): + """ + Unprotected Gelato VRF requests + """ + + ARGUMENT = "gelato-unprotected-randomness" + HELP = "Call to _requestRandomness within an unprotected function" + IMPACT = DetectorClassification.MEDIUM + CONFIDENCE = DetectorClassification.MEDIUM + + WIKI = "https://github.com/crytic/slither/wiki/Detector-Documentation#gelato-unprotected-randomness" + + WIKI_TITLE = "Gelato unprotected randomness" + WIKI_DESCRIPTION = "Detect calls to `_requestRandomness` within an unprotected function." + + # region wiki_exploit_scenario + WIKI_EXPLOIT_SCENARIO = """ +```solidity +contract C is GelatoVRFConsumerBase { + function _fulfillRandomness( + uint256 randomness, + uint256, + bytes memory extraData + ) internal override { + // Do something with the random number + } + + function bad() public { + _requestRandomness(abi.encode(msg.sender)); + } +} +``` +The function `bad` is uprotected and requests randomness.""" + # endregion wiki_exploit_scenario + + WIKI_RECOMMENDATION = ( + "Function that request randomness should be allowed only to authorized users." + ) + + def _detect(self) -> List[Output]: + results = [] + + for contract in self.compilation_unit.contracts_derived: + if "GelatoVRFConsumerBase" in [c.name for c in contract.inheritance]: + for function in contract.functions_entry_points: + if not function.is_protected() and ( + nodes_request := [ + ir.node + for ir in function.all_internal_calls() + if isinstance(ir, InternalCall) + and ir.function_name == "_requestRandomness" + ] + ): + # Sort so output is deterministic + nodes_request.sort(key=lambda x: (x.node_id, x.function.full_name)) + + for node in nodes_request: + info: DETECTOR_INFO = [ + function, + " is unprotected and request randomness from Gelato VRF\n\t- ", + node, + "\n", + ] + res = self.generate_result(info) + results.append(res) + + return results 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/slither/detectors/statements/chronicle_unchecked_price.py b/slither/detectors/statements/chronicle_unchecked_price.py new file mode 100644 index 000000000..47ad2ddc5 --- /dev/null +++ b/slither/detectors/statements/chronicle_unchecked_price.py @@ -0,0 +1,147 @@ +from typing import List + +from slither.detectors.abstract_detector import ( + AbstractDetector, + DetectorClassification, + DETECTOR_INFO, +) +from slither.utils.output import Output +from slither.slithir.operations import Binary, Assignment, Unpack, SolidityCall +from slither.core.variables import Variable +from slither.core.declarations.solidity_variables import SolidityFunction +from slither.core.cfg.node import Node + + +class ChronicleUncheckedPrice(AbstractDetector): + """ + Documentation: This detector finds calls to Chronicle oracle where the returned price is not checked + https://docs.chroniclelabs.org/Resources/FAQ/Oracles#how-do-i-check-if-an-oracle-becomes-inactive-gets-deprecated + """ + + ARGUMENT = "chronicle-unchecked-price" + HELP = "Detect when Chronicle price is not checked." + IMPACT = DetectorClassification.MEDIUM + CONFIDENCE = DetectorClassification.MEDIUM + + WIKI = "https://github.com/crytic/slither/wiki/Detector-Documentation#chronicle-unchecked-price" + + WIKI_TITLE = "Chronicle unchecked price" + WIKI_DESCRIPTION = "Chronicle oracle is used and the price returned is not checked to be valid. For more information https://docs.chroniclelabs.org/Resources/FAQ/Oracles#how-do-i-check-if-an-oracle-becomes-inactive-gets-deprecated." + + # region wiki_exploit_scenario + WIKI_EXPLOIT_SCENARIO = """ +```solidity +contract C { + IChronicle chronicle; + + constructor(address a) { + chronicle = IChronicle(a); + } + + function bad() public { + uint256 price = chronicle.read(); + } +``` +The `bad` function gets the price from Chronicle by calling the read function however it does not check if the price is valid.""" + # endregion wiki_exploit_scenario + + WIKI_RECOMMENDATION = "Validate that the price returned by the oracle is valid." + + def _var_is_checked(self, nodes: List[Node], var_to_check: Variable) -> bool: + visited = set() + checked = False + + while nodes: + if checked: + break + next_node = nodes[0] + nodes = nodes[1:] + + for node_ir in next_node.all_slithir_operations(): + if isinstance(node_ir, Binary) and var_to_check in node_ir.read: + checked = True + break + # This case is for tryRead and tryReadWithAge + # if the isValid boolean is checked inside a require(isValid) + if ( + isinstance(node_ir, SolidityCall) + and node_ir.function + in ( + SolidityFunction("require(bool)"), + SolidityFunction("require(bool,string)"), + SolidityFunction("require(bool,error)"), + ) + and var_to_check in node_ir.read + ): + checked = True + break + + if next_node not in visited: + visited.add(next_node) + for son in next_node.sons: + if son not in visited: + nodes.append(son) + return checked + + # pylint: disable=too-many-nested-blocks,too-many-branches + def _detect(self) -> List[Output]: + results: List[Output] = [] + + for contract in self.compilation_unit.contracts_derived: + for target_contract, ir in sorted( + contract.all_high_level_calls, + key=lambda x: (x[1].node.node_id, x[1].node.function.full_name), + ): + if target_contract.name in ("IScribe", "IChronicle") and ir.function_name in ( + "read", + "tryRead", + "readWithAge", + "tryReadWithAge", + "latestAnswer", + "latestRoundData", + ): + found = False + if ir.function_name in ("read", "latestAnswer"): + # We need to iterate the IRs as we are not always sure that the following IR is the assignment + # for example in case of type conversion it isn't + for node_ir in ir.node.irs: + if isinstance(node_ir, Assignment): + possible_unchecked_variable_ir = node_ir.lvalue + found = True + break + elif ir.function_name in ("readWithAge", "tryRead", "tryReadWithAge"): + # We are interested in the first item of the tuple + # readWithAge : value + # tryRead/tryReadWithAge : isValid + for node_ir in ir.node.irs: + if isinstance(node_ir, Unpack) and node_ir.index == 0: + possible_unchecked_variable_ir = node_ir.lvalue + found = True + break + elif ir.function_name == "latestRoundData": + found = False + for node_ir in ir.node.irs: + if isinstance(node_ir, Unpack) and node_ir.index == 1: + possible_unchecked_variable_ir = node_ir.lvalue + found = True + break + + # If we did not find the variable assignment we know it's not checked + checked = ( + self._var_is_checked(ir.node.sons, possible_unchecked_variable_ir) + if found + else False + ) + + if not checked: + info: DETECTOR_INFO = [ + "Chronicle price is not checked to be valid in ", + ir.node.function, + "\n\t- ", + ir.node, + "\n", + ] + res = self.generate_result(info) + results.append(res) + + return results diff --git a/slither/detectors/statements/pyth_unchecked.py b/slither/detectors/statements/pyth_unchecked.py new file mode 100644 index 000000000..959aee6a5 --- /dev/null +++ b/slither/detectors/statements/pyth_unchecked.py @@ -0,0 +1,79 @@ +from typing import List + +from slither.detectors.abstract_detector import ( + AbstractDetector, + DETECTOR_INFO, +) +from slither.utils.output import Output +from slither.slithir.operations import Member, Binary, Assignment + + +class PythUnchecked(AbstractDetector): + """ + Documentation: This detector finds deprecated Pyth function calls + """ + + # To be overriden in the derived class + PYTH_FUNCTIONS = [] + PYTH_FIELD = "" + + # pylint: disable=too-many-nested-blocks + def _detect(self) -> List[Output]: + 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 self.PYTH_FUNCTIONS: + # We know for sure the second IR in the node is an Assignment operation of the TMP variable. Example: + # Expression: price = pyth.getEmaPriceNoOlderThan(id,age) + # IRs: + # TMP_0(PythStructs.Price) = HIGH_LEVEL_CALL, dest:pyth(IPyth), function:getEmaPriceNoOlderThan, arguments:['id', 'age'] + # price(PythStructs.Price) := TMP_0(PythStructs.Price) + assert isinstance(ir.node.irs[1], Assignment) + return_variable = ir.node.irs[1].lvalue + checked = False + + possible_unchecked_variable_ir = None + nodes = ir.node.sons + visited = set() + while nodes: + if checked: + break + next_node = nodes[0] + nodes = nodes[1:] + + for node_ir in next_node.all_slithir_operations(): + # We are accessing the unchecked_var field of the returned Price struct + if ( + isinstance(node_ir, Member) + and node_ir.variable_left == return_variable + and node_ir.variable_right.name == self.PYTH_FIELD + ): + possible_unchecked_variable_ir = node_ir.lvalue + # We assume that if unchecked_var happens to be inside a binary operation is checked + if ( + isinstance(node_ir, Binary) + and possible_unchecked_variable_ir is not None + and possible_unchecked_variable_ir in node_ir.read + ): + checked = True + break + + if next_node not in visited: + visited.add(next_node) + for son in next_node.sons: + if son not in visited: + nodes.append(son) + + if not checked: + info: DETECTOR_INFO = [ + f"Pyth price {self.PYTH_FIELD} field is not checked in ", + ir.node.function, + "\n\t- ", + ir.node, + "\n", + ] + res = self.generate_result(info) + results.append(res) + + return results diff --git a/slither/detectors/statements/pyth_unchecked_confidence.py b/slither/detectors/statements/pyth_unchecked_confidence.py new file mode 100644 index 000000000..2e99851a8 --- /dev/null +++ b/slither/detectors/statements/pyth_unchecked_confidence.py @@ -0,0 +1,50 @@ +from slither.detectors.abstract_detector import DetectorClassification +from slither.detectors.statements.pyth_unchecked import PythUnchecked + + +class PythUncheckedConfidence(PythUnchecked): + """ + Documentation: This detector finds when the confidence level of a Pyth price is not checked + """ + + ARGUMENT = "pyth-unchecked-confidence" + HELP = "Detect when the confidence level of a Pyth price is not checked" + IMPACT = DetectorClassification.MEDIUM + CONFIDENCE = DetectorClassification.HIGH + + WIKI = "https://github.com/crytic/slither/wiki/Detector-Documentation#pyth-unchecked-confidence" + WIKI_TITLE = "Pyth unchecked confidence level" + WIKI_DESCRIPTION = "Detect when the confidence level of a Pyth price is not checked" + WIKI_RECOMMENDATION = "Check the confidence level of a Pyth price. Visit https://docs.pyth.network/price-feeds/best-practices#confidence-intervals for more information." + + 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 bad(bytes32 id, uint256 age) public { + PythStructs.Price memory price = pyth.getEmaPriceNoOlderThan(id, age); + // Use price + } +} +``` +The function `A` uses the price without checking its confidence level. +""" + + PYTH_FUNCTIONS = [ + "getEmaPrice", + "getEmaPriceNoOlderThan", + "getEmaPriceUnsafe", + "getPrice", + "getPriceNoOlderThan", + "getPriceUnsafe", + ] + + PYTH_FIELD = "conf" diff --git a/slither/detectors/statements/pyth_unchecked_publishtime.py b/slither/detectors/statements/pyth_unchecked_publishtime.py new file mode 100644 index 000000000..e3e2010d6 --- /dev/null +++ b/slither/detectors/statements/pyth_unchecked_publishtime.py @@ -0,0 +1,52 @@ +from slither.detectors.abstract_detector import DetectorClassification +from slither.detectors.statements.pyth_unchecked import PythUnchecked + + +class PythUncheckedPublishTime(PythUnchecked): + """ + Documentation: This detector finds when the publishTime of a Pyth price is not checked + """ + + ARGUMENT = "pyth-unchecked-publishtime" + HELP = "Detect when the publishTime of a Pyth price is not checked" + IMPACT = DetectorClassification.MEDIUM + CONFIDENCE = DetectorClassification.HIGH + + WIKI = ( + "https://github.com/crytic/slither/wiki/Detector-Documentation#pyth-unchecked-publishtime" + ) + WIKI_TITLE = "Pyth unchecked publishTime" + WIKI_DESCRIPTION = "Detect when the publishTime of a Pyth price is not checked" + WIKI_RECOMMENDATION = "Check the publishTime of a Pyth price." + + 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 bad(bytes32 id) public { + PythStructs.Price memory price = pyth.getEmaPriceUnsafe(id); + // Use price + } +} +``` +The function `A` uses the price without checking its `publishTime` coming from the `getEmaPriceUnsafe` function. +""" + + PYTH_FUNCTIONS = [ + "getEmaPrice", + # "getEmaPriceNoOlderThan", + "getEmaPriceUnsafe", + "getPrice", + # "getPriceNoOlderThan", + "getPriceUnsafe", + ] + + PYTH_FIELD = "publishTime" diff --git a/slither/solc_parsing/variables/state_variable.py b/slither/solc_parsing/variables/state_variable.py index a9c0ff730..227a84c61 100644 --- a/slither/solc_parsing/variables/state_variable.py +++ b/slither/solc_parsing/variables/state_variable.py @@ -13,3 +13,18 @@ class StateVariableSolc(VariableDeclarationSolc): # Todo: Not sure how to overcome this with mypy assert isinstance(self._variable, StateVariable) return self._variable + + def _analyze_variable_attributes(self, attributes: Dict) -> None: + """ + Variable Location + Can be default or transient + """ + if "storageLocation" in attributes: + self.underlying_variable.set_location(attributes["storageLocation"]) + else: + # We don't have to support legacy ast + # as transient location was added in 0.8.28 + # and we know it must be default + self.underlying_variable.set_location("default") + + super()._analyze_variable_attributes(attributes) diff --git a/tests/e2e/detectors/snapshots/detectors__detector_ChainlinkFeedRegistry_0_8_20_chainlink_feed_registry_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_ChainlinkFeedRegistry_0_8_20_chainlink_feed_registry_sol__0.txt new file mode 100644 index 000000000..6b7653ed0 --- /dev/null +++ b/tests/e2e/detectors/snapshots/detectors__detector_ChainlinkFeedRegistry_0_8_20_chainlink_feed_registry_sol__0.txt @@ -0,0 +1,3 @@ +The Chainlink Feed Registry is used in the A contract. It's only available on Ethereum Mainnet, consider to not use it if the contract needs to be deployed on other chains. + - (None,price,None,None,None) = registry.latestRoundData(base,quote) (tests/e2e/detectors/test_data/chainlink-feed-registry/0.8.20/chainlink_feed_registry.sol#25) + diff --git a/tests/e2e/detectors/snapshots/detectors__detector_ChronicleUncheckedPrice_0_8_20_chronicle_unchecked_price_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_ChronicleUncheckedPrice_0_8_20_chronicle_unchecked_price_sol__0.txt new file mode 100644 index 000000000..6ddbfa4e5 --- /dev/null +++ b/tests/e2e/detectors/snapshots/detectors__detector_ChronicleUncheckedPrice_0_8_20_chronicle_unchecked_price_sol__0.txt @@ -0,0 +1,18 @@ +Chronicle price is not checked to be valid in C.bad2() (tests/e2e/detectors/test_data/chronicle-unchecked-price/0.8.20/chronicle_unchecked_price.sol#74-76) + - (price,None) = chronicle.readWithAge() (tests/e2e/detectors/test_data/chronicle-unchecked-price/0.8.20/chronicle_unchecked_price.sol#75) + +Chronicle price is not checked to be valid in C.bad() (tests/e2e/detectors/test_data/chronicle-unchecked-price/0.8.20/chronicle_unchecked_price.sol#65-67) + - price = chronicle.read() (tests/e2e/detectors/test_data/chronicle-unchecked-price/0.8.20/chronicle_unchecked_price.sol#66) + +Chronicle price is not checked to be valid in C.bad5() (tests/e2e/detectors/test_data/chronicle-unchecked-price/0.8.20/chronicle_unchecked_price.sol#101-103) + - price = scribe.latestAnswer() (tests/e2e/detectors/test_data/chronicle-unchecked-price/0.8.20/chronicle_unchecked_price.sol#102) + +Chronicle price is not checked to be valid in C.bad4() (tests/e2e/detectors/test_data/chronicle-unchecked-price/0.8.20/chronicle_unchecked_price.sol#92-94) + - (isValid,price,None) = chronicle.tryReadWithAge() (tests/e2e/detectors/test_data/chronicle-unchecked-price/0.8.20/chronicle_unchecked_price.sol#93) + +Chronicle price is not checked to be valid in C.bad3() (tests/e2e/detectors/test_data/chronicle-unchecked-price/0.8.20/chronicle_unchecked_price.sol#83-85) + - (isValid,price) = chronicle.tryRead() (tests/e2e/detectors/test_data/chronicle-unchecked-price/0.8.20/chronicle_unchecked_price.sol#84) + +Chronicle price is not checked to be valid in C.bad6() (tests/e2e/detectors/test_data/chronicle-unchecked-price/0.8.20/chronicle_unchecked_price.sol#110-112) + - (None,price,None,None,None) = scribe.latestRoundData() (tests/e2e/detectors/test_data/chronicle-unchecked-price/0.8.20/chronicle_unchecked_price.sol#111) + diff --git a/tests/e2e/detectors/snapshots/detectors__detector_GelatoUnprotectedRandomness_0_8_20_gelato_unprotected_randomness_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_GelatoUnprotectedRandomness_0_8_20_gelato_unprotected_randomness_sol__0.txt new file mode 100644 index 000000000..aee2ea4dd --- /dev/null +++ b/tests/e2e/detectors/snapshots/detectors__detector_GelatoUnprotectedRandomness_0_8_20_gelato_unprotected_randomness_sol__0.txt @@ -0,0 +1,6 @@ +C.bad() (tests/e2e/detectors/test_data/gelato-unprotected-randomness/0.8.20/gelato_unprotected_randomness.sol#42-44) is unprotected and request randomness from Gelato VRF + - id = _requestRandomness(abi.encode(msg.sender)) (tests/e2e/detectors/test_data/gelato-unprotected-randomness/0.8.20/gelato_unprotected_randomness.sol#43) + +C.good2() (tests/e2e/detectors/test_data/gelato-unprotected-randomness/0.8.20/gelato_unprotected_randomness.sol#51-54) is unprotected and request randomness from Gelato VRF + - id = _requestRandomness(abi.encode(msg.sender)) (tests/e2e/detectors/test_data/gelato-unprotected-randomness/0.8.20/gelato_unprotected_randomness.sol#53) + 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/snapshots/detectors__detector_PythUncheckedConfidence_0_8_20_pyth_unchecked_confidence_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_PythUncheckedConfidence_0_8_20_pyth_unchecked_confidence_sol__0.txt new file mode 100644 index 000000000..ae0dc2ae2 --- /dev/null +++ b/tests/e2e/detectors/snapshots/detectors__detector_PythUncheckedConfidence_0_8_20_pyth_unchecked_confidence_sol__0.txt @@ -0,0 +1,3 @@ +Pyth price conf field is not checked in C.bad(bytes32,uint256) (tests/e2e/detectors/test_data/pyth-unchecked-confidence/0.8.20/pyth_unchecked_confidence.sol#171-175) + - price = pyth.getEmaPriceNoOlderThan(id,age) (tests/e2e/detectors/test_data/pyth-unchecked-confidence/0.8.20/pyth_unchecked_confidence.sol#172) + diff --git a/tests/e2e/detectors/snapshots/detectors__detector_PythUncheckedPublishTime_0_8_20_pyth_unchecked_publishtime_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_PythUncheckedPublishTime_0_8_20_pyth_unchecked_publishtime_sol__0.txt new file mode 100644 index 000000000..cb331c8d5 --- /dev/null +++ b/tests/e2e/detectors/snapshots/detectors__detector_PythUncheckedPublishTime_0_8_20_pyth_unchecked_publishtime_sol__0.txt @@ -0,0 +1,3 @@ +Pyth price publishTime field is not checked in C.bad(bytes32) (tests/e2e/detectors/test_data/pyth-unchecked-publishtime/0.8.20/pyth_unchecked_publishtime.sol#171-175) + - price = pyth.getEmaPriceUnsafe(id) (tests/e2e/detectors/test_data/pyth-unchecked-publishtime/0.8.20/pyth_unchecked_publishtime.sol#172) + diff --git a/tests/e2e/detectors/test_data/chainlink-feed-registry/0.8.20/chainlink_feed_registry.sol b/tests/e2e/detectors/test_data/chainlink-feed-registry/0.8.20/chainlink_feed_registry.sol new file mode 100644 index 000000000..cf5d1ad4d --- /dev/null +++ b/tests/e2e/detectors/test_data/chainlink-feed-registry/0.8.20/chainlink_feed_registry.sol @@ -0,0 +1,37 @@ +interface FeedRegistryInterface { + function latestRoundData( + address base, + address quote + ) external view returns (uint80 roundId, int256 answer, uint256 startedAt, uint256 updatedAt, uint80 answeredInRound); +} + +interface MyInterface { + function latestRoundData( + address base, + address quote + ) external view returns (uint80 roundId, int256 answer, uint256 startedAt, uint256 updatedAt, uint80 answeredInRound); +} + +contract A { + FeedRegistryInterface public immutable registry; + MyInterface public immutable my_interface; + + constructor(FeedRegistryInterface _registry, MyInterface _my_interface) { + registry = _registry; + my_interface = _my_interface; + } + + function getPriceBad(address base, address quote) public returns (uint256) { + (, int256 price,,,) = registry.latestRoundData(base, quote); + // Do price validation + return uint256(price); + } + + function getPriceGood(address base, address quote) public returns (uint256) { + (, int256 price,,,) = my_interface.latestRoundData(base, quote); + // Do price validation + return uint256(price); + } + + +} \ No newline at end of file diff --git a/tests/e2e/detectors/test_data/chainlink-feed-registry/0.8.20/chainlink_feed_registry.sol-0.8.20.zip b/tests/e2e/detectors/test_data/chainlink-feed-registry/0.8.20/chainlink_feed_registry.sol-0.8.20.zip new file mode 100644 index 000000000..262ede23f Binary files /dev/null and b/tests/e2e/detectors/test_data/chainlink-feed-registry/0.8.20/chainlink_feed_registry.sol-0.8.20.zip differ diff --git a/tests/e2e/detectors/test_data/chronicle-unchecked-price/0.8.20/chronicle_unchecked_price.sol b/tests/e2e/detectors/test_data/chronicle-unchecked-price/0.8.20/chronicle_unchecked_price.sol new file mode 100644 index 000000000..e12560fa7 --- /dev/null +++ b/tests/e2e/detectors/test_data/chronicle-unchecked-price/0.8.20/chronicle_unchecked_price.sol @@ -0,0 +1,119 @@ +interface IChronicle { + /// @notice Returns the oracle's current value. + /// @dev Reverts if no value set. + /// @return value The oracle's current value. + function read() external view returns (uint value); + + /// @notice Returns the oracle's current value and its age. + /// @dev Reverts if no value set. + /// @return value The oracle's current value. + /// @return age The value's age. + function readWithAge() external view returns (uint value, uint age); + + /// @notice Returns the oracle's current value. + /// @return isValid True if value exists, false otherwise. + /// @return value The oracle's current value if it exists, zero otherwise. + function tryRead() external view returns (bool isValid, uint value); + + /// @notice Returns the oracle's current value and its age. + /// @return isValid True if value exists, false otherwise. + /// @return value The oracle's current value if it exists, zero otherwise. + /// @return age The value's age if value exists, zero otherwise. + function tryReadWithAge() + external + view + returns (bool isValid, uint value, uint age); +} + +interface IScribe is IChronicle { + /// @notice Returns the oracle's latest value. + /// @dev Provides partial compatibility with Chainlink's + /// IAggregatorV3Interface. + /// @return roundId 1. + /// @return answer The oracle's latest value. + /// @return startedAt 0. + /// @return updatedAt The timestamp of oracle's latest update. + /// @return answeredInRound 1. + function latestRoundData() + external + view + returns ( + uint80 roundId, + int answer, + uint startedAt, + uint updatedAt, + uint80 answeredInRound + ); + + /// @notice Returns the oracle's latest value. + /// @dev Provides partial compatibility with Chainlink's + /// IAggregatorV3Interface. + /// @custom:deprecated See https://docs.chain.link/data-feeds/api-reference/#latestanswer. + /// @return answer The oracle's latest value. + function latestAnswer() external view returns (int); +} + +contract C { + IScribe scribe; + IChronicle chronicle; + + constructor(address a) { + scribe = IScribe(a); + chronicle = IChronicle(a); + } + + function bad() public { + uint256 price = chronicle.read(); + } + + function good() public { + uint256 price = chronicle.read(); + require(price != 0); + } + + function bad2() public { + (uint256 price,) = chronicle.readWithAge(); + } + + function good2() public { + (uint256 price,) = chronicle.readWithAge(); + require(price != 0); + } + + function bad3() public { + (bool isValid, uint256 price) = chronicle.tryRead(); + } + + function good3() public { + (bool isValid, uint256 price) = chronicle.tryRead(); + require(isValid); + } + + function bad4() public { + (bool isValid, uint256 price,) = chronicle.tryReadWithAge(); + } + + function good4() public { + (bool isValid, uint256 price,) = chronicle.tryReadWithAge(); + require(isValid); + } + + function bad5() public { + int256 price = scribe.latestAnswer(); + } + + function good5() public { + int256 price = scribe.latestAnswer(); + require(price != 0); + } + + function bad6() public { + (, int256 price,,,) = scribe.latestRoundData(); + } + + function good6() public { + (, int256 price,,,) = scribe.latestRoundData(); + require(price != 0); + } + +} \ No newline at end of file diff --git a/tests/e2e/detectors/test_data/chronicle-unchecked-price/0.8.20/chronicle_unchecked_price.sol-0.8.20.zip b/tests/e2e/detectors/test_data/chronicle-unchecked-price/0.8.20/chronicle_unchecked_price.sol-0.8.20.zip new file mode 100644 index 000000000..746efabf6 Binary files /dev/null and b/tests/e2e/detectors/test_data/chronicle-unchecked-price/0.8.20/chronicle_unchecked_price.sol-0.8.20.zip differ diff --git a/tests/e2e/detectors/test_data/gelato-unprotected-randomness/0.8.20/gelato_unprotected_randomness.sol b/tests/e2e/detectors/test_data/gelato-unprotected-randomness/0.8.20/gelato_unprotected_randomness.sol new file mode 100644 index 000000000..108859e9e --- /dev/null +++ b/tests/e2e/detectors/test_data/gelato-unprotected-randomness/0.8.20/gelato_unprotected_randomness.sol @@ -0,0 +1,62 @@ +// Mock GelatoVRFConsumerBase for what we need +abstract contract GelatoVRFConsumerBase { + bool[] public requestPending; + mapping(uint256 => bytes32) public requestedHash; + + function _fulfillRandomness( + uint256 randomness, + uint256 requestId, + bytes memory extraData + ) internal virtual; + + function _requestRandomness( + bytes memory extraData + ) internal returns (uint256 requestId) { + requestId = uint256(requestPending.length); + requestPending.push(); + requestPending[requestId] = true; + + bytes memory data = abi.encode(requestId, extraData); + uint256 round = 111; + + bytes memory dataWithRound = abi.encode(round, data); + bytes32 requestHash = keccak256(dataWithRound); + + requestedHash[requestId] = requestHash; + } + +} + +contract C is GelatoVRFConsumerBase { + address owner; + mapping(address => bool) authorized; + + function _fulfillRandomness( + uint256 randomness, + uint256, + bytes memory extraData + ) internal override { + // Do something with the random number + } + + function bad() public { + uint id = _requestRandomness(abi.encode(msg.sender)); + } + + function good() public { + require(msg.sender == owner); + uint id = _requestRandomness(abi.encode(msg.sender)); + } + + // This is currently a FP due to the limitation of function.is_protected + function good2() public { + require(authorized[msg.sender]); + uint id = _requestRandomness(abi.encode(msg.sender)); + } + + function good3() public { + if (msg.sender != owner) { revert(); } + uint id = _requestRandomness(abi.encode(msg.sender)); + } + +} diff --git a/tests/e2e/detectors/test_data/gelato-unprotected-randomness/0.8.20/gelato_unprotected_randomness.sol-0.8.20.zip b/tests/e2e/detectors/test_data/gelato-unprotected-randomness/0.8.20/gelato_unprotected_randomness.sol-0.8.20.zip new file mode 100644 index 000000000..013d3ef28 Binary files /dev/null and b/tests/e2e/detectors/test_data/gelato-unprotected-randomness/0.8.20/gelato_unprotected_randomness.sol-0.8.20.zip differ 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_data/pyth-unchecked-confidence/0.8.20/pyth_unchecked_confidence.sol b/tests/e2e/detectors/test_data/pyth-unchecked-confidence/0.8.20/pyth_unchecked_confidence.sol new file mode 100644 index 000000000..58880c382 --- /dev/null +++ b/tests/e2e/detectors/test_data/pyth-unchecked-confidence/0.8.20/pyth_unchecked_confidence.sol @@ -0,0 +1,193 @@ +contract PythStructs { + // A price with a degree of uncertainty, represented as a price +- a confidence interval. + // + // The confidence interval roughly corresponds to the standard error of a normal distribution. + // Both the price and confidence are stored in a fixed-point numeric representation, + // `x * (10^expo)`, where `expo` is the exponent. + // + // Please refer to the documentation at https://docs.pyth.network/consumers/best-practices for how + // to how this price safely. + struct Price { + // Price + int64 price; + // Confidence interval around the price + uint64 conf; + // Price exponent + int32 expo; + // Unix timestamp describing when the price was published + uint publishTime; + } + + // PriceFeed represents a current aggregate price from pyth publisher feeds. + struct PriceFeed { + // The price ID. + bytes32 id; + // Latest available price + Price price; + // Latest available exponentially-weighted moving average price + Price emaPrice; + } +} + +interface IPyth { + /// @notice Returns the period (in seconds) that a price feed is considered valid since its publish time + function getValidTimePeriod() external view returns (uint validTimePeriod); + + /// @notice Returns the price and confidence interval. + /// @dev Reverts if the price has not been updated within the last `getValidTimePeriod()` seconds. + /// @param id The Pyth Price Feed ID of which to fetch the price and confidence interval. + /// @return price - please read the documentation of PythStructs.Price to understand how to use this safely. + function getPrice( + bytes32 id + ) external view returns (PythStructs.Price memory price); + + /// @notice Returns the exponentially-weighted moving average price and confidence interval. + /// @dev Reverts if the EMA price is not available. + /// @param id The Pyth Price Feed ID of which to fetch the EMA price and confidence interval. + /// @return price - please read the documentation of PythStructs.Price to understand how to use this safely. + function getEmaPrice( + bytes32 id + ) external view returns (PythStructs.Price memory price); + + /// @notice Returns the price of a price feed without any sanity checks. + /// @dev This function returns the most recent price update in this contract without any recency checks. + /// This function is unsafe as the returned price update may be arbitrarily far in the past. + /// + /// Users of this function should check the `publishTime` in the price to ensure that the returned price is + /// sufficiently recent for their application. If you are considering using this function, it may be + /// safer / easier to use either `getPrice` or `getPriceNoOlderThan`. + /// @return price - please read the documentation of PythStructs.Price to understand how to use this safely. + function getPriceUnsafe( + bytes32 id + ) external view returns (PythStructs.Price memory price); + + /// @notice Returns the price that is no older than `age` seconds of the current time. + /// @dev This function is a sanity-checked version of `getPriceUnsafe` which is useful in + /// applications that require a sufficiently-recent price. Reverts if the price wasn't updated sufficiently + /// recently. + /// @return price - please read the documentation of PythStructs.Price to understand how to use this safely. + function getPriceNoOlderThan( + bytes32 id, + uint age + ) external view returns (PythStructs.Price memory price); + + /// @notice Returns the exponentially-weighted moving average price of a price feed without any sanity checks. + /// @dev This function returns the same price as `getEmaPrice` in the case where the price is available. + /// However, if the price is not recent this function returns the latest available price. + /// + /// The returned price can be from arbitrarily far in the past; this function makes no guarantees that + /// the returned price is recent or useful for any particular application. + /// + /// Users of this function should check the `publishTime` in the price to ensure that the returned price is + /// sufficiently recent for their application. If you are considering using this function, it may be + /// safer / easier to use either `getEmaPrice` or `getEmaPriceNoOlderThan`. + /// @return price - please read the documentation of PythStructs.Price to understand how to use this safely. + function getEmaPriceUnsafe( + bytes32 id + ) external view returns (PythStructs.Price memory price); + + /// @notice Returns the exponentially-weighted moving average price that is no older than `age` seconds + /// of the current time. + /// @dev This function is a sanity-checked version of `getEmaPriceUnsafe` which is useful in + /// applications that require a sufficiently-recent price. Reverts if the price wasn't updated sufficiently + /// recently. + /// @return price - please read the documentation of PythStructs.Price to understand how to use this safely. + function getEmaPriceNoOlderThan( + bytes32 id, + uint age + ) external view returns (PythStructs.Price memory price); + + /// @notice Update price feeds with given update messages. + /// This method requires the caller to pay a fee in wei; the required fee can be computed by calling + /// `getUpdateFee` with the length of the `updateData` array. + /// Prices will be updated if they are more recent than the current stored prices. + /// The call will succeed even if the update is not the most recent. + /// @dev Reverts if the transferred fee is not sufficient or the updateData is invalid. + /// @param updateData Array of price update data. + function updatePriceFeeds(bytes[] calldata updateData) external payable; + + /// @notice Wrapper around updatePriceFeeds that rejects fast if a price update is not necessary. A price update is + /// necessary if the current on-chain publishTime is older than the given publishTime. It relies solely on the + /// given `publishTimes` for the price feeds and does not read the actual price update publish time within `updateData`. + /// + /// This method requires the caller to pay a fee in wei; the required fee can be computed by calling + /// `getUpdateFee` with the length of the `updateData` array. + /// + /// `priceIds` and `publishTimes` are two arrays with the same size that correspond to senders known publishTime + /// of each priceId when calling this method. If all of price feeds within `priceIds` have updated and have + /// a newer or equal publish time than the given publish time, it will reject the transaction to save gas. + /// Otherwise, it calls updatePriceFeeds method to update the prices. + /// + /// @dev Reverts if update is not needed or the transferred fee is not sufficient or the updateData is invalid. + /// @param updateData Array of price update data. + /// @param priceIds Array of price ids. + /// @param publishTimes Array of publishTimes. `publishTimes[i]` corresponds to known `publishTime` of `priceIds[i]` + function updatePriceFeedsIfNecessary( + bytes[] calldata updateData, + bytes32[] calldata priceIds, + uint64[] calldata publishTimes + ) external payable; + + /// @notice Returns the required fee to update an array of price updates. + /// @param updateData Array of price update data. + /// @return feeAmount The required fee in Wei. + function getUpdateFee( + bytes[] calldata updateData + ) external view returns (uint feeAmount); + + /// @notice Parse `updateData` and return price feeds of the given `priceIds` if they are all published + /// within `minPublishTime` and `maxPublishTime`. + /// + /// You can use this method if you want to use a Pyth price at a fixed time and not the most recent price; + /// otherwise, please consider using `updatePriceFeeds`. This method does not store the price updates on-chain. + /// + /// This method requires the caller to pay a fee in wei; the required fee can be computed by calling + /// `getUpdateFee` with the length of the `updateData` array. + /// + /// + /// @dev Reverts if the transferred fee is not sufficient or the updateData is invalid or there is + /// no update for any of the given `priceIds` within the given time range. + /// @param updateData Array of price update data. + /// @param priceIds Array of price ids. + /// @param minPublishTime minimum acceptable publishTime for the given `priceIds`. + /// @param maxPublishTime maximum acceptable publishTime for the given `priceIds`. + /// @return priceFeeds Array of the price feeds corresponding to the given `priceIds` (with the same order). + function parsePriceFeedUpdates( + bytes[] calldata updateData, + bytes32[] calldata priceIds, + uint64 minPublishTime, + uint64 maxPublishTime + ) external payable returns (PythStructs.PriceFeed[] memory priceFeeds); +} + + +contract C { + IPyth pyth; + + constructor(IPyth _pyth) { + pyth = _pyth; + } + + function bad(bytes32 id, uint256 age) public { + PythStructs.Price memory price = pyth.getEmaPriceNoOlderThan(id, age); + require(price.publishTime > block.timestamp - 120); + // Use price + } + + function good(bytes32 id, uint256 age) public { + PythStructs.Price memory price = pyth.getEmaPriceNoOlderThan(id, age); + require(price.conf < 10000); + require(price.publishTime > block.timestamp - 120); + // Use price + } + + function good2(bytes32 id, uint256 age) public { + PythStructs.Price memory price = pyth.getEmaPriceNoOlderThan(id, age); + require(price.publishTime > block.timestamp - 120); + if (price.conf >= 10000) { + revert(); + } + // Use price + } + +} \ No newline at end of file diff --git a/tests/e2e/detectors/test_data/pyth-unchecked-confidence/0.8.20/pyth_unchecked_confidence.sol-0.8.20.zip b/tests/e2e/detectors/test_data/pyth-unchecked-confidence/0.8.20/pyth_unchecked_confidence.sol-0.8.20.zip new file mode 100644 index 000000000..6e5fa1b9f Binary files /dev/null and b/tests/e2e/detectors/test_data/pyth-unchecked-confidence/0.8.20/pyth_unchecked_confidence.sol-0.8.20.zip differ diff --git a/tests/e2e/detectors/test_data/pyth-unchecked-publishtime/0.8.20/pyth_unchecked_publishtime.sol b/tests/e2e/detectors/test_data/pyth-unchecked-publishtime/0.8.20/pyth_unchecked_publishtime.sol new file mode 100644 index 000000000..74ab10fe3 --- /dev/null +++ b/tests/e2e/detectors/test_data/pyth-unchecked-publishtime/0.8.20/pyth_unchecked_publishtime.sol @@ -0,0 +1,193 @@ +contract PythStructs { + // A price with a degree of uncertainty, represented as a price +- a confidence interval. + // + // The confidence interval roughly corresponds to the standard error of a normal distribution. + // Both the price and confidence are stored in a fixed-point numeric representation, + // `x * (10^expo)`, where `expo` is the exponent. + // + // Please refer to the documentation at https://docs.pyth.network/consumers/best-practices for how + // to how this price safely. + struct Price { + // Price + int64 price; + // Confidence interval around the price + uint64 conf; + // Price exponent + int32 expo; + // Unix timestamp describing when the price was published + uint publishTime; + } + + // PriceFeed represents a current aggregate price from pyth publisher feeds. + struct PriceFeed { + // The price ID. + bytes32 id; + // Latest available price + Price price; + // Latest available exponentially-weighted moving average price + Price emaPrice; + } +} + +interface IPyth { + /// @notice Returns the period (in seconds) that a price feed is considered valid since its publish time + function getValidTimePeriod() external view returns (uint validTimePeriod); + + /// @notice Returns the price and confidence interval. + /// @dev Reverts if the price has not been updated within the last `getValidTimePeriod()` seconds. + /// @param id The Pyth Price Feed ID of which to fetch the price and confidence interval. + /// @return price - please read the documentation of PythStructs.Price to understand how to use this safely. + function getPrice( + bytes32 id + ) external view returns (PythStructs.Price memory price); + + /// @notice Returns the exponentially-weighted moving average price and confidence interval. + /// @dev Reverts if the EMA price is not available. + /// @param id The Pyth Price Feed ID of which to fetch the EMA price and confidence interval. + /// @return price - please read the documentation of PythStructs.Price to understand how to use this safely. + function getEmaPrice( + bytes32 id + ) external view returns (PythStructs.Price memory price); + + /// @notice Returns the price of a price feed without any sanity checks. + /// @dev This function returns the most recent price update in this contract without any recency checks. + /// This function is unsafe as the returned price update may be arbitrarily far in the past. + /// + /// Users of this function should check the `publishTime` in the price to ensure that the returned price is + /// sufficiently recent for their application. If you are considering using this function, it may be + /// safer / easier to use either `getPrice` or `getPriceNoOlderThan`. + /// @return price - please read the documentation of PythStructs.Price to understand how to use this safely. + function getPriceUnsafe( + bytes32 id + ) external view returns (PythStructs.Price memory price); + + /// @notice Returns the price that is no older than `age` seconds of the current time. + /// @dev This function is a sanity-checked version of `getPriceUnsafe` which is useful in + /// applications that require a sufficiently-recent price. Reverts if the price wasn't updated sufficiently + /// recently. + /// @return price - please read the documentation of PythStructs.Price to understand how to use this safely. + function getPriceNoOlderThan( + bytes32 id, + uint age + ) external view returns (PythStructs.Price memory price); + + /// @notice Returns the exponentially-weighted moving average price of a price feed without any sanity checks. + /// @dev This function returns the same price as `getEmaPrice` in the case where the price is available. + /// However, if the price is not recent this function returns the latest available price. + /// + /// The returned price can be from arbitrarily far in the past; this function makes no guarantees that + /// the returned price is recent or useful for any particular application. + /// + /// Users of this function should check the `publishTime` in the price to ensure that the returned price is + /// sufficiently recent for their application. If you are considering using this function, it may be + /// safer / easier to use either `getEmaPrice` or `getEmaPriceNoOlderThan`. + /// @return price - please read the documentation of PythStructs.Price to understand how to use this safely. + function getEmaPriceUnsafe( + bytes32 id + ) external view returns (PythStructs.Price memory price); + + /// @notice Returns the exponentially-weighted moving average price that is no older than `age` seconds + /// of the current time. + /// @dev This function is a sanity-checked version of `getEmaPriceUnsafe` which is useful in + /// applications that require a sufficiently-recent price. Reverts if the price wasn't updated sufficiently + /// recently. + /// @return price - please read the documentation of PythStructs.Price to understand how to use this safely. + function getEmaPriceNoOlderThan( + bytes32 id, + uint age + ) external view returns (PythStructs.Price memory price); + + /// @notice Update price feeds with given update messages. + /// This method requires the caller to pay a fee in wei; the required fee can be computed by calling + /// `getUpdateFee` with the length of the `updateData` array. + /// Prices will be updated if they are more recent than the current stored prices. + /// The call will succeed even if the update is not the most recent. + /// @dev Reverts if the transferred fee is not sufficient or the updateData is invalid. + /// @param updateData Array of price update data. + function updatePriceFeeds(bytes[] calldata updateData) external payable; + + /// @notice Wrapper around updatePriceFeeds that rejects fast if a price update is not necessary. A price update is + /// necessary if the current on-chain publishTime is older than the given publishTime. It relies solely on the + /// given `publishTimes` for the price feeds and does not read the actual price update publish time within `updateData`. + /// + /// This method requires the caller to pay a fee in wei; the required fee can be computed by calling + /// `getUpdateFee` with the length of the `updateData` array. + /// + /// `priceIds` and `publishTimes` are two arrays with the same size that correspond to senders known publishTime + /// of each priceId when calling this method. If all of price feeds within `priceIds` have updated and have + /// a newer or equal publish time than the given publish time, it will reject the transaction to save gas. + /// Otherwise, it calls updatePriceFeeds method to update the prices. + /// + /// @dev Reverts if update is not needed or the transferred fee is not sufficient or the updateData is invalid. + /// @param updateData Array of price update data. + /// @param priceIds Array of price ids. + /// @param publishTimes Array of publishTimes. `publishTimes[i]` corresponds to known `publishTime` of `priceIds[i]` + function updatePriceFeedsIfNecessary( + bytes[] calldata updateData, + bytes32[] calldata priceIds, + uint64[] calldata publishTimes + ) external payable; + + /// @notice Returns the required fee to update an array of price updates. + /// @param updateData Array of price update data. + /// @return feeAmount The required fee in Wei. + function getUpdateFee( + bytes[] calldata updateData + ) external view returns (uint feeAmount); + + /// @notice Parse `updateData` and return price feeds of the given `priceIds` if they are all published + /// within `minPublishTime` and `maxPublishTime`. + /// + /// You can use this method if you want to use a Pyth price at a fixed time and not the most recent price; + /// otherwise, please consider using `updatePriceFeeds`. This method does not store the price updates on-chain. + /// + /// This method requires the caller to pay a fee in wei; the required fee can be computed by calling + /// `getUpdateFee` with the length of the `updateData` array. + /// + /// + /// @dev Reverts if the transferred fee is not sufficient or the updateData is invalid or there is + /// no update for any of the given `priceIds` within the given time range. + /// @param updateData Array of price update data. + /// @param priceIds Array of price ids. + /// @param minPublishTime minimum acceptable publishTime for the given `priceIds`. + /// @param maxPublishTime maximum acceptable publishTime for the given `priceIds`. + /// @return priceFeeds Array of the price feeds corresponding to the given `priceIds` (with the same order). + function parsePriceFeedUpdates( + bytes[] calldata updateData, + bytes32[] calldata priceIds, + uint64 minPublishTime, + uint64 maxPublishTime + ) external payable returns (PythStructs.PriceFeed[] memory priceFeeds); +} + + +contract C { + IPyth pyth; + + constructor(IPyth _pyth) { + pyth = _pyth; + } + + function bad(bytes32 id) public { + PythStructs.Price memory price = pyth.getEmaPriceUnsafe(id); + require(price.conf < 10000); + // Use price + } + + function good(bytes32 id) public { + PythStructs.Price memory price = pyth.getEmaPriceUnsafe(id); + require(price.publishTime > block.timestamp - 120); + require(price.conf < 10000); + // Use price + } + + function good2(bytes32 id) public { + PythStructs.Price memory price = pyth.getEmaPriceUnsafe(id); + require(price.conf < 10000); + if (price.publishTime <= block.timestamp - 120) { + revert(); + } + // Use price + } + +} \ No newline at end of file diff --git a/tests/e2e/detectors/test_data/pyth-unchecked-publishtime/0.8.20/pyth_unchecked_publishtime.sol-0.8.20.zip b/tests/e2e/detectors/test_data/pyth-unchecked-publishtime/0.8.20/pyth_unchecked_publishtime.sol-0.8.20.zip new file mode 100644 index 000000000..178b65b38 Binary files /dev/null and b/tests/e2e/detectors/test_data/pyth-unchecked-publishtime/0.8.20/pyth_unchecked_publishtime.sol-0.8.20.zip differ diff --git a/tests/e2e/detectors/test_detectors.py b/tests/e2e/detectors/test_detectors.py index cd9ef2c02..74a68cf63 100644 --- a/tests/e2e/detectors/test_detectors.py +++ b/tests/e2e/detectors/test_detectors.py @@ -1719,6 +1719,41 @@ ALL_TESTS = [ "out_of_order_retryable.sol", "0.8.20", ), + Test( + all_detectors.GelatoUnprotectedRandomness, + "gelato_unprotected_randomness.sol", + "0.8.20", + ), + Test( + all_detectors.ChronicleUncheckedPrice, + "chronicle_unchecked_price.sol", + "0.8.20", + ), + Test( + all_detectors.PythUncheckedConfidence, + "pyth_unchecked_confidence.sol", + "0.8.20", + ), + Test( + all_detectors.PythUncheckedPublishTime, + "pyth_unchecked_publishtime.sol", + "0.8.20", + ), + Test( + all_detectors.ChainlinkFeedRegistry, + "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",