From d96b87e140a102da86703ff16882323abeb15850 Mon Sep 17 00:00:00 2001 From: Simone Date: Fri, 4 Oct 2024 19:52:08 +0200 Subject: [PATCH 01/18] Add Optimism deprecation detector --- slither/detectors/all_detectors.py | 1 + .../functions/optimism_deprecation.py | 91 ++++++++++++++++++ ...ion_0_8_20_optimism_deprecation_sol__0.txt | 4 + .../0.8.20/optimism_deprecation.sol | 27 ++++++ .../optimism_deprecation.sol-0.8.20.zip | Bin 0 -> 3060 bytes tests/e2e/detectors/test_detectors.py | 5 + 6 files changed, 128 insertions(+) create mode 100644 slither/detectors/functions/optimism_deprecation.py create mode 100644 tests/e2e/detectors/snapshots/detectors__detector_OptimismDeprecation_0_8_20_optimism_deprecation_sol__0.txt create mode 100644 tests/e2e/detectors/test_data/optimism-deprecation/0.8.20/optimism_deprecation.sol create mode 100644 tests/e2e/detectors/test_data/optimism-deprecation/0.8.20/optimism_deprecation.sol-0.8.20.zip diff --git a/slither/detectors/all_detectors.py b/slither/detectors/all_detectors.py index 44a168c2b..0222608a1 100644 --- a/slither/detectors/all_detectors.py +++ b/slither/detectors/all_detectors.py @@ -97,5 +97,6 @@ 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.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..426ec7949 --- /dev/null +++ b/slither/detectors/functions/optimism_deprecation.py @@ -0,0 +1,91 @@ +from typing import List + +from slither.detectors.abstract_detector import ( + AbstractDetector, + DetectorClassification, + DETECTOR_INFO, +) +from slither.core.cfg.node import Node +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 = "D." + + 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.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/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/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 0000000000000000000000000000000000000000..de18d4a0dde56342b0a92296cedfe5b567d7d705 GIT binary patch literal 3060 zcmb8xX&@7f0|xNPT(7xDnF*63I+WKjxoOyPjNI2S%r(bK*<8tyE7x4fohTI(nKhxQ zmt1p?NI8?t8Abei|KI-~{?GH_`Sg7MSs^)rdVpg9K>!cQ*fDOLs}ar*0DQ0p08{_~ z06v6(3&Mp5xqD+n!mwT#0uCR1J{%wDj=dFv4Mx+z3i3@K3$sQDp$~(3+xi$*;MEIn1q$rKC>C0KnYPvxWH7yzd@WBqM(mPSERB;> z-_*HuZNrr>OI|;cZL?}2cp@U)>@-Xv+F+mOIVlfayj6K=K;IOP`Xa?LCs&@iy%@Rw z%4lT4(U!#4td6%A>lr#CNP`%0MKrhwQ#LDLjviJ2LF6av zM66t8{#HRQB~)=(m@ZAQsAo(pXO0^Qt*@3)W9X02bxTHgfwK9!P7UD1Cgn|IuEp0+ zd1TiB*3}CQy2;dPg`u{4_{D@#(cj>M)W+s6+dK-=x{WS$RSwX~wQ4(kJLnvbcz)o< zL$!q*+@q_e z4L5L3Pnjwyxi+GN%*GS#qsW6k-jax1ttN}0YOdi9uZ8+U`GYuVl*nKw%(^ti*=e$; ztd`yq0pKuSjms1GlH91fOCwm<^NgFJ6k^YxKi+JT@>$x4N`3$ zkw#bHuk@r|T6K8e94(`M)H2$_lrMRC%)SGnfr|we-K6r&j+d9Xo!?EaFo=Z+0C|GH z)-xY|{8=P~M(Xw9$>57eSs&Ryhk6`VTZc36Rq(0n4QJ=njviTNKjBIPFOvk(xPKCL zx57-ANwLioKjWFUCU)+1 z_IqtcS9qfTt^}H$#z2bLR|K-2#9K;cpKk$O9}`Leze*03B=@^y>6`R3qG_R?0>P(2 zBVn#pRigcM+R2|pRjz)ffQl?9mlj)$+|%&7Xf_o&p`NX2Z$9Mi7wFxobK)yZ=CPXH z`qBJx=+Jrgs#EE~t;zx3^j#FVZAuIl>xxR2=xYD>>0s>wIp>{iMK z92l=S^qtCXYx)S|dzBnamh|il#Z&TWA6XXjrZLi>@sIQEKd<6wiNa3YyzohSJ!#)0 zFoff56&mGo?I^Xwz9s_=c@Kw2Nw10q6ym;(ai0mgpC^L=llUOViJ3!o!7gR2H6c-w zU4*78``NC?Skl(=5MU zFxNl+GfpbsC)beOQJ~0Uo)~<6?bg%5j?+nX7H(x_4z}DNSeU*oiKCh1Z1el}BF1H6 zV8!N>Qj?qo1sJciznK+x*c7dMFo}zs3i2U9t`6aO3>~7{O)^+LHzvQ=v%I}fA5kAd~9s-T(vSQFKs zE7K=0^(+^B%JA{q78 zr$j^4a{*Xn>Uh3#Sy5wUy}~eg{0{OgwLeJg_nX^KitS6M)7qY5A!7&k3DgtFw6&%9 z>A(N>N~jY*{8~O!tv@MV9%kI8d)Jg`cr*$o=a?pk(;q$AJ(F{@El&BI>!7jQG94laB5xj-udX71P% zHP1(1xv{|++N;{lLRA^Fat$=M<#aAG?Av}In*b@?KbqfpEGk$QyolkB!ruSiOY(TA3oeyvqksY48G@^Z8vlX8CH!^nHky3rMexmUQslc3I{SG z-R}K{Gel_%^!d1tms{MfGQo@?j~TGL?9w*)OTW#)&Y<|p6=v9@LYXV>F6l4tPr?(e zPu&JWsDjX#yUC9^yGLbuj?&$f@5^11$jC^bAcWD|Zt0AYMA*O=Y1HI%h06|E>8Fd% z7iaBBPmAk!E4?w#(0K{IRKFRYQwohp+qoyceuFQ!8?a-m%R>5Kc#2Y z*D;f|tB&D!?OSk^XjX1in;h~^UmRgP%!Qgh-P(a;#3K&x*xSSz0Z? z@Ch!6hxk!`YsH7XQWrCR_IiVr0l21MOHl5WyI;HBGtRQkW0UmnM4n)E5&cU|7;;cQc*gi_hbG2ELvs?e@pEim$;Mynl%E zU#?O_8Et9FvF732$#3muH8XmX6nk>1&8?4R?Qhy`apVEX;Ny|%o!G}NltsIYk{#|b z2J!J;uG!(q#0uADOes`$26+87-L^zn(vtty40-lwd}sEe!0}F_Oy7Etz!a}V1lqoVxA} Date: Fri, 4 Oct 2024 19:55:24 +0200 Subject: [PATCH 02/18] Add Chainlink feed registry detector --- slither/detectors/all_detectors.py | 1 + .../functions/chainlink_feed_registry.py | 103 ++++++++++++++++++ ..._0_8_20_chainlink_feed_registry_sol__0.txt | 3 + .../0.8.20/chainlink_feed_registry.sol | 37 +++++++ .../chainlink_feed_registry.sol-0.8.20.zip | Bin 0 -> 5853 bytes tests/e2e/detectors/test_detectors.py | 5 + 6 files changed, 149 insertions(+) create mode 100644 slither/detectors/functions/chainlink_feed_registry.py create mode 100644 tests/e2e/detectors/snapshots/detectors__detector_ChainlinkFeedRegistry_0_8_20_chainlink_feed_registry_sol__0.txt create mode 100644 tests/e2e/detectors/test_data/chainlink-feed-registry/0.8.20/chainlink_feed_registry.sol create mode 100644 tests/e2e/detectors/test_data/chainlink-feed-registry/0.8.20/chainlink_feed_registry.sol-0.8.20.zip diff --git a/slither/detectors/all_detectors.py b/slither/detectors/all_detectors.py index 44a168c2b..0b93b9d75 100644 --- a/slither/detectors/all_detectors.py +++ b/slither/detectors/all_detectors.py @@ -97,5 +97,6 @@ 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.chainlink_feed_registry import ChainlinkFeedRegistry # 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..951dfb584 --- /dev/null +++ b/slither/detectors/functions/chainlink_feed_registry.py @@ -0,0 +1,103 @@ +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/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/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 0000000000000000000000000000000000000000..262ede23f1aed4f717506adaae6be3b1b7dabd49 GIT binary patch literal 5853 zcmb8zMO+jNxHa%$XkqB??ixC!yAhBW>F)0CP6;XLA*H)fq`N@@>24_p=6mnAzl-~v z#j`s5^ZRS5A|gowfB-CjdZ3&ko!-$dB@qB%Q4auc0{{R^J9B#%Cwmu1QyXh*D^m|^ zTYFD04}UICS0__zKQ~toFH>_5OFMfXYc2;*R~K|7WWXl?APfMIh>Nr3gYmhSEX=fv zfeJ=k>=_5BqLX#V-czr7_7Q>q%Dzw8Bsa(1JLtT{&@W2;t3-C!$gOjko1+B9Szq#U zFLXicpZ=jmaL)Z5l&kO2tReE0rc3cijqj7Z!Pu?WCd^_-dEs?^g5`(4ou41kcyQqk zJ${{fnm-4Q{n8X24oCkUk}46PpUE~f7wm_y0QWI1?;ITBK_cnbCmxwuqWsZng#55P zstTPsL%T2D51Y+mr7QPZ@jPjes!c`&ZI+}P;r#;5M$9=a+ zU&q~Ft0sD`x?Eov#XOhklVvMCk?Fyp6KJz9A3v$Zb-;0ZsIF!xNCfRWflSgHR3*v9 z;l|q_FM2fJ&r8_bhA5taQg@88Xv2!sfmkw%tkO(+4C;@?0ODxx5fUl42U)*5LRto#?V+%$hyBJZ5yd4J@^WZUrshV1ud-16I|>` zr--~&jGZ2KU1WW4o#XW_Gq%8lUZ~6XayzwFT9ut6* zUW~>vDLWKCt*ar{neiQjJ@t`@tFQXe!6o!EQo7U)QOXK=L0Jt+H6ARoPp5g#8Zfs4 zi0bx<%wo8~!*1z^f806fa#L9~x-;cDWQ%X(OT`js#?%*+`Bs;%kbN|i5_+`0x)E;T z*dF*87?(Udr+L-Th6Gux_kaARpFHbb{8DlZDLT2cSAS_>`}YOm=T@^*1F07hl;gcW7HLR zhA{0lxHfeooy@rnVT32uarOHK#knKwld0&tu<11-_vHS>j*XUYo4Kkm35g9Fm_&JI z5PFtZ_WK>m#v;b(-Ok`W1zO@t`~tg~T%zx64k*P`6-keJsKvTOLXZxxdZ{E!lRtMM zv}_L!NbsRMCX%u=(thT0TAANEZ{!pjdipqG1|+-tc7{Z296@G>ZFZ7``V-UqU%D-Q z{eYZt*|NiPJ?C0x(DE&fxp|HfnZAPO%b9>q^X^XMKuaO(xfD zlzIZ0q(=mypgBeT=6u|3NP znSu-oAf}ol&LWUd^Uq7~$3yrokLace6N~{6CTR%<|1q5wzP|7Y(hQxo>$T;etVgaO zE%%=dfhf2eO#t5F8S?jF?P|u}yUZ7Hdz$hAJv?*`Es$1(Xt+Ujsm0H>FGn8r(6oDv zsy$cSK52?uRk{-+{LKd3ux!lTo6fb9rCr#_A~ouca}Xu-XtE7k3w6<=r^ z?~Xb<1gREa890Wz4Grh@iTToK9UuD-Z<;0Lc+dPait!O@j|AzmMEQ6WQq(rO)0|MU z-q0j|_qW%oUKQX_vN|J3Sh1BY!C9mkjyzu~ooQPwZadZx;w|UqIl`F%jJblfpZw|nGUpSKXN{f0|1 zs@!V&D?Son*~fQZ>w&A9iO#)6dKIL`J@I9=R1!c|iXdC2$tr5LmMPX)aibup_)?Sbn4YO4v351VD83)x*+0nJG7y>Kiiacz*df z_d#GNVG^IfODDLT+EU-=pIjBqw0}{hVK=NtXN#bTECl=2FT~uZ zOHTaR&C0~YKS_H=IOiD1Dw@Y9i=7eeM2d~PAEF0%=TxuI>_FBaqU~Xw`&33z*f745c^Ff@MPi#9( zn9rE=r7)X6N)%J3p1HA4Sx-gUs5aMdsiB?&N8OB3n`z5EpM?L*51&*8v^-UFP@>PX zKyrom!-U<1#kIITBFWa^U&Xf}99XLBAu&2*4-fmVlyH3wv+ar{r3}Hkih5T9YkUox zqO&AC67v)l`vUCEy11V9Zj91|ovrU5E884Jws6fZx2=gKG?sXp;>iRZ%v^c;N-)gG>#gORo?uL_9Lni`Hm70?^qs+P))70{m1 zpE}w9Ws0$5Qfw$RqY#sdKCQ1fo^%G4^)FMJ)L}zv z-J_1k#Vpa*=8T0hO5Jq$BTyDIT>^>L#NFh&(DGA`)Qk2?{xAb0MA*oJ^Od$FDVr)1 zo=`AN(TyE0_$*?SY)351+l(n| z?`fAK@JK`mlhr;jAGUk=Q%J04ZgMHj6A6$bB9nCKxi_SShrQg{_MPsMZ=Df$+?+_X z{FOwAv!*|7YVoH3l%%55zD&wCpEEU>G56&(w7Yn1UZY}C;aKN&Z|)Gb0fpGMAm?-w z)@a@xZnWkdnc3&$wGsAmBHR!w1%PZ0XOm(X!1Zg*gZvLOC!Z%J*=Zk7OhC;aoGpK$ zd?+^Akoc)#-`n88N=IJ;R*$W$L6!_Omli1;-BNZUf8P+)YpqDgb!cdK*6{j#mpsNT z{2}~$PBPrfAeh!bo4Ia_!2SYN1`Gnn?}78C?ONr#dnnNT$!&ax&ULuE^X8YOV$xa_ z^%0If6C3t)&hcG17IDlRLlaB;Y^hTW72to@+O;4$FFLwb{XMArz7@AK@%`|c&f#G8 z@*j`!>X>}l#a8EDiP@vDnnFgb*~p@iKz1w0Fu3E^vk^d@#Wlal&i0AnA6bS_gnBZ_ zr1Spi&WLX0-4itol9h^+wG0uZ)kQ?Go#;B4wvzXap}U*yNc2fRtE?D!5eZ{G+7yW^ znZSvIeysu@_$Rrd4s16I<&wLt3#!`KxL7v4rGDOz0C0OYis-W)XVZg9XXx3&${PdQ z(>#_69wsM9M1j`#gCEX1A9-niz#=5La=XtMZQRFfXIAhCno=Ke{;0~gxopX#iOHw0 z-wyxdzdsE{X*#8BdAK0?iWHgsi9vBI#3*(BB_+o7oi`xX*It@v7q>WFzP$}d=Ya%o}`l5B!+;8kaOSG$fx3;V``Gqm&3nnS%D!$)-%LU zMsTTqHHnO>0axPHEv)@v3Q#8+Z*%^?Ots>RVFUoOCVO*LOcC!Jxt{$H=)@VO|iFO^c+ zdvHTITNp_E`73@wVdwj?kVBTFbE0*L$BD^I4%v8*=XQpXePWt6svY$60p>jfJx&3Q z48|&!&-|`DSv0AW&lSTMLGI>IrL6jl1bfGuy++OWgce(%7(C z!itp}LK#E(O}!xlvhhE7!VLvhN51M)t>zb<~aiHDOQ_2Ow zheJGhj-I_EQgYr4pG!VY08RDc$uIi`?VrOY!60+P_vy1W!$=U~f+# zVOYheq)}#4(eq8^bXfMG>UW~DdV-_^2Y?O>NYaLtvUak7h*rW3G$ks}w zS@w-F=82Gr*l)N39=jGa;^OoLDRZ!cnoflyQm zTxb$zBG90c3f0&2mQpAxWcBjti97Cn5Te#1FBUS^>+p=25vi`&8CaQCrzKQl`ES|7 z_9~=IXrXGwQkYTmg0d>@mL3dls9y5E-%>?dVU%g>jHBtXkoOH`VaySyE?oIDO%fjj zKPo$Jd$12 zm);M7*>|f0Q^#Y`C?g)PD*p6b$H|1~UEd3;6MJNMfF8A_w-Lk!iocn-gj8{LF=7aa z$2yPPPsq{G@g3&I`fFXNT^9{RIIsDVJ`kTN=mNE)stbsYfqEUEjMr)%p^LkSaK_|vvyJa#f^MngI1yDCfZzP#g>9TOxM40tQW)t5|PCOsNYK)q~xTpSCd62)BD_`Yv7zyTB5{euWJ z?7iE~k|=~VV7H_CcszxaA+iPGSKZu^`?`8^m( zamb0z)J)ECMjsk#(&0`MqSzT9-h1XisOISx1Wzh35tJ#nW3GROO~GC}M#VP2zzXkF zE%$`=1E+e|+;m7Wbrp_}oK`4l<4(SZnb|Y!#`+>+b0fmrI=K; zlKcJqb<+5ii(HITs1&?+{D)GSa0JfJ$e6ojmk8P1-$gy>WOE$c6xJZubbyn{w0syS zwtOF2fg2%P?ecPg;*NO&#`rhTHT`?W%#^ZuS(2`k5h@b@mrDe1I>Oj5PdC$_aTcVpO2- zL0|4|4Tzo`Adu;rZ~nX~$z$VRgyU1l(kB@DP)LI$2Q!+{b9`befuKZN^HN_?m;N&A zj@pT~J^7AuxwgJVP`^94U1b%I_G49oMD5EO3C6e%4w{RLOu=3Mp(IT_#DLBZQG@pU z0_RSh@F@0eq;WhJf%Vfj_QTsz>y>gta<>cS=oky;yOdf)qFcEMQR6sVyA5OoSXjJ} zD)cD2VyiYGjoyE*Hhq{m_k@=vMY271F#o|#yM}di;q@Ve?c5l}`#7Sqp_$Z_#D+2s zM=xIdm6j>>g;;S>&9;o#5|Tg5TtB5O(-6*0ig62Du)k8~$$Y`^$cXO14$h{cdZaRY zhjsOe-viB)1`?HK25;FT2yBnux_$<0_mjD|7wLtSaMsi_Q?XTkC`*P2%1Tzax5^yn zl5hFZfa|Hz{D7N^xTh8Kmlwy@C@b+;^tjELq#eAY$XIyEXDM<5KOafM*LY`oKFo|* zrk$qS1wUb%);l_uPQYXH*Ko8{5rC42|GPE+$Ibu$!Vv!F{U7C7OBEUAe`*N- Date: Fri, 4 Oct 2024 19:59:55 +0200 Subject: [PATCH 03/18] Update slither/detectors/functions/optimism_deprecation.py Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- slither/detectors/functions/optimism_deprecation.py | 1 - 1 file changed, 1 deletion(-) diff --git a/slither/detectors/functions/optimism_deprecation.py b/slither/detectors/functions/optimism_deprecation.py index 426ec7949..8d81851a0 100644 --- a/slither/detectors/functions/optimism_deprecation.py +++ b/slither/detectors/functions/optimism_deprecation.py @@ -71,7 +71,6 @@ The call to the `scalar` function of the Optimism GasPriceOracle predeploy alway 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)) From 6afd2810121d4d2f2906c9141b2e1ccaca2c898d Mon Sep 17 00:00:00 2001 From: Simone <79767264+smonicas@users.noreply.github.com> Date: Fri, 4 Oct 2024 20:00:00 +0200 Subject: [PATCH 04/18] Update slither/detectors/functions/optimism_deprecation.py Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- slither/detectors/functions/optimism_deprecation.py | 1 - 1 file changed, 1 deletion(-) diff --git a/slither/detectors/functions/optimism_deprecation.py b/slither/detectors/functions/optimism_deprecation.py index 8d81851a0..329b78adf 100644 --- a/slither/detectors/functions/optimism_deprecation.py +++ b/slither/detectors/functions/optimism_deprecation.py @@ -73,7 +73,6 @@ The call to the `scalar` function of the Optimism GasPriceOracle predeploy alway 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 ", From 909ab5193e2cc9eb64baa87613e2b7610e94d190 Mon Sep 17 00:00:00 2001 From: Simone <79767264+smonicas@users.noreply.github.com> Date: Fri, 4 Oct 2024 20:00:28 +0200 Subject: [PATCH 05/18] Update slither/detectors/functions/chainlink_feed_registry.py Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- slither/detectors/functions/chainlink_feed_registry.py | 1 - 1 file changed, 1 deletion(-) diff --git a/slither/detectors/functions/chainlink_feed_registry.py b/slither/detectors/functions/chainlink_feed_registry.py index 951dfb584..82ab17424 100644 --- a/slither/detectors/functions/chainlink_feed_registry.py +++ b/slither/detectors/functions/chainlink_feed_registry.py @@ -83,7 +83,6 @@ If the contract is deployed on a different chain than Ethereum Mainnet the `getP 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)) From 96c6c94dd6c5946cb4031a88c39aecc86e1504c3 Mon Sep 17 00:00:00 2001 From: Simone Date: Fri, 4 Oct 2024 20:06:52 +0200 Subject: [PATCH 06/18] Only consider if call target is a variable --- slither/detectors/functions/optimism_deprecation.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/slither/detectors/functions/optimism_deprecation.py b/slither/detectors/functions/optimism_deprecation.py index 329b78adf..08d736a2b 100644 --- a/slither/detectors/functions/optimism_deprecation.py +++ b/slither/detectors/functions/optimism_deprecation.py @@ -6,6 +6,7 @@ from slither.detectors.abstract_detector import ( 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 @@ -59,8 +60,10 @@ The call to the `scalar` function of the Optimism GasPriceOracle predeploy alway 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.expression, TypeConversion) and isinstance( - ir.destination.expression.expression, Literal + 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) From 6b503a9970f50bc60b98977d61498e8dba69f05a Mon Sep 17 00:00:00 2001 From: Josselin Feist Date: Fri, 4 Oct 2024 20:12:49 +0200 Subject: [PATCH 07/18] Update optimism_deprecation.py --- slither/detectors/functions/optimism_deprecation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/slither/detectors/functions/optimism_deprecation.py b/slither/detectors/functions/optimism_deprecation.py index 08d736a2b..752e8bb2d 100644 --- a/slither/detectors/functions/optimism_deprecation.py +++ b/slither/detectors/functions/optimism_deprecation.py @@ -42,7 +42,7 @@ The call to the `scalar` function of the Optimism GasPriceOracle predeploy alway """ # endregion wiki_exploit_scenario - WIKI_RECOMMENDATION = "D." + WIKI_RECOMMENDATION = "Do not use the deprecated components." def _detect(self) -> List[Output]: results = [] From bae46ba9cec65a87e8bb686850f7463e72448ca1 Mon Sep 17 00:00:00 2001 From: Simone Date: Tue, 8 Oct 2024 13:56:18 +0200 Subject: [PATCH 08/18] Add Pyth deprecated functions detector --- slither/detectors/all_detectors.py | 1 + .../functions/pyth_deprecated_functions.py | 73 ++++++++++++++++++ ..._8_20_pyth_deprecated_functions_sol__0.txt | 3 + .../0.8.20/pyth_deprecated_functions.sol | 35 +++++++++ .../pyth_deprecated_functions.sol-0.8.20.zip | Bin 0 -> 4367 bytes tests/e2e/detectors/test_detectors.py | 5 ++ 6 files changed, 117 insertions(+) create mode 100644 slither/detectors/functions/pyth_deprecated_functions.py create mode 100644 tests/e2e/detectors/snapshots/detectors__detector_PythDeprecatedFunctions_0_8_20_pyth_deprecated_functions_sol__0.txt create mode 100644 tests/e2e/detectors/test_data/pyth-deprecated-functions/0.8.20/pyth_deprecated_functions.sol create mode 100644 tests/e2e/detectors/test_data/pyth-deprecated-functions/0.8.20/pyth_deprecated_functions.sol-0.8.20.zip diff --git a/slither/detectors/all_detectors.py b/slither/detectors/all_detectors.py index 44a168c2b..6f1f94433 100644 --- a/slither/detectors/all_detectors.py +++ b/slither/detectors/all_detectors.py @@ -97,5 +97,6 @@ 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.pyth_deprecated_functions import PythDeprecatedFunctions # from .statements.unused_import import UnusedImport 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_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/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 0000000000000000000000000000000000000000..258a28c938effe4b95c819b04d6e7629b1eb21d7 GIT binary patch literal 4367 zcmb7|RX`IAz_m9TCfy+^p~wJXq=X0v(jg6_I|huD4k_sl84c1*B^}+}jlhPqfWi>T z|NZ{^@8Ub>;<-Av=i<>;!^M*W-~h+~*RCqY!9`B_Y7_tfODF&!3;+ON0dPmCtsTtE z&c+IEXA8CWakqgxdANIny**r^cK$FAFF4f7%f`{k*ADFL?cq+0hYv6Z0KxzOnb=qd zky{bZ;@Qb&Y0`oLcPH*H*62iCCRw&6Zxo#{_O)!%8nYE8=4b04kU?QGR)y)jQBe0J zCtIbGVr3rg`N^HAbE03H=2)eewg&#Ag-O~4bx;Z zMcx=|CT%?QX0ylc&+7@;(X~4|6uo~0~2Svr|WGLH6B0QN3QP$>Dc#F@rfFn zQ>?lIqD^drGPDpG+>SVG;vo&K`mCND*HbS!zv#XzjH?6fxsWg39=~6%r4Kh-@hKDJ zdH6GTuEl=T-DX4j+BL6nzuF+675L1x|0|WclpR_YSkLy2#DRzk@pdgxu#=q%)BIWd zWoGiA#<70YPQd}G+ZM-uJ9{nOYbe9d=Pzy9$*2jrG4#dSxfs`4`6qRo=!DO}k?sN7 zWg=LUE83z25!mmj)aX7JVxnkzY3=p;5O>=-wH2IuZ2flR)BAi(ac>F}#P`M+OxfAQ zghu7usu>4V8*NA9QGH~J)p`5P3;Fy`gcI1}JNu*zj%4$Zv_ytP`CE;e{AuAcy=F*f zy-dD27a$PQb|K1_33bjDMZo~dys2!jg;oJcBM-MGPv9)AQWbx=a|xca{a zoI;NR1G;v<+n$+wFz$xSXWqZ*UoSNr(RJSfIc_v%%oE9>1Qvn{D9J@eBc(a_ww7Vj zZo7m`AM_ZkdxQCetyOlB>^q9Zo`MQHO3fo1S0`Nx2ekOv^+bvqVTmKs<$RewPG7R0 ziPz{an6bS1JQTZRJJ0?S(z7gWzOQG?Za^?lJYP~s{6`@jq@i*IZVuZ5Iq zQ-2lRpZB1mn$MC|d#Hm0X%j?o3N~>NG;1SZRJUC{n+{g~uzO$dZ*avvz!fSj3;bj% zbrNqpFw~fZ^sb)Qccp8o^o~oTUWZ#SrVu~@!}Wl7+82QU{vU`y8)%f8=*x@P&+45< z%mj=#yiDEa9Jz_ggj+(*-zS^r>|O}X#V|aaE6ba>aH+zqht?S#axI`AO_<^>;;dt; z@ra|1a4a?qeSg$6YcLd_9#^H+v{Vqz5?-#`Hak&V;hLm@LF_`mFD`IwtUv zc97kf`QjmPlZztX+52=Q(TobOV>0Tj)KM~_iq?yapT?uyWT(?-PDP*X$HXdX2Ps0M zW1bs5G)OLsuzMF-;KlE2V)Ur&xY;sVS^1kqqHxC|2cVK=7(B3B8bcI3UwzAXrBVqo z&Kuf1pPHtVf7gGGH{3`}9Bc;6-~z;KYlWN1Ly(iCrRlkSH`Kp(haLmO{KWT@wo%Nz z2E#Hx!PWC7n!@ZqTE@9AkNQU$F=ZlBdIomvm16Y#d-Ym*%QGtc-=z4))4q&{6b>%E zdEU^sz4%fjN20~-X{95+x`>=Q@K%)B8_N-H8SnZwu@z)1P@v<%9C?2rf)dQ#CVEWj zFnEyQ4z!F&GXql@D#8_8vkhD-7S?6!X0YkQ-dd40ZWW&~c+?RiploUu zrj)HnR!cSmj6F^hE4k0Utp6%vL!@Pt=y`3Q|MYW6__WYjfQ%mF1jZcJDWl$m6Pb(P zFk7wry0TD9*jy{e(ABF<2fV%h!jGzw(X(KZl-2)Y;C!NpZ!?L)!iOc3AW&4@V!D@5W9f128c zlF%6k1T{2noD;qlrq5F^Voy=-^+bYWzpaVK^C;j_`R8WIxb(q{?M+mU^l5r6}Gz8o{_rDy@&sM{`W)2IjAvHG`xd1eO zSdBlvZ5AVLeVlDv(Y{`SU{#h?qN3z9%VF`WS49ko4P-OG!n_%je?NSXT223$4%eJ> z_^i1@$Wn4r0aIxlv-#Wp!&2*4yGK*7LA*JuH6&>-V5G`0JPZ7219Bfvh1-q0eOO7C zg{eoY#FG5wqVI-m9W;&`L@8cbUxsH8*?ZMayH})-fU|3!jESiB%0OJUcGG|F>#Awt z02fD7SWUM&`+3!W?-Tde*;hq2Se~J)R(hetG?_KRyb2-kF63QgADhHdIPIry+;ukp z(JC);Y~qJIZuj4eE|J|Higa$WSxnWEzoM!?9R@`QeEE?_rgYa(BK^aAvQu$;U9|Jl zs0lNH*~<)1*d~uGFC>&G_ijf{IBVcd67rImE$9eADy(o{>?~Y=XAxn03RYLOkijIq zHS=6N5{K+^4zCeYzRaCN$Uo4o{)NilMk?hgYjJ8PRQoX+l49L(mZtNl>pQN? z4h|G0`Os(EPC(dq&^_fMzG1~xK0tTdIv{Xr?-TtD(^%E55s8FpqITpqMRRRYd^xq7 zt7GDL-sSmULl5zqhpg!*vj?2KWDgwMJyVg@XsfXL4aCgTyy%a9W=>bsQLfxt+< z+VE*gRzHSb6G$0qG^Ut@!(?yzFI>EO{P+jHj>aZVbsx?0Gn*|;JyW=y4}`AhA~4Awh>glnXM`9>HdE@nQLO)uG{TWN^=UgF?A&FkDK??r;zSiw zrbGG&P1)ZKDV;n7El4PNZLHmQ)kIedj-=dSd3#lxG7XRAUw=R(LHf=a1 zLhkL)ejPl$9v2*D;ZdCGX^d|E{s%7iw(Uu(1V(%~u(;d?kM{!HHpVYC zx9AM{ujeb0i}$is%(~WuKNhE&aCg^^__iO9`-z%V8=U*zsQ~qp5g_7^TB`AIjtrx2 zcrIqZYH!N^VjD}?tzc1+;@NU=(&ASXwIicY-QoIcbpxHiMDDzYeMH&oH!m8J`AuD@ z>BV}!*4r=TwqMBqh7g)8$uq?y;_!V$eWEVm|Nc=;GiT!o+`ApFL*E(5zSUA20a0tt zjGQ(&)-5bcjM>$)bUzEl6?++aow@$#*B($An3_0BJF{1yN;8o<%V{O}{kR4@j!*s) z^~P45z0t!(!k29C7q9=A2~Xn4D^uGX@HpeD603L4#vyQh-tZ!@4Hhw%;XN=m%m2={ z_t)$@3nGu7Fp2GWOzr#tFs67Yxixnt(dP@D< zaHp%nr*|#xfSklPX7w$Qp>XGlwTnbm|)wT9!_>d3Vn0Y?6v-5Dj0%CBMi_op^TR9vb{C za)_1Dp}1%v3~YW!a!Nv9L}buWKR!>_^+!3WN`82730qeRrAD{0f6&*p49G1bWK=A9 z?B=n?sWiCNCCXLY6%#U1{hnqpQrCAb#h}lWkOb-@LeV)4TY!gSlwoszxg22vKz$nJ zM%Ioi0zZXBb6DRmUWDjxq!^tMSe2XjX}1xUGMdB8*f`TG|7A8X{<;y5YL&+YD#1% zxS>KSxO%m-&f(1t^z)uh4Ow^i%R{^gNlr4Kn|0oq{Ex9cg=?WN< zH3lx-KaZjzJ zOoe3jT~jQ7J_^i@C^V&j9##?LYNHy}W;QF)_=Rzh=vfbk1JZr!9rXPQqW*bDAJ_c2 z_fa(cnM+#VRf@dczuuV5)%M?xN=LSLVVgc62`kT|6wGk@;z>0deGT|93a> ipPu;t2?PEQ{?DdDTMeJ!e}=&S3jL3H|7i~Z;Qs*=iC9zs literal 0 HcmV?d00001 diff --git a/tests/e2e/detectors/test_detectors.py b/tests/e2e/detectors/test_detectors.py index 2c6a5f55a..e30dc962d 100644 --- a/tests/e2e/detectors/test_detectors.py +++ b/tests/e2e/detectors/test_detectors.py @@ -1714,6 +1714,11 @@ ALL_TESTS = [ "out_of_order_retryable.sol", "0.8.20", ), + Test( + all_detectors.PythDeprecatedFunctions, + "pyth_deprecated_functions.sol", + "0.8.20", + ), # Test( # all_detectors.UnusedImport, # "ConstantContractLevelUsedInContractTest.sol", From f25344c6a29a5dfd501f00a474fb4b9c520fc11a Mon Sep 17 00:00:00 2001 From: Simone Date: Tue, 8 Oct 2024 18:32:11 +0200 Subject: [PATCH 09/18] Add Pyth unchecked confidence detector --- slither/detectors/all_detectors.py | 2 + .../detectors/statements/pyth_unchecked.py | 79 +++++++ .../statements/pyth_unchecked_confidence.py | 50 +++++ ..._8_20_pyth_unchecked_confidence_sol__0.txt | 3 + .../0.8.20/pyth_unchecked_confidence.sol | 193 ++++++++++++++++++ .../pyth_unchecked_confidence.sol-0.8.20.zip | Bin 0 -> 10811 bytes tests/e2e/detectors/test_detectors.py | 10 + 7 files changed, 337 insertions(+) create mode 100644 slither/detectors/statements/pyth_unchecked.py create mode 100644 slither/detectors/statements/pyth_unchecked_confidence.py create mode 100644 tests/e2e/detectors/snapshots/detectors__detector_PythUncheckedConfidence_0_8_20_pyth_unchecked_confidence_sol__0.txt create mode 100644 tests/e2e/detectors/test_data/pyth-unchecked-confidence/0.8.20/pyth_unchecked_confidence.sol create mode 100644 tests/e2e/detectors/test_data/pyth-unchecked-confidence/0.8.20/pyth_unchecked_confidence.sol-0.8.20.zip diff --git a/slither/detectors/all_detectors.py b/slither/detectors/all_detectors.py index 44a168c2b..75e838e53 100644 --- a/slither/detectors/all_detectors.py +++ b/slither/detectors/all_detectors.py @@ -97,5 +97,7 @@ 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 .statements.pyth_unchecked_confidence import PythUncheckedConfidence +from .statements.pyth_unchecked_publishtime import PythUncheckedPublishTime # from .statements.unused_import import UnusedImport 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/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/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 0000000000000000000000000000000000000000..6e5fa1b9f1b470d9ad9ca7632115fd7f49e67ff9 GIT binary patch literal 10811 zcmb8#!(t_Xf&|dmwrzK8Cmq|iZQIt3t&TdjjT_sxjSlC%+0SCC7PYEBI7+hM5JDhe zAV?si$x_+f+aiAc-KtMp8yxpt}-5pG=%uQ|0%?wQ)9W1QP%pFY4nOq(1 z49&fq99`TDja^KwtUb(`Y+M~3U?Ctu^g%#^KtKedqAb}y*_?|OW?K0Wb4MJkDf@`R z<5lnkiC0~Fuzo#@2_|gf8>1c`w?DvZu0qSa)yi7SeG7x!BBT1=1)cEOzMKy%mj(L2vTo1kVgAq6G zbDc#^-Jl*yYG+I(}>p@)08k5IKg{0sf87SFhln~NU7 z7+6`c4GXsJ?W2=)MFxXdJZ@ITUjgvZThsnTj@Z%rjaekGDI_vl6-=r6pF{b2>;?26 zpNr&1a2X8(^aerHy%k2hR2UkqxP~IXb1|{#vJeA0>?AY&4)$;J>-g$WZaQnd|TX>U*kgyiP1H^b`)^Q7E`&R7I7h=F;jKJ zP@`e!?;Cv0D$aR-x&^+P-1ShbVWIQ{+ffo)8L+^&8w}ihkuz#Gg~TRb`htXDo#KVr~IKT_eB|sZmLcZ)4C9}aLES>uKzC?3&G2TnKOvR_h6c=0JtkNs{Yp&ma-sU= zXu5w7bd`4w&2k@oeeRAyf*$W?^8IXj0+^V`*?3j#%qdxXF>lgM=kJgvGk?SVc8(X( zM{cos9BQ7q!+{nfL{I9|&L|U18ebS9)NquHj+*By%9&utAQSRrGnBs5Cpu+#W%SE3 zucGW}&KuHR8szK_Z%du7siib6Gb3?^1YVA@i~~Uo2(CEC3)R&=$^~iBDXMs#O8ttT zpncQCnX59UaA(s~uC?D^y&<|nRxs8wPa*W-GYY;PJWc!(c%mfdDvxeUmBI3kS~|N( zPD%-@vl}p|+VPT{F2#)yU~cZTrzGaHzQV`P*N1b*2+{Odfneps*sR)RGOH4t>miNL zYGw|5o~wHiRtL~3*+{OOiBNHyH7XB<{UHhP$)-^im`?C@kuOB!k-=3qoS|%FG5iSh zbZ*TjT$GHzBL2wwdcBG){%N+>(yX{vsd_YW8CN!Axvp{N5ikS(MGoj}uayS9TU= z;T&NNs|1WbMA!&S`eQV@8buVAB7`M(YtidUyR#GTiS}TM8^%!_ZhaW2wIO{hmu^s` zq_pQibyBqJV48&8Vca2`hGitO*TrRQ^;{HJIlFI&AQ?+AXED?MU6%^_(nT}#e6bng zWw>V{>tgF;lZ3x+)v)!Kn8?y&w}hvI2-ENo17%azlugWdTX8_p+}Oo=J31A#532C*i-wmyX@H~k(EVj>Pc8;K2CpVpfX?_!^2CEw zYVmh|?3!ZNvyVZqJ1uFIKa1~gIB@?VLMCtVzJ~5SE|?6tct8ej)z<*PdkE{?fzO1W zIWVVM<)>OAgx6%Hfm6250e{|k&}>SASCJmOA7pE4F}yD>lFzkEI>)+ybqe6@S}kYZ zqH8(P@Q0CDShD!Z+UWK>5HeQP4%Pkg9nQtj`Gg<-f*;cdQU+0ijqD1mj> zvE*$2ARMmAuSaDnH2M>gcaHG|$^~T;J1ALDS@Vu_+l zrZS;z7a zCmu-;_I^O1YfXAlih$uh)+&p0;{;H7sNT~|?8_4Cd#*g$H41;~)Q^(&19xR&yvj1S z$Kz;4vBa5AU2A2C(Ex*`jTS3VLnInC!xbgZ_iYALv7SY)ZQD8-aQGm)@f!tXydgq; zj{_0?*5*W<1JCoUu2^X<=DNu}ablNrExP`*#VqP;V}dnsV@7^+NEVI*9_h^J1M9%_ zro`Shq&YEjl0iUQ-PTwojV^68*V3#HWDdr2AQJ2{c71VFJJtr1M++214}c_}QL94)Z4-n1XT`WN+`_Vw`}9v8&4|qCAmS zZAlAByXoIYHrmg$rBdy8JSH(&1TsqR#cp40wu!V$&)uSVp!$F#zzn=2k*{U77Idu$*#A#X)FQmMNqUT#fi4B9_BQV~MqWi6yZ>u9f3Dr3L;CuZf z6HgNXax4N*zD19IXc7TR0}XXP&$LVofDz*<4RDqT#wEA~#v4R2+{{ z9>gN;|#vR}EUXx0i$L;nU9cz~p^) z5Vo~!Lm@$H40`F4)sIcN4YiXSjHqeSNE<0Wq7D{`oUR3@Vi7wk5@okLveb0?z=AeS zd66=Wn^$h5Y?Qaia+npd6Zsz7xG^+#+K_T1Lo>3@vQNE>2GN7qQ>$7U1B4x|)owfE z*n5f@jxm^yD5lHFQLAE$TwZ@eu=#u^w6+ip_7Cmh#vxQcUz>%_&&AP z<;_)&5tDNvskUYhAdD%^%C>|``w;yliN&6)^04o;nN^MYDu}DzF7@H+&a~$x3lpzk zqh3GN^@-nj>(zVvBa%J56Mk&cA2nBx_*jKneOZQN0Oh|5H16;KWFGOj>+_iH7~#Br z;}mbHz8&ec8n~4EhEi@)h#^$m=R8f8LjGnc!MEv(C;UBJaeg6Ur0uHPioU z#ja4~n_kkpyU2B43sX13RP$okJAL})(Lkx1QKGj}-ZMcIp7DplmjGtPLcd zI0>#8BMgedBk`JU6&265z1THSmE~E7BNU&R!5eNEKy^HipGVWlY)M#z+stxuzUXR^ zU4L%M2x$}WtqovU+m7!0p<AoD5!puTV!}l%XeQ<(~nG-K0jg0rEc zYadOV342&{o+al~C54Ez!V%aYUZPSFMR)`ETKUxLWoe$!Uq2E$UZ^;l+GQ=GA(=v} z1~!na7Z#0@9+gF*8eftg`M85?qE;3Z1GRGffw$%PDOzpLXki#S2%5lu^kUTe!m5#Y>N5fj5O*R zYQ-JCdq{qU-!G+-_l;G3|4v4_YOf#Htvg89pT$$g#?mD{3cm@E1+E4A0Z%aU3PZs; z7Hr$9Qa8)JN>PieZMgdBg0F+{D??J50&FJf58efpzATeVrgdR4Z7FfiDOjA(vY#1G zEyyHl{kRR)-BLr6Apszl31|{B&f1iFDoFLH^&ELnv*TYfRw&~{Ch2lV!k*23mb}>J zh%m5Blf#^gj$3^vope&o?WpT3im;4Nym;tq__70|22}64ykMR5ikOeUax_gWYIa;bX|yHmt091& zJi>dM=z37oxa6Q0c$BT*3u5)5a*t}r3M=+eF&jvnk6E1NBT-(KKjS@d5z1oR9OEr? zrwxNs1A2p=8{a8V%Ol$)$)A{&sJ(J{?%_1U^@e#Pd*-_t8B}n-5r|~}{`tlvBpOHE zN0LU76$U^?JyiA$L0F{mnBKRA_zJMBnu{=@!&Iu;HeuRcFAcNf{1#S7>OIO)pE8mH zI*Wmb#~SssMk$U0!|6V+-1*vTEF3@Q#B>AF5l{3BrV8qH^^;!~KwFfrFW&`c1A}m+ zkTrGC=B)65J&myf^k1hqdV~8LbtG&3^S>zCto|TZXzU|@0!m~nUBZ47ZId5QO2RJ+ zlQGFGmbcj0%19JaxxU&!yve*dmQXAuZDbn!F2hu-D(xnb*GitJv2jOIOU;Xq`1}HG&tu4@^no+73+lxmC~ut|NvoBXQ`y{>O%Yp+4xhEPJver& zV7F;jN!#z>aCY34Eb#`DgZnC33$MXir$9tVqU4BG(@i+2^^mQhK3+cA4$=@A9~D(Y zcHQM$to8l*3iwyyBreE|`!fggUz+Sg%Tu4_}8A ztcH}>kud0p<}o#-QY)5~ef9wRuj!cxmEpT%J>J+MQb>Jll#q9+dRxaDDPVA8O=XOI zUr3Su0O6-Nig;&;I9$eZN1gfOu7QQ(Vf2kKLBnaA(1&q~gDd8SrI$-xj>L?`3H=gG62AHvkjy2LN;<*@l9Zz6|k=fn~Juc)Vr`*X&wgtpc=H= zuH4(n;d@>}Mpuzxm%l6m?iI$Q5etb;+U{fdwcr;H$8#f0kffi<1{yqqPV}cFnm903 zB5p!GUMh97KxbTY_w26&G`br0TB&PBxiYP7^B=iw2_Zrra*DJdCYzFkxYj``UWZj% z8%?WS5*D$1&MET^}ss%YP=8dpu$Oy#v@y*6S z?^H(&l?=o*qQbZ(#%UZF#cc5mF}L7PCW1;}u@Wmq4i~e3Z4W!vK!a1G>}ksi)3#{c z!vXukCU%KQpfmJK*zeD_u7R8NJ4amXS_9z@z+Z-D!PT!$Q9ad#a>#Re5 z&a)TfuNtdWUp{;#yW+OfAks^Wo!)DUksI+)eAD>VpG$?2`2SR84^9I+*LZ6u4?F4K z!Ar~n-t*!wL1>`3MLjCfE!17h8nxW*tm3OxWLR5ruVjRD`5NU@YX&c(p67A^N4GnnE|6otD%|RgpI1FDrs@x6No0HT0)tuaBpM)srA@ zE*i=Nh7j;wxP+aPju_Z%>M7;4Jw;Cda&tWISL^K4!$&$Dsb2lk?XN_+i@)7?PRajy ziF=LUeyJZ6u63@V72J!A7gLs{((BUJ^a@fjK;nui7p!FVdUsjf`ekS7aV7|*FK~0| zyqeI_W(=LcR3_AKYQh=k7z0ketEP}Y5V}K2kR6c1VT-_#(nqdim<+iC7B8MU9BP{5 z6rb)MDgyfe|0Gj?>m#Qq$DpjNAz(IE@^Gwo77iGPTuG#BX3R@Ty~)5e3<>SvMn8It z+v@cs4N2tI&(8>uhO&X~q%Y^tRKx%%uv( zQM7W!?l?`o223Qm0)yldc&cAJF4>4@+%QphpqA=O4wnxl91g_W1{;m&m;9V|i`Ihr zKJS&Z~-^ zF3_+-;%DpDr5aNy{L)a`mtAnsuZhc{c}wR4WY4D&P*(`p`h_2IcZORrWp4hVhVj4efKZFX#D2O%qxl>e8RhWNie9Wj08FsFpssrb1zsqy^ccXmmk@dkgRD?d6cy^ zyh-3_mwsxyC5b2ufY)bheTpd>0^Wh9xR0s$f0x}P>i+R!+_lp%fb5VZvwdP)IHG>5 ziATL4=SZcum{5pVOzqk(v%q8E#K&J^uPI=Kr>5exUcT&r7h+@CL$#$8m}*%Y_KH0t zYHr|uW_plH53c-W=B0PKX0IDrVYn=^bUM1AP2?Nk5#r!B$6-8phOb-aJsQ23Yp5jPdm` z?{T5x0CqRTnJfXRjDB%bL0_h#x^PG6@7HmckFiGM>BWRi^@>@4hn_UO@q?_gWB&yW z+bK!KA-`JMCXS095trNQARP&89C+Zdn;3B?HpGXcYqhsj0;!-(-FJWJC*o*_Rumu( zfFsyWv|VT7l!*w1H|nnugmetWkn%y4sUI|0IE5Bct@U*rxsRT2IUMHH5f4U&?Sned zTek(Jo6OgN_>x`NgA4>BcF?J?7R3^xjiRTIyo#&ObWj37+2~Vjb9e1c{Ms){LU2 z5*L`|?n-vXx|h-ej}2?&L80v_4f`sXvtP$)q9dJQ`-6e`Us{Y6os-nhRw|PVev^E-;?$WYYvmH2cl(Df)4sYUZBLz#61L~N zsGq}F8wi-HI0M&!asn(4OtF(F@fWL9k_MkUjbTpXOI@3NHjYCVjZq>r~=!=3X!u8F`733En z7UBe^D+{R!Wa;+>S$r`m|OJAA&Uunv);o+LHG$qe7xVM7}j-ZNmLu1~ebdXD1x zPS;jvx7Di)0<+U{wJfE0w zHgx4r<&FCsd)jQR{HdS(2VfPlK*5$+uO`%3V1OOpH|PBj&nT_0x`&T2wM;R0q^M3n z&pja;Js}tvhbV^5a3Zp(XXqqB-7!qUuXsq$j|1BMRy8`3N#_;}YDc+%Y_(4#8&jqA zzeTZjeCuqySHV|DB}apMc|fS-6~F#Kg|5~MLJyU)3F9wnPp*TX}$sqlVnyR-}RSm*D)3c^?LCdsL)-hRqOFvFk& zuq8+%esnI|RR{8OR_e3pi08>>=|j$){#Xd9{r%-;bM-TM6YM7=BGc$xE=Ws93?u1Z zqEt=FDmbLdcxRGKke={TliO3TUq!RHN^r~%UzohtDB6I)!;70I8Ku^38*9DC?=)r< zotqpv{U5>c(#yw^_j{Umkv6Z@T|-dx*(RsVmZi1sTJlaj;5p3=1z{h5mt>h0bD|#q z)*y)?tEP6htjIi<*=kJ;0dCv4f`xc1)i(%c4b2u}Prt64Y+%c!q-i7L2FIjg$DrbL zj=o>d<$xQNIe=;MGOsm}tUT(`%0%`)_XB3sWfrVXRpA#!?f$z^-ns3 z7p?f5msAV9U1c`pCEr?8bqS0IXC>bFV|r+z8~Xw(Ucsu4Zsn+oUpD$X5Xf;8P%RRI z3w3K5E9!!8rg-QRf_AOF6ZOYu;kIqrsZbB8*}!bnB1yS%>?|uAk-VJaooOIq_FWkA z>|h7}q*eJfdHaO_E|&NGce`^aUTn6o?mw0;)pE9cg0o5G`mx5`dV`z7DMWeF)2E?* zKq}0Qwi5L}SxB3sLt&8Aq<81sq!x(yvajYoS8D5yn&!U-(JI2hmDF^=9ms|nbAc??rCM6r?9EbR2fihpTHQ(c98v4xCl^pvR?#>K zz64RfT*MnI#6)^$rrQdv>+$PG7ne*WQ$bLvrnb{$4ZkXP-oiA+XFRzzEgn|Mn4&9k zR6GWnMW7s5jdH7r(&nRHdK_ef!UR0+@{$iyViIEVE=es~iCt}_39xA%II03Zac3nT zU>QK1idgTmyEQp1$JA4XF0agqWLsHgW47%j?S!cdv`EY?UFu{KYMcdtzVr&Sp*g z;tCKKS@jdKq-Fih%UFx(wMaw)`F#@c?c=ZX$R2A$dJPPXS8;W{q_61?1)>`-UY5?y z8tNwzXM=s)BO#^J<0^zQq`zLIfJA!AzOa+GgLzsgS&_kQ2I z(~S7)93)dYu1)++HcsU>NcgWZ_SNU5&)5!0-Q0HZx{4pFYTnN$c_DVLV`_zZnb&%7 z-7AelRdj~BMPNw~?xf;58#HKh*;c%gL^yCP@zE^wxYCJZ?{fe9cvRy~UKyEdY*M^+ zu&GB_IztRg;7pSZ$9bW8k!HAc~QBVZY5KY?$rZ~z1OO{Q(?J-;-7vY$wAD!D$XEeSbuK(}%I3xvl z8a>l%8FnR|Jj>YjL)o~Zwzf$%Q>vrcC~1%z2J;MmRF(ks82b^F3G2mABX^NJ2UY6s z_Ez&UfAAm%B2QH0APxq>Z(NPQT5hf^rYz?;j=|7}}~}oC!&H@D8UJR6kzxUtK*+ z*mr$*(DDJT5{-+@4&nidXxaBCeK;L(8^SXJQ1k=$USKWb6tY27K70(vR=XybgB?7Z zZ)3~SjBy>B&u$Il8-6s8N72Hzg3EUsDdD#tToDKZ-6DSkerwS+)JSn}X}NGuD5%1l z2GMEo48mYcPgC2zNwXx(Yv%wilFz`ag;0>miTk*YiR;>Za8}zq-Hu!3w&a!#V^9B1 zw{s}?hJx;%JHzfm3;)AHj%M28#n#W4A7yu|8_*O*`M9b z0ibmH5&&w1K|Hr5K|MU$tWA)OV?iZw%1^AexWB54lR-4?&-p{FJw>_nrM;arLmK;x z8HF2U*N~#Ilu-laY0_;*%MLRLtWOXx9rjZ#CiDC-Q9aji_d_j6k+FZ?paU`CvnbhB zn_Hyx81<@xFdR7Z?>aNj;R;c;SuuJuQRR_dEG+@h1&Cw^#DmIjZD9aFdG_dMa|rQ? z?6=pzzI9PAIRsojp0#9qG0V0WKM8*|K&~yKBVY#1F6e7!jOW_iT{?9yF6^Db}==GkAx{qQz2is@t@@Sbhsi#YWvg%9%_QbOw@>d=jumWvjzh&%>0?~ z@pTNx;?R7OqMK7>u^WIeFk(0CAx%bVZ}EUPqm-O$k<}rI?~5x>-?OBYfEJ_laF1M~ z{)^Z#3_^V_a{nX&pW^wg0Hh0zuuj{uqwAfVT2_OW{v)H<2Ft-L>7eLZk}gvKqhyhU zUkI1ZdrkGpkfJ5z7nnz_JhMszDE{wq!zonx&h*`1i9a>%d~rltubzIh6!ArD#n+2O zfOHa;s4(lN7#|@PJ@F-U09N_y35C&UbCf-LOD8B)iP5~Bu|(nTKrL;ro3!5n^ryO*J#wE=6b~wmINBjU`KvGjLrc`w>(`&C zY<$ZLNT-NpwmL_AH&2d-f{qNi>6)~bO>Xr5#&?)xwC$+!G4EiJH1R;PT{$l++;=y! z*7CEGa{pfbGm}3`U#?p-cQ2TW*$nd(+CFp{4cY@>;Cxdei;@48pHe=9)O$6_rOHWS zXH?If=n_3Obt_GiceWiz%GMEjOTv{%kq9aD+W-zUJ4Wg)^WW7h7l(ph%nszyMGn{+ zTwD_p54dq8bEP@YP|t%mJQvTc&JMZN9BpfxLE zDyx*c?9FFF{Xe%zOfpyz!dek4tx8+gq!gf}xZ}&^ur7v*R6s3PA9+8kiC+|2aE7_u zCVQCj>31!|9%0OSTx-a)J*8}K>A6d)_G=eGeuG0ayz15jn)Yr_PJ%Q^4}%5pjEX3^ zEKrEI1^dg?jdaj^)@{tp0dXdDO>4wy37TYb4qEZD!3fNm#PE zF~;ZR2*-W0VA0r0^p`?Qlzj#p(%?=qhB)BFzmaw|(yy;Bs);1-&wO1CwfQPW&zxEq z7AMro`ZIowX!YlE{?YB5OHPp6*Z2AQ=2`K6wyZf4@SK{S`Xv>+Y$VLpowcRiA9oU* zH|K@A%@LJIhgp~fJ})j80Jl3~Rt4?mL={L|9lq3Wy-~kMlB;}Xv4QS)(d84y+gBUd zNZUc@%AE3tb}2J2WTZIOewjy+Fq9n}ps7BF2K6u;HJ%9tCownM2?Tp%6hvf!7F$-( zrknnTST0zs&z(F2n>n0AQnmY;@rCl!T%+`LxXNl<>~pEYnYOt+@j;|Den-q4Ve2qXlssmc-%5 z#53F`Rk>B&UCvVQv#)pm*Z!_$KAF3}xNg)m?!G;SBi5|Ux7k=HZd9~%=XnN{n775S_Yh^6>o9wE9Dz&~=c(G5u z0X){XCNuWd3S=k zR)6aZ-*-{73a^a**-(<9cD-68NOv$Be)zDr6=R}Lnvj1W51rPAJHnfKQ%Pr^;Xg3e zMZrsj)T+a$;V6Nve}{1AQrp6bGu*?iDl|~5Urs;kl7Z8gdd!;grio7()>q`1=3Mz? zP?EP%)DsDd+uHQ5r9&R*^E&fAx-e*i*wy@#<-9rvAqw|so+_`3pV2o-V2*awG`k>y zjgRCqFR0*t{|4}1>TYeh97C;Im$Wz+g~st4pWG>#LObq95Jl9^tNy@m`#M-DuSDApwRs8kA3an`~k zvl*9nzF&7Zm02d4Wm5UWlb|O#jiVXwLA+~P^sEVq8Z(Ru3jD$X)6V^&tNM^fGhX>(bzFTXX3rfz!-Z{(3 zr0?h91YqWIvF_N=-J`MD84zQS39rcxfB#E1B~+3H1rq}Qza8g)DChqs2=u@6|5lw! WvXD^!J%awHhX2{o{}53SkpBS-=ILPo literal 0 HcmV?d00001 diff --git a/tests/e2e/detectors/test_detectors.py b/tests/e2e/detectors/test_detectors.py index 2c6a5f55a..5efd26153 100644 --- a/tests/e2e/detectors/test_detectors.py +++ b/tests/e2e/detectors/test_detectors.py @@ -1714,6 +1714,16 @@ ALL_TESTS = [ "out_of_order_retryable.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.UnusedImport, # "ConstantContractLevelUsedInContractTest.sol", From 7e54d41faae06a5048b782845ddd97297cd67fa2 Mon Sep 17 00:00:00 2001 From: Simone Date: Tue, 8 Oct 2024 18:32:29 +0200 Subject: [PATCH 10/18] Add Pyth unchecked publishTime detector --- .../statements/pyth_unchecked_publishtime.py | 52 +++++ ...8_20_pyth_unchecked_publishtime_sol__0.txt | 3 + .../0.8.20/pyth_unchecked_publishtime.sol | 193 ++++++++++++++++++ .../pyth_unchecked_publishtime.sol-0.8.20.zip | Bin 0 -> 10531 bytes 4 files changed, 248 insertions(+) create mode 100644 slither/detectors/statements/pyth_unchecked_publishtime.py create mode 100644 tests/e2e/detectors/snapshots/detectors__detector_PythUncheckedPublishTime_0_8_20_pyth_unchecked_publishtime_sol__0.txt create mode 100644 tests/e2e/detectors/test_data/pyth-unchecked-publishtime/0.8.20/pyth_unchecked_publishtime.sol create mode 100644 tests/e2e/detectors/test_data/pyth-unchecked-publishtime/0.8.20/pyth_unchecked_publishtime.sol-0.8.20.zip 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/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/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 0000000000000000000000000000000000000000..178b65b38891d69b198dce4828330e4d726101e8 GIT binary patch literal 10531 zcmb7~LwGIm>cL=o7U7TG#5F=bJZ^u0RnLukmc)6iXHdRLPm?S|T3>Q@%&~*&ogPv| zyJ8rGPge&t%5%NNtE&4UF35~!TEsw)N3ysv-z%LAWUKDBJBocnNY9T`XiU)+@x}FA z`7D$$3TUi(E(nH*aAUFBlQw2?w(uJn9!MBdim$_cf^a5z(x2{hi1rytPA2>EHIXvd zfj(DpuO=aSK2&z`;X_XXVzH@9qD_Ma$yh7zapN*RY0~Ke^XL_##L)LcjJ&RMF{v#1 zoG#P&FC8~0TllxASxhGT_rurO+(JMOwQ?3BfnCf9&VyqH4UtfQBKc_b|0A4 z5RgdTrrvNm{Xv+#wBvNip_lhZsn_pW|M2|Gcng6*jH&!)R(DmB0})_}<)HRFs30ce z_UzbazLZ`afL)YR#foVtOUFHHLx#qwljp*qoH)+`HON+q47O5Pq`$pDy7!`0$DgI{ zY(9hnz|9#;`94@B1=LC}&4l4vyKFTnFPeoG^{C!5Z&o2%^6}a3UV3$(Q&+I;k`{M& zEFJcHCFKp|jNp#StE$YCweVb4gR3Y_9&St1(WY}+STj;V$r&Zb@c|q;p6|9$*a{Y` zzpA97YZOg>()hT_rtIv|4PN_MW|<0%1KD`gCbI~(UWf6Q<{cEMfJMA7^mfEWS_SAw zJGqI*BMo;I8~`w;#fX>yste8zzd^%u;usEGJtGGOF?H&+Uy-m(sPr(kb3R{E=cl(4 z3&|6oO@0jeK9ldkAZK6Y)5Y4b$#pX5CS+O$1VYb0eA*iWu$~FyiL>@{61gbHEZX)u zg_PxkcSCqd4`~zwbm63F&gxFs^%is8iP}5!I=&;8A^T>+Ce-7}Iu5ou_^8S)=6gm; zzQU{l1FFcL^5BtJTHjpJlPb)7&t@nOD6hp*9FT#)`z38ruxM{I;uwIK0|@=tu^{Sg zO3A@glpRf=Y>Yqhcq6S2!?`aMgakg*Wtp+c=bQQZ#KH8B%|>3sNuiZRA~Z(HUl~EV z54M#6j?B~kisU((_ss2yORfcYFi~P=W>HLU$%*CyoG!>>WO>pSDa$lCxj~r=@REJUU{#>NR9o8ji|kf?%{kib|J8`0GA~@7ji# zyVOR-(%dRtC)J_P(_3^;5wG;ENKaP=(?fw)54Di8 z?o5-0c)DmnNExeUzX9%tvrCyry$mJ^!%{WTB1ND9Qbx|pCeURlu&uW2cp;D0!U7a| z9F`ROu|?@Zimr*CUsYslIhG1GVb0tiNS8>_>LOa|fV_@TRwIqxv_x0B#!RuFtC#Pz z+cBGcHFNF`_ewcLZY&h(mzZ186B{*z#^&BoS+WVv>XiC+E`r{$4Pu5^bDN^+rj~z) zw}o6sOz7xN@x~2;J9}NQ+rnxiVymzDvoh7+Ake18m8Us#lIlpP zCc zKMDr{OjKT|8ZJ;k^t8>2&U1ekOzV-ZsS-&nt_Beq^s`>Sl5!l_4I|S_Oa6xIm*@Fw zQp>bOM)pfcb_f&xYUscoUTOgVUB3soK9m%J!HL;yV+lFh4sMk7?4~G$Lrc}nR;zII zc8i1LFB%Pi!*^CB1%DCEw*}GKVXH|o@1ypjW--rY{N~Na z5bI}LQ?>GuV;ERAv9o7xFll z2Pb8P7lA-%Hw-a_7moS6c`X90($snak#BdkY;vNsB4Y$>vU`v=nAxr>DPx@C`9L&o zlOONqy&jl&*OQr@T_)e$Yt(W3r|9jU0$14JoCu1uWYlO+ujBe+<$(wO8UOR$)2e^} zM`?m&s>`S41`NHq8S!qSOq6io@@Ck0f2vHzPOFA_JlbTu0sx1Mt@-S!%9bDVCh~<6 z`c2DU7iK{CMIMZBJKUiNu&}7r#b~Ly7NK?%Im=e=gP;6*XVJD!TK;@Na^~Gcg`i1N zSA0Q!cFBw}7tp0@ib80&DzgG-Nc1j}2m7lfJJyEZ$0G(JC#KR(M8^qu;ANWd8$sP* z?7+!2LfRH13v|T2TDATab-j|zjN3q)!Arf`stespD|KHI!;MxNaHh4hp%(zwt($pn z8s0Cl;#iI0c8$1ywagZgwB?w9T5nSnp5je~U`W<-$8cdAwqE{6^h#CFB=npqKUdL> zRbf7M!Zb{fpxxNx^iQ9Sgu8r;rVwV5cP5=9+1k@{p1gumR29S_&-*;N&F70OF z8PA0oMBI=k1^Leu&Q6?C8}`fnO(0Se?v&7}mmF3%f^D%jl3e0WNL6WljWHVcttOc=Vk|x$pCmh;&chtwlZWnO3u{uq5|;xd1SnK2l6_{ z(3Jkjl9ub#F_eL9A?HAEbPA{_b`jj)aV8m}MCkgT^`hhb!#_4FeoJMG^gTq=n}+Mj zB&?i~C{=m~2KiltcyA_v8`U2kmq(RO{oTelpXa7rHGE=*bs~S(HDe|qgc6@pj+uph zD-?7snrYnAVDY_UD1NzLiEBZrWWA;@fxO3{xB$S%#^OXl&(@Y+BT+nANYtWmuGd2z zUVBV1=Z+eLO=WK}REfv~y6_p^xUvtvxeB;|%+RC*XIQruMElNf#Ybpbp9iErK(`zD zR0_TINy@`HTaUDN^$CuB4%YgieDYKckCocmy}NNOSUMfLmXU3QxQ34)($r3->o43x zm%8kJ4Qu2_sOcumix#B)xppzVpI z4rX!?2DBxy3dosFpgI<=D^FzMCq+cG|1J_y3M9Z|Xw0^t)zmTAHTB|PBtX|>rM*MW zbE%=Wl(Gw)obf=3DE)&@FsjwyB!QsDFs0RgvR|N@19YXJ#_2+VeS%OQ(p6aRpK&YJ zh);(;3SH-D^OW{2(P7mb(t4)E`5b=|P#aUDWkhzaRtI;!n4Rwt^&aP#V~C3Op6RPD zlUi3{rdXX{@&qz<^~)Yu*m9+!!P7EuYYrTf{DyK)SOrWM)(#eb5o;$pyLZ#=MgH0} zGNcSG7sgraiybgq!&Jj86ni36xgZhyDPWRXqJCqh0%4X!tir9{!}Xc_kc?8(cFG5q zluN3D1=!(l7mAG{XeZ3{vNSafvj6SBmYJgA03-2H3fHvrkF}1~TD*y?KW8En-n~?N z-nnP^K)S#UY6FlJ|DsW>6vQ1NbN!8})Th@uA)Th+887B6I<7IUWH8B;;d*g)2f3m@ zGCgY;t6`9#7DXScc+z=-S;cF3Y{72NW?SdB4dYdetj@q7m%ZqN9*33I5&M;=1IKmz zfPcz|G%Y8mFd?*iq?f#;*#n(q!m%i$>HAjbM=hQ$3mMNk*j)@@*5K2Ko9QMI_7@an zp2|XSTz4l*ma$%H8XyE<?N(J`0${*>ouo#!zUiYIV!b z(w*A{EPAgoFwhz(nhal?SuP4Pl-Ar^8B>(Cq`#3!#Z?S%x=yI0Vl<5iRh2=P>7O)W zkEi7pCfveDpf;$5n;q%o;HGFO{rqT7=q{Qx^p5tCq93Sm(alZS(P_bm4w9bL1_C{K z7AZ&=rW$?~UclXL=3P+=EoGT!hM+P63-{)hb85JorlluBE$%P_-sT(J?On3BvUOWA zkFB02N6f~f*}8Hd&l=p_EPR(-rfy)T>U~;T2}|m(bO@oN8xc5bk+YnB5Myy~k4^J| ziV0liBc`#BHZKr0SJLgsDdxU%oyhN10WgpAZ6kcrGI9$x(lKBn#ds}Le#Y5QWP2gJ zAwXm#jP)O;2vCsg-tlTi8Qn$f}Cu% z9U;J{UJqw7Cn%F!yI~A|iofPuF{L^W$Ullx=l=`Xk5e?rF4)OYT_EAu)mTHvM-fI2 zO&JdDmSS=pbD-~C|I5scp)y#LPqr&Pu_=S)u*~tsa5b{VD7;FS4C6DkVKhd^z{Ul` z9Udl4QBjnnBepf&z8&y8uIdK=y<3sWWS*FelEp!zR=UZ72WS{ZB;poHyxJ~Z?VM33INS6!C_n|t1RvZM~@j&bm0Stw+ z$qFGtyND8jY?COW4+|4)PeFKHkWu5v6+( z6`Ou5vRz=7Oa?8#Q1ClkT7PM(KPZ5~OrTL)kb)PV>ms0`jjuXgtpKCfl6pLSX@}`a zWD!MqGm}@Dv>F+vAbLsVuDue%h;yFS{%D*5GU~(2syP;8;bs9tYbE|aMvi*9Ipb_d% zFZH^(qP1{KE95=q2RP7C`~cfS_02c-vOjjAX16|uM_PvFU81Ht^@KaVPQ01Yaus+Y zm)(u|M|Hy|98p0oA~R9=87 zeMM@e0ZL9D7=P9^xe61AL1>299CzDX!N(Jh#Nl)q4GRnf3no|Fq6{7B)`u2(#WKf^ z63Z?Fk|;tA4Z^s2BUXAk)%HPJjyh%MC|I5yn$%JJrM*5{8-FL`5gqj9yydg3>3gS- z2ww*SYN?O`GGWm)M1xQG;VY8Y<*0lJt>z)bG;8SvAGSvvcXLm!=;qU1~OYXtYkB#bY0Y4fl zTMSr$O0e{1PDJVIQN)CtGhuIG0So9w$2wlRyD`>9eX1itcIa3r7q+QVB(>2^U%{N<-U8QWom%3V4sg$iq*0>_+ zu4>Zd8c?o!j@}r>`6pbpyxQ{FP1{`bG(F=-WdEm9PN}q{-T~MyPJ^LCA}`3t+D6yZD^T0SXG}sO`ELT!XUHK?7fT>L?@%X(BMVbat+#k z_=P(^G^?KV&dhB;ej8Xqvrcs2z2NcGG32^2xA_Z$55cyVWX^N(bS`?@0xY zLX0}<5!Cf?Surt8CZ_m3yF<9C*nD`Yw^d`WvY2_8HgGLHVPuIlQ1A9XRvq#qiFy^^aqIte)E zz-ZnD-p~$d>noRBYQ%!h+_^I5N>>#*jZWM6JCZ%eH*Z#U5VZ?jJwMRX7%jY`8^lzU zI=1@5ruN>n2X>nDrR6bg0_|LPGu;1LCfp^pf#Pfx};JuPbp7&FkR2Ds0^iUgJMTfTA7*b?Eg zLprX8ijapFeu%1MLFcV~F>wqKTm6|D{qHvtajhit;M_T!bvV*`upx`~PY54V*>r2A zZVwSA%j8)}hYpyk)FsMXqoTL)(sK7rKu5^ueCbEZ>NWpPzm1JLt>c0L zB~w{qFK*rrR4IHOSee{@^>bI4ZY?U*07^0~w5@k%c}SllNduOH?e`3e{nMRHZHO4o zbo*RMS2P%&ozry9tS0k!R%#5~(f3$NhMxFjVd(XIhdVqYpyh@c;3n5LdsS20Ss^^T zjvaT{FV16GQM7s}o>{UmU|+S6Lip*raQP*t45#gVc2M**hoO zhm2Y0v?ck=reHmZ$CAFNMn}5XT$KPFJzhQOGhx4Lfl2p=t360>vNLz7YN}}6Z^DS_ zprI$@1eT=r0R2kTP65XAhQzrb`ur-UK8qtyz)1zi>alLH162G z679v|0apzaBPj}N<)p0`N#isW3ppGaSHgY%B)wi(8XE7i@Aj684v){fa>F{aTXe!b z;O(02mq;gH_)l<9q$o6GLaLBbyfteJQ9smsFR(VSQlzL=I~;ymgxua(d;<1pGOy=P z#7D|@9{RWp=Uv`IxkpF>R<+_@YffQeBiHCd7WOG107n#CT>YSVeO$Zj_;EIC2wWUTFopm`r-JlK!1Bun_Y#W7*>sx#L(s# z7&CzHOV4y4OIMS}s$IT1NJZ`rE&TP}w_t43X4nsAS7Aki#=((*IFZwb!i^EiuY936 z$DjJA)zZ7=ij#e|InS-{S?lSfBiJ*O>~Sr~$1NQLbilk*Kk2|0po+|n3gvQIBS#m3 zqhhS*;!nP7V-i#(Lp|oXB;9o*ymR8BG-$m2n$q~MS)iv9c2Y7BhE zN-9BNI75tsy_s|M1A#Tv8hmx|wAJ{nGDAL%CHDv?BZUu_y z%6NS}J?!xVN%`pz;VFe;Bkp|8QuUq@h1V5DPg&^A>6??9lSpz9Il3v&=ByXnxnk;H z%oft!1g@r|<&K!B*jWqJYwuD27dt(^pY&>cg8xBeUF;775afS6fOwDA*^X(hm4+m%4I=i??w zyYiwkH)n;sqGHxi&S>K84LFDUe>tZeyJ95P`3v!rcC>NMA{ z?2*X2sy7J{XL$@43}b0pYREdnS{*U5|L;4+mcm;f%iW8xQrBbR-#>mLSrU)hBz!j>Z5QR zauPkT?IlMkG%xU>=1-GQ%6tL?;=JOotL^lm4qJZ(nLIELDOKK)Us`AL6h;Oz-aQz6 zxDK4u&ahpMe(B-Ud_IKkRwjxg)?bWAX-sp+edV(@c*001PKACRB2@^^9(y7awEq!r zGqK-lzxtd>6Obm=sZ!L7?b!6;|JgXN#GW{Gnt}o@Y3Rmd*v z`u>^DJwGALq|31AaeoYLO9Q$Ms2EM^!Nk%r~4nxpT~uI9egTP$aOU2 z7ZhF+^-FF0^~_lxjAr5$^^UpGws4+Fm4qrt8=}P+jSx%tZwag-MP!Wgi5!8%O23n5n!&|Fy_>s<_~VS+vrTT5&H5|Fc)_i*U;MA~f5c zfE*LT@rPy2-5A-uDO&aOn~e{9Q)~8Y|5R{to`Sx(^k_+@tE&qL>uF9Z>%ZP{_RMqbcfjo4X0IN9XEou{KJ;X^L*!4wb5nXPuYMR`BMPt#D^zafS>x&jtMSs zi9$i5kJ^j?rLNvWYKrwz$@kJ~HoAG(1G>q3!IR3gKj;JEl1t|XX6Xf+-+2x3M{bTj z#fLZozPW!QV6bN<1UmrZ_gi5LhD%&?%9o~KPu>etBsFIdqx=kdwo@AFAl0}VLI5y8 z72WF;G+Y8))g2o_wNoTllUNVMDtT8(;N~ljE28Dpr>)U3Pfhj^sI`=z+!U$B@t1(i zkv2)d;c|$gG;{4`dL#~5Gv_%Zl6>VgG6kLV)3K8dm~by>4(A**pt!Av`**AM{Su{_ zl3_8P=?_a5WdhNOa?+_b7XSCuBs!r|1nw{PwN;@I4CUv5Fc`x72QgP-@|AIWHhHTX z>WK0zZlCQMb%^)Yt!wT@Ng|o4InO8bX-+tVaCz-O=-cS(nxwJ`9S)~hhCu`lTH$jV zJ4J1s-7cQ!q=YC1J8T%b1q#a`^W_}G;+6lIayhB{vU4P#IxO!<%_}5Q-Qful%uvO!bhCTowm91dEy<*kc&4r;Z4onyjTW_fPSRK)Pbc;F%J7Cp~?D z-A8?Dv!!|M0{$4KGIT_^L=TfxBD2W_B2e&m@!2V4@`{g=R($t3>XEpZ0;Ue1P`Jd$ z6qmPtJO~2Sks(h#+2!yfUp3&Spwq&64C)Ox4q)q;DExSOwx=JC-E>M-fE$BuXfD;v1TV{495rHxzUd>kUmZ9 zy_u~J`a`zAmv32saze_~fSJAP!alp5r$jt)18RnY{(x2WYC2JpaA&`SE8$f|0mh8$tLXE zPn|R``UbC>9rAIEzoO8D^M&q9w4F!HorbX!rIO)^#CMHKwS}2Rnh zrfxjl1Lg@f(~ki)Kp;yt)E`n}JQJvLcJ6D&iCl28SAjuLU-*?M;=Mn@OI4qxHh^bD z9klijjK@80CecnY!TQQ7a?@%Rag}n4inX*YgsU~?P@-Dv+iMGV0O?V|(IW@=DJpW@ zHjb;!BOx#~ivp(GMh9;&6TLI900NzBaP>73&978n!;V5O5%CeOzdztwue=BnUs5im5W;0V^ zHHMRL6fl_;eO+;vRpqFcA%{5Euov7md;?ks15Hd#J({)@FwQL3*z$J2gU8Rs&brO6 zggG8}45pqFCUr{JmCuv7k-bJdjSq7t>2+|i&u+B@^{E-b3s-PoJl${C{pl6OZYJYh z2iywlcUBDPUZJfI-}g?}!Ue0eT%aEiPMZUe62rgDx?K*E=WuSx5%6UnZWTDKCwJ27 z3Et;ZUX2JM)S76vf3cl<$uzckrB-zy51pqGHiCr;i+aHz@IS;R03GtQg7g&2jp6vt zfN#0id=Yjk!DqqwEg8gX)ZUp`WfBtoK7R<$&yV|ZS`8+*X!_#Nb`1)@0vYXv7)ZWG zyw_Bc$08lGCXM);=O)pgpD7N^?Y0{Qjp<|g3}4C!wLdto;PEdj(t+D$wIZ^7`VK+-w15}I5~9xTj~*KL7M(cJ0gmI4oZ2{H+WK@S zeXuNGb=-%V9ZIMW8(fC|RS}}kLZYlI{fEzQxO_BIpNU3J4VG7_QFEc z2$XbPx!Ox`zXz-Ft}O{sX<>(n`()du*uW!l0GF2^p*y{J1j3n+q;y|*#kV}rK>d9~ zvthp>HV(#(qRhu3r8zZQyFtZN+%ed+E{iKxo{+1$=6=j#=c72QhLE?yghKjK{+E*7DPWROC z5@W3u>Pp9mfw@M!iaNtv!_tLeCdSrewPJxcV&&3>7C zK^xfMAR>+?=p?x4TKx(&O96mwIAClYD@M9CC@I$S0?`Fr=D{kNKg59wdw>~AK)kzJ zUAYTzZ~ayJ#o)j?aV}g>NLEEJlKx3Hf-rB(Y%8@7DX+?ArzbTi)i~ti zRt$NYiphwmoFMk9lqqSO_*t-lJ2xpt7F0$*nb6|>rNqyDjx$pHX>gUb+|IGI%sFC= zvVJKJX|@|rn#c+k;!N+6M$fe=l&jsZ*xMSCbjb?INjmvUG`h?2a2lX#LYodRILJDT zc4v=NqsH9wEvYnw@@byqQopWS7zFxgXs3cH`#!$RFEFfaZfVG_<oxg6u%SMspNw z3y~QYr5A52iX@cVN_=6yC1oTMSl})DTg~>7&Tx2Z!Ug?cNdNh1`CRFiWZ!okFKZ{9 z7Zk=uv!*m9-G2R#AlIvZW=>e^L&Tp_Ps}gsGBD2C4iJ#}+yMUy?qWT1$D`G!lQuy<-u%pMWE*tt$vP z<5mj!9>_3#kw&r2Ns1c~PaKJ5rP0?-6+EonYls(dy_mY`fQyQ(F!2(RyHXe>Y6DJ)qR^ZOzKIQkY^TB8-U^61? z595r=W7+k-k~J!m`OPtLhH#b?~8AHeM`o%6aCvDsISC) z6Xx0m`@RAO6TaghD>Oe#RqE-GVm7IR-D&iNwb!);1@d}rQ7T5&<7FTBoi^lib4S2+$q(a_>OjZ&w2k>@LOg<>L3OwAy8Fr}1LO()IH*d}GXAbUVA zZS}1F7kP`#Kr^B?U?A(b1|4dLYkfx?$vu@-!Ux$3AE;3^DLU{Scp zgIJN5jFc&0ECaYVBuvORTDPr8LQvs7(F$(iWTRHjVAdm6GS^`7*1{ zNf_|@dzZgPt0)5uA^`fo>x%ywi~oQ8f&T~pcX?4!1{~sllEDA1;eV|8PlW^m`hUHm BQ^^1T literal 0 HcmV?d00001 From 24a9d9d36f67d219bceaf7120db14e2c6ed5817e Mon Sep 17 00:00:00 2001 From: Simone Date: Wed, 9 Oct 2024 14:59:51 +0200 Subject: [PATCH 11/18] Add Gelato VRF unprotected request detector --- slither/detectors/all_detectors.py | 1 + .../gelato_unprotected_randomness.py | 78 ++++++++++++++++++ ...0_gelato_unprotected_randomness_sol__0.txt | 6 ++ .../0.8.20/gelato_unprotected_randomness.sol | 61 ++++++++++++++ ...lato_unprotected_randomness.sol-0.8.20.zip | Bin 0 -> 6629 bytes 5 files changed, 146 insertions(+) create mode 100644 slither/detectors/functions/gelato_unprotected_randomness.py create mode 100644 tests/e2e/detectors/snapshots/detectors__detector_GelatoUnprotectedRandomness_0_8_20_gelato_unprotected_randomness_sol__0.txt create mode 100644 tests/e2e/detectors/test_data/gelato-unprotected-randomness/0.8.20/gelato_unprotected_randomness.sol create mode 100644 tests/e2e/detectors/test_data/gelato-unprotected-randomness/0.8.20/gelato_unprotected_randomness.sol-0.8.20.zip diff --git a/slither/detectors/all_detectors.py b/slither/detectors/all_detectors.py index 44a168c2b..c317b6c0d 100644 --- a/slither/detectors/all_detectors.py +++ b/slither/detectors/all_detectors.py @@ -97,5 +97,6 @@ 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.unused_import import UnusedImport 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/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/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..1a173f7d4 --- /dev/null +++ b/tests/e2e/detectors/test_data/gelato-unprotected-randomness/0.8.20/gelato_unprotected_randomness.sol @@ -0,0 +1,61 @@ +// 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)); + } + + 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)); + } + +} \ No newline at end of file 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 0000000000000000000000000000000000000000..013d3ef28d1b71bd8543ddf8923c81735f85437f GIT binary patch literal 6629 zcmbW+MME2af&}1T#hoC*-Cc^iyKC{{8r%yMcXuydBtUR?mqIC8Ah=5@USQwb`(AeD zFsJzipQZ`|q67dQfCW(ZRWd4TSm+=j0RZG-002J#0I;)hw)Ao{_jYynaPzXU_OkhG z?qTWr+0Dh(#?zC-)6Lo3#?Rf&!^_;#!`j}#$A-hv)6EqX5eZ-l0E7YnVlgpxy#IK? zrHgZIBA5lEt`7A56p@MAAaTkS&t5|AI~nn$4N^X~OLNM_ zX}ZWz?z}Gi@|W`YjxCwX9W}!ih##3A`&(E!W|{!j-fPGydv4=nZUrul0-dy?#>sNS zWhTc)v_Vf8r;~=9RvDRks2E!{xm)>wteNnxW%2UyHA_RwE;}{!%Rc5zZOyhXu{i!6 zr5|9z6yb3+tJ-4-CMhq%WqW=-YS24CJG=Ml*)`)bpn=?xarc}g$wE1m00*<}2?ePp z+5DaA0R^tb=#zvDQJ2W|GJgbYJ;WAu{0N6v6;%TT!=S?!cPOVjV7fEk&Sm1HkLQSS zl8*gbs)@hYyB*1b)(stPv#<11|3||mmtd2}me1z?Pg3uoz4i@RUGjOVFN}{AW@`7> z9Q1}nR|bwIa%V-#d30&GO>@t*c#;fS4XCTt%4@>?z5lrK>1xsE_}6Jf@p0`Eh{+?z zIB4a31e20`=tJj={YA*xnb3tS-FFie+%d@B)Gr7aQZ2^Tiv1d(u6NZn=ck6#cG1*c z8~AoSlKoeCe{K+-Sy8LMa`rr7%!i8Gi6L`#QWm54#O7WB1Rfy|?z_5GHgJ;;>w?ng zdlZ#~Y%}2WnL{&+bSq-iUz~?WVoa;|?J-D++T-Si<9k~=pP!r456#GGnT8u(Pc!b! zP(Botck!@m#;*u+I$~=ma7H+Bc_Q3Q&EHF(#-j-~(b;YRG&~`+eVNuU=d8wAXjh3w z(*^O9N#E)^1)iqkj)-?zyA2z3&s4t)51Xk)U;VwEX0ecORY$!oGF;_!yJAaN2_EAp z!1)|$^7^ll_j+wbzI42y!1w|H|0-`9@4IJw=MT{aD4q-L$1l2bJGNd+qhayt_H zWNM!EbMc+|m^{2Kt0|E2hL^Cz^)=XIR@XK)V$ASTZ0oM@ouSf6U$}>i2=MQAK_&3!O!T?WeWghX6+&@h>5#je)x*iIkm@wTDU9d*5|+&{MUrm%tn`}%8 zs<}78&4$2fg?}#Jv*ACDkrNo;6Fmq|hN(ij+IK^cKNOZhGQ9C3n32^IJ)1q$diXNL zn}*iXjX;VW9nG+P0f-Z~>{A zIEDZNCZrx17e!!a^It=Vc($O@Qcn+y*hg;8R{bv-xxBh4WaK8crkH%waPwDBn3g`2}Z!cDzTkWy!sm*5d~dd@G)^@Q~M0PTV{>HN5@f&$JcFm z%r~A|c97b)1o7VLrSGO&2S6U{+9b1o=0DoWW0D?)i~0_tPFO+{G{3?oM#_B?ld6-` zJW8{`Id_EBa1Oxl2 z;&`smo1_gNnMshQw9K8Hk{AVsJhH8yn3xX;H~Y?Qc~0t zF+sf=rgk4A7X6!7Khly1SIXxn-6Z!6prx!aCxhx{yh1J0qF1AK^A4QfgoWkt3Fi@^ z6~>@h9*Y(|bXRD#Jj>qWcP>-(3i?5Qh2*^3dnlc_tv@zlE}-w0aTH6TWFJ7?>@%rn zCtnbS=Tz*#%MgunqlrEg)IdvsXmR{lZx;1lC4JERICwbyqhFfl{Ys`}f-|+4Xc?UM zS3vN7-iU^Z@l@1E0Q`mlf4YKgEM6UV{VbbXDuR>e{XSnu7R?@Efh*6R_>Jk~r0V5A ztVpfDmpbdw3m`Kw`_HU?;Ov_F1+r@Xf#UdK2>(V$dnR6h^Vg%0`24y)lV22jD87k} z**)t&VO>Dz^)?UB6}@bXzMb5dytUoAoCc71Nzu-AWROffYD0B0!@q5D+fWC}ZU4gh ziRXo>>*Gg}(A746d)JV^w7-wce{%%r7cOlZ%Zm-cpqLca2SWlmwUujlTMOT1%Lm zKa73X2BT-tO&d}wo?&h>cD__|%i1~Hn^QcG5l*=UwlLwmpUP3gY3{EJf9g;T!C~rm zbkTl<`5XKM%Z8#8{_zgA`h0$f7x7hN&p9`SmO9;QXy#OL&@*G zH)~)eKarq_A_jzD|ZxD*(O5J`DpQ$LU4URD4|HQ$FO{kC6g-EI; zpb>+l@G1MHsAy5Fx`@@-{H@uIuC!nn?)&!TlADCK5nV~dnzOzjIUpnaaJ zB{|>V0F;WRzV{hns+cY$N^0QAK{tUbG_vITsGvSScLpzXgt@U ziii`@nrZrBkD<0t!Lt_UOxu~qjP^#ijUf#m!4ETSa^`fot->U}z7&}`@1dpZWR)Xs zSCM3lkktz`)~T+b7X{5t)Yr-d@U}6XPvEbgC6S$;Ewh)tuIkil^S@8pW0`8r6ht9U zp&|=df?dv2AAx#ABae&Llb*kbIrFC8MHV(h&ejM~LG|n6M}^e-@_p?2hhIA$mKI7J z?S7{dx5jAT8ub|lSL|{x*fuW@vmdYtyRi3^E_@|1?M(FLoW@xT3z%;AaSXOKD2tdJ zRxg#2oAFXDDvFP`vrP0~M9zwrp4%2H8=3sTL-qRuJt&Y6f?s?!O1H1!aN10019%Nn zO?;Qod5U|2w>|fbdYjZrXftg5yT4U)yorWh-XgZA)%~-AbQy(^M4jSFE~n)Hq8KmJ z6+QK!Y%w>H^)7lnL}*1!RdAbn_-*~}dcoB3!8fZFheLxYWkRO&p_Qy*f}Ci7`x{tC zq(%Oab;fI3``4U{$K4mX(BDf0<)2B|p`eWwDsR1HwN`Y7ahXvW{G+L!`2bUNVB%?# zuu6EFsYvOC8o+>Cm|pdm8B)UD?+6#(qMbccTbBAad2q)_P%uUPsxbvF0PWfk2#RU4it zzV4T^NLdF*AD9-Qh&X6-VGrU`D2`8m$fj-!&mW*-7sJ{jOXpDv=UDV2ZjF!X`SJSF zb8yloPZty2&m4LAWB*Dd@aLUFE+Vb(3xT`5%Gxp`a-i>SD>}6C>pwP<&N92WKD?fK zvj!3egG7f=jQUU2;a%7&!@rNEho;LkD`-h6zOriNS@ppI4@K^-{yl zwXjKkx+!SeUWaR-E;mOukDWVW38xt_T?ozz2QWk@+prCZN$@b(*M>ZS#8^h4cYpr0 zdkSsWC`dJ$j=7q>OW)A}o8O+b`&d126a)_K{#Z-6ek+KWYNx?ZD15vq9HXE5j`qkG zGX)Yy*+J!^5WYsJ4s-T5I6U*cq4D#G7joxL)*Pbp5`VB0xNviqaJalmc6`_b^myL) zr(S?1W+v~lQ%tXhV70VOn*e)~*`FM7Q*#xe4qQ#i_u`uh$5P$p(@L_k2g8Ll+&}t- zN=I%^%xEhvza%&h0#y38t@5uaihJEY{56G_tIf{WJUV}tHU(E@JtWv^l+@*Ss#iDW z%;s<5WD;SPuD&2f7X;|A)(r+U>aCa8yS%LG-jXaT3^05B5=>q)#Hh3~W+A8oRTfYh z1r{7Kq5ZZ?jms>Gt}>Jy9skg+62*-`#4RJGgPTMd16R%N>*`OYM-m7WI7EzcF0mP_ zOQ<+&0%J1N*K5Mp1!%6_1JAXeKD?GfBXC3~v6+%dZD=h-a(7^g0-RhN#<-DJlOgni zlRsbk)Vf8f939iyg1&;o*u2*r$9QZRrkaS%;s;Cee%Ym~jUGj5eo$IpnCqi%Sk!!UE?DEBX@zSCVGKsK;a%o;;T@ zK6?h>oZbZQ7;_NUaR;7tBx3M51h8VpEw2}+RUDKg~g)Rm$y#}!;Y>_q@Qks zOvS4GI2uOFZv1aX`KEHljbtqZSnm8Bbq?2;(Sw}_<@NnyUUp=;wKWL0k)LfnEYc3$ zBM>kyVuk&!IoF0d-j^11Lie!x7b+j@(+muiIy=7qYEEr4A^urVtL4ZLb@%UEcX~Sy z>_(AC`rkDxQeJn$w9!ni`FOSEJ*^%oTLZE!xwimgPsKcrf=6Y>*J=mKZ^?ZNhO>-2 z$c^V9%^LO}8^ZhblUi$5dQ=*JmACO4mkvKkJYyi+juxNX=-XPwaW_1=B3T^EbamvQ zO-u2qW;;HM?_`nXgzMg1Ae@{&*0#KsbC=v)^Hv*9%2aCIqhg`rWUp!8)D(Y4#JJc- zGQjxLjeTB(p3uB6d|(npXrrxf6kH}e^1`7&bZ+|xcV1PVC3VhSf9UGO67mkbi1+K5 zNnQzc{_24j5u{2HIC=98vdiTM&xw$FpLmok^U`0Gbii}83NZ_@p9Dx5qoV!igR zoL|nV5>3tC#cp!Y#l&^4>`%v{8Ri3s-G#-aK67L^<+fUZL#Uen%jdIVV=*Q>t*6hC zLY4P)(#?m0SajaY57nk{-0F_Cf+8v2Z$9!NUZR{-f7I*}O_C$8U!VlR;qWOdS~&Oe ztmjxRPbjdrx`*f0jjCPhA26)3BA>X7bLB;9+<>GWW8q`6W2`~Pc|^<_C?OtTiF--);KqWh29 zchxwJe^~`TQ9N3U@Wy;0idlBOZ*25}ujf7$lZ~IY_6~P(=ZxH1aZD8*$g2c_6`$T! z{Tcn|v*T5C7p76#gNAYuElK_502+COB2iI#;sZ7d9Wu_InYfvjLjZ-ULa4L3ttr|KAiLBC;>nY+6b%=O77s0}-=_w{0f>%cuX%8&>_?Kq>- z3fmhyLSP~~t)%zt!7gp9)=fzA&03EHDVV?|#yhrWXODck<<|*H=?nBJWgdbhZd&#j z=M_sfy$IE&gg0&aWW=*2{)0+7;BNi`pZ@6(g|SL#@MR01ROYnI{6{)*=wPD>aY?e< z0dT*dEuSvzYj0n_@^@JGsP*QYeirdbA#*CYaAiMkGrK;WOYPKw? z;!d*nS&cDY-&tEI8+Xr$7crl%r(jr^)30#A$1Nrt6jLjd_bpNl4|$jq+VC-XFoxgb zjz=kLt~zrmyaB=8XzV)E3pIK=i`(1^<)FbQ;@)ArXd-_8HtyLE11Tix@!R|Tcbl@4 zxKbYU_~D$&^)MlYWHeLn&fa$3fBjF^tB@?wHJ&5NP$L%HEiq9pQ0P)n3{T-*;8dVz zq2gDl>^b^4d;;qAx9=7Svt|SD9)vUy253nF@mvJbPjE}y#5<~d@faoB?9Vnu>|09rDV*Sws?BUQh)KeJd%Rbvn?h<@<4F& zpaWk9E5{lu@~a0d*vin+PqV70!?Wd|Qu*#HES-y2f^Fhh>Vca1B%XjxqH1KL+(is7 zZDXQUphSDm(8M3-zB@yI6{X`mzT5Mhzkrws@qw*CFT`I_nN?uV!n%lC8&=(9Fn zyD$0jh@b-7t2Jm3pniq7c}x@QHytNd&yXHjtJ{|~(}_K7o)XUt?vg+9 z9RbgkZUQ(xn@3)ipmT*AeLd&kd!F>j&L-TfPipq>d=0DyaGP&X1*tEeb#`iYb$Q$d zdmD*$W^zr!AD6Y+Lb~fq>Pb#8ymSxQUxd&mpjnX zYf9XV2<({)8_@i08c+?ruEY$tiWDF+ebtQXxWl z$Zhq>lvHIFrt-2`!A{4ga)B#bj9r1+FVu?lfZwmT46Nwg{#z0!)#!qG>=6pQ3jIlN zMb@Ch=30SRi3d#dBm(MLeq_R)k6y@%$_lf`8f+O$Z|jwp0(7Spq)Yj`q%Qn_K~Eb- zrh$1fa!Ojw5QwKJ1xw4ypqmcwNt|e}Z_miw(@t{pZ?bV&@0xeppw^@IQxe}>)i7GVjNpaI*yIu zYTH&5w;LX66a{g%>N(7L5CM@jQ~~q%Y~#Z~`^8PB4o*7eVB(M(qpfuuG4g@EGSlcc zbhpuDE~_|>`;nOEj(N>WfkR_lc6x?Zn?Cov0UANel9O@DlOc0gM5ocxG!?YQxnep> z!m+CdF1t2fh0HT*JB=gH*rbeR35#wYL>Y-&W}-?x8z%k2;T&s@R0!FGlxR7=roEES z9a~2aa>c;7816ifesT$o4?bk!>bJ`)S8xk6wEGUM*oi8Rnuu7=$cG4_FV!65ZXge{ zEggEZ4z$%Gz?82xHz6{JmJ>HIY4O*_!Mzc*O-HGu{DPqrm>SYri9puD7aj*CA`qZV z5-L%9-r9;kBGTx{{xNe By$S#T literal 0 HcmV?d00001 From 138b62664d1295f617db47e8d1dbba70c7256a9d Mon Sep 17 00:00:00 2001 From: Simone Date: Wed, 9 Oct 2024 16:53:49 +0200 Subject: [PATCH 12/18] Add test --- tests/e2e/detectors/test_detectors.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/e2e/detectors/test_detectors.py b/tests/e2e/detectors/test_detectors.py index 2c6a5f55a..a1ec1cf64 100644 --- a/tests/e2e/detectors/test_detectors.py +++ b/tests/e2e/detectors/test_detectors.py @@ -1714,6 +1714,11 @@ ALL_TESTS = [ "out_of_order_retryable.sol", "0.8.20", ), + Test( + all_detectors.GelatoUnprotectedRandomness, + "gelato_unprotected_randomness.sol", + "0.8.20", + ), # Test( # all_detectors.UnusedImport, # "ConstantContractLevelUsedInContractTest.sol", From 2d46a3deaac05eff66996d32a96d163377152439 Mon Sep 17 00:00:00 2001 From: alpharush <0xalpharush@protonmail.com> Date: Wed, 9 Oct 2024 11:17:43 -0500 Subject: [PATCH 13/18] Update CODEOWNERS --- CODEOWNERS | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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 From 4a9cdcef2c65246c023c88e14d469912b86b9451 Mon Sep 17 00:00:00 2001 From: Simone Date: Wed, 9 Oct 2024 20:16:51 +0200 Subject: [PATCH 14/18] Add Chronicle unchecked price detector --- slither/detectors/all_detectors.py | 1 + .../statements/chronicle_unchecked_price.py | 147 ++++++++++++++++++ ..._8_20_chronicle_unchecked_price_sol__0.txt | 18 +++ .../0.8.20/chronicle_unchecked_price.sol | 119 ++++++++++++++ .../chronicle_unchecked_price.sol-0.8.20.zip | Bin 0 -> 8287 bytes tests/e2e/detectors/test_detectors.py | 5 + 6 files changed, 290 insertions(+) create mode 100644 slither/detectors/statements/chronicle_unchecked_price.py create mode 100644 tests/e2e/detectors/snapshots/detectors__detector_ChronicleUncheckedPrice_0_8_20_chronicle_unchecked_price_sol__0.txt create mode 100644 tests/e2e/detectors/test_data/chronicle-unchecked-price/0.8.20/chronicle_unchecked_price.sol create mode 100644 tests/e2e/detectors/test_data/chronicle-unchecked-price/0.8.20/chronicle_unchecked_price.sol-0.8.20.zip diff --git a/slither/detectors/all_detectors.py b/slither/detectors/all_detectors.py index 44a168c2b..d69b20820 100644 --- a/slither/detectors/all_detectors.py +++ b/slither/detectors/all_detectors.py @@ -97,5 +97,6 @@ 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 .statements.chronicle_unchecked_price import ChronicleUncheckedPrice # from .statements.unused_import import UnusedImport 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/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/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 0000000000000000000000000000000000000000..746efabf66030b2e0990d1243c6cd27d8dd38c32 GIT binary patch literal 8287 zcmb7~Lv$q!w5(5T+qSKaZL8BsI<{>m9osoEJGPyUZ5tR#J_Tcj}2@ZH(8l-73dqw?k+$_ z_`aDDHS8<4VU#Q#*CrK#=OiuSmme5PCQ#V(aUa0=$$vmzk*apiJCyP<=VJ0m+M=Oa zChWdz<6|y0IJK_ulc%Ej!>%uQagf2`NuYiV!PSVodYtmWTuwceBn6s=5qIi(>%U^I zihxRjULLVjW|1N@ox^Up1?nH8VfWBT?e-y|gAJlO8gw3qR$0#s=s5ZTsW%)PL9o|a zMc%gQw7P@}1o!BN;Su;HV@CwCT71WHaYXqoJ@W)IKBI!H`b*xxvwWN_iL|9t7)#jU znOBOgn7?Ko^X)$@Xy}se7WKGrB`5I|A!}p1Fd8gtfM@P?*V3j_n^tM^bB_JY5*hU&UaU#jY)`8{J-r+ zTMjmiLd~(-sqWu|K6#T|mJjI~W$TY&lgmYwiQ})usiw*|880&!$9knkS0^Xyo+SFe zol`g+E>+D4pFfE&E5M6^l_+X66aA{1CzR6F-VOaowTyq}WXC4%u|K&`Xpb=Kzj5>3 zPFxXh+Z1W>_@4h!SwLp+w-}?=o zL5lqcwkYIf?T~dzSGY5h$~X&`B8Gd1LiI*t9f6o!53?~f__VO z+$4=^Lfmov%_LD(lFr|?o;SY>u5k#J$$@c|^3Zjfs*UD}RKZ?qEL82b&2ppW(u232 zrAF-G@L`sIT()L*L8~{1>K%8RDytpuKVn&S>kix+bx#PtvZ~C2XkdE-HWNA$$>=q*A#}%d`zNCbxTp(AL_0|L zn3cuKtKQn;Kbg`Q?+>qs*sw1-LL7?Z)ETkgw%WioPu3VDAZGubp_cdubztUIb@}t+ z7v^BvjTPcfJvpnP={kBCb9rb?&q1ast@&+Rf#LL7l5yEWtk=A#RMRO=T5RhPm21x$ zrEI)FjxSh(c8%L8yWAjO>DgAhUErza62+x@ zzoSo$7B&YNT3K^s#GN-RoGrrqQ<{8r>$o^$82E?oeQjo3J@EFe4%_#7x}%XW;LH? z2@m<0m7OuM#U5;@BJDho+WFLMfa_zuT4&SUhYeIl2=Jnt(G=@*j*`7qX~X^}LfdjZ zH2P(VQLp=Gb?|1#-JA|c*kge;a2Yizeh&X}oY~C)#^eG+W`rTMF_0Pr?<+fU*%zP> zPQkcQZx$QXXc?`h=w;WEm;s6O3ii25K{uy?U~&y+z6!y{M3xNYVOls9H% zwVtHZV;eqt%x^pvzb(5jXV8pFt$FyG`-oTd9`f2gH+k<6#R#Ldp%SonDJPH#+$O67eFe!E^_dhZ+z=&=*B)K=53PN%#2+#dz%SSuwkUs`f*;LYgqX@a4A9Z z%-Ug%JcI74Dl$%&9TkMKhjxW2dyJXg{wJxNd4NAEBy$>7!_-}zr|6N{!^%XLDGT4e z-TAU6Lm&Rmk}2#I{|loMvtt-9xEaYE(RL706pdDfb+jmInmQ`_h@6vOrc%eB;&qcU)n$d_tg!SJR!AI<_wDH6)%He z4MZM0Y;jZg%bqPcx}%0aZNGmnIW#aJ={VpnF7_T57iGJJ4#`F+i~q)U$Xb8vh~1i| z(deTkQ902%XGLwCHDep5j7oAoAIJ|3GeR6UAA(T9r)iw7=`I)nu=WVp6>%@!M{@}L zDfBdt9>cZ=?)UDJZ0M`jwPxp}pm zg4W-|Ft*itpOc?bl)3)J7-QuTVwwEigvL)V9=g1&X|&k-<*7?z*3Q_)s1!sl3%_qp z&YS2 z+KY@8f0Rnk{)Jz~w#7sw%Sc|TrR`T&gOx*#(%hXNh^0p@G(&f>>p2aN@O4Y#G7s_} z_j`w~Rc)()1s1CIh%WdQmda)=L$?ifZP06O6uqPtQXDn0!Ch_48;Qa?-B4bfL~0x? z&ZRhwj#$U+=n>AkRJbGnts-|Pxj;Fpsl8b2hvYJwJ6s?W(kYA7DUn^3PeOs9MbZ@aMPH`}KM zqsSfQq2^S{Z7H8NLD?>Td5DVhi4MZ!HrND5ZA8yGf{uJMIns}z_ixvA}k%C7hZ&0M+27L+8zDDS>R&$Qg5LVsvm~`VzinDVHVg{%~3Bou{Jkl zX+c9hIkI4(Rgjo!9y|M;+9TuIJ_6Av|5~Rkl;mBTKyXdI1lB90-9bjt5jcw(V0n+b zj?<-8BE%RE1>!XmgyqPfjN@6|hMDGu4^1h-H3}SxBuVV_VqMkK&Nf0m=SpU3V-jNQffqaX7n#FEZQQ@_K7aXZDs!?wuYMg(B)#bS%*ofZDoFYjyI|VBlg$9 z`OiieGPt07k|~)QGmLSY@5@27YktJwx>!Z(b=05LbNw5fltS?U(0gP3#FZq z5y`CH5$!~+YMJg>kCRK~T46!wSbsdj)`+Ha*V%?*;=!4imD#KP!g5R|i-mGMHOnDpR5qs|P)5eidVMO`=g&F_l$n1j=gpU|l+ zY>UNF??bKDsb?N^{RyWK9Xs0>9<$UN+&p+I449{~7(2jWh1UF07f+!CP5y{(E%~Et zBjxIPNqi-{taiA~|1Dd1YW}C*q0fxy)>L%)5H1qt;@jayNh5>jvD<19vxPsufUj0O zSx;VdSzo_u#p@@RbCaC*CniniWH|koPsl)7~8^WM%o`fr|!p?h*szEF3?=`OI-o<8*0;FFaGip1gEjf9=`_h zS>57d*nD2vDAfx;6Q{i60F%}m5r00a^3FDB1txg5-#BNh-B$sboz*mLWAUTbYkgFp)p4Dl?ioO*fP z#X?;a4gWASQp%W0YG&LWDU+rkZe_K)OG zk-@VM-Wa$XtIuxr-1@1FWk6$P9D=~-h*PFc{9Ec{H&&Y<5^n1d>bp4c@VCeAA@FII z%ajSX1DX(F$r*afR>9bZU6sGE>=d`QLQDF-g)NdzHt;+KfCI9o#8rIeF(}6Q_}Vi< zBg=NNQD>BGf5NR)exSAfrBFZ80>d{uhfRvuzbG_HK4&G=uJ8k|Q;Ewsr)OdkX=cXq|h^JrQSq$S7jJ zBKf15?OXf)2zrXhHO&JV^(qTL*|^P7@WOllxRj}o?=3~;mt2l zDG%33o7_Ljma5|)=u2x~Y7Ot(!lCU3lmaUf_o}!|WnZHDv~CiXs24qc0NERnQ~&Cu zTa|Ou^Y~pmMjQz_E6+}s{xWt0Ng5d|dBLN&Jd8qbQ$kBUd%8QToWZ6o;|06Yy-{Vp z`Yr5UI`H4XDmhHSN$y8L+@~R^xInyj%-Cb29tKTUM|uexEPfhy%}Gld`fy%s(CHmV zbD94;P>29QjI{u5F87vB^mL@SR{eTg!uE?tpf5f(2ESsPD-Hr2*cGT+`0K<(Ma0sGEX2NUejzlkc12AF2zP_9du5^Ou20YTzv{RQv0jaJw zHsK5k-zJ6z=2SOigFj2VW(rgx8&@Ck=4LH;e(c2}Ae9SkZ4uqw4<01~ee(}PQ&=OW zn{@tM$(fdEZQ}!RaB$(S^8<`Cq#vpwsPxy(X&4(lsjY~{s+u?8@q*36io6j}cKBXNXTh$ML&Gm0AhLCuh|J(7QPW)svoI7Ogl8LjMJa z?0b~8#L0ArC-ri~iHMhZ;SgfF3BBn=L=laEw!j<4t$*AAlmP6{R%40H-m(#YV$-@kpf)PCdoP!tmH=b7HaeK zHU@t$pRW7ng!O1scv8=7(z_0+SXQ~t2|nw53{LvKaC|0;%+qY8pZ2f{*4|VS&V6^G znp4`nKShWx`Oj)?^`(RGfQ=N%C5j-`wV~&x^mzpAr*6nhqednAA!Q?KFEYYxgSx%9 z&BJ;tKW6?81k!uAdBYJI_AS^&dieI$=6IJHaM^&d4o}7Oq4c=N2X6T?H0z$wRU#5I zkjOKK=(gPM1iR^0v?g_A=W&VbkMDO|p-#;cWp@OHpL#p}bTZi8S#(g#AXxDMMHUlT zERDOx`|!hp2W!JD;-H|8WDld?x~Afdi~0KG|4mKLEgcyW|H3!LCRi>u2pUCn zw!)liHEG%8@mHkduwO$|zBMtw2*{7K@#GJ`Cl4-^QtaxFfo9trxAtbLRC$qWzYDE= z%_ex`yw1v9_SlPeAYYg~fPmTBs5QayopajCZKBn&H;R%DE!OLT|Kh$6eX?CGUtb$v zSdbVvzV$|?1dhTOe4U)9dre|KS;eCpjuudJ)!zj4ccpx;A@IEZ5c>nsn@6I#v-yi9 z6rn=TQOsG1@`#8XEcO7FqKz2cJizFt7 zkxlZ&1gd^ik}=4M)}jouBQu9?qT@wm`tHD}LQq>Ji#L%#E1E6gY)CE8Panl(I#1Ju z54Y@|ufIrC0HJuS=J1O`p;Ph-$y7zk8a>F@2hz;4FsYjdUwWDKw(DWytwY!y`JwHh z_I0<;XpM*6Sb{xbjZlvrg1Sm;_Yic{Pxp&%K4BK}+Wh;^BJ}&upfJ!Vxvn6qXMwv2 zbq3@D?~wBb3IZ?h1KC1LmFfi-MSQR=VP-9i=eaLGu%l*>`YL8+)8<4(*63)&=NjnW zCtcvM%JCLAdL=aS-X#)jGhVOl+@s6vul+N?Z#jRuf0nCKu?1lUCJjIDvA_N49dYQj zG7)nberKs$u(!N%l7IWR9RWTUmP2Cm4A$L~N|?4aTV?ltPKcbl_zAN>(@kqTXN%6R zUhUr5s;_-2#_DxF;v4S3$>^t_e_-FKktJqyoL{i78)6ZIozIW-NLUQ)X=H#%79rhy z)4As!T4(MhbXwB5m{Dw8!73-6@$vSsB$F-p{@Uv3F5~u{xBU1HZ)Hh>&B;$o5?y+L zc2*-vCeus?;Eo?CJu9J1zkGQV*oc0hk>?eKpQ)3puo5@18SU(G7mXd=*w+5(9s)__ z+aM_}!*B*?{&|=z&7b^p+-AIbnAdf}NsgB|KEj;CKNwG*kK$!1litFMEB4k(4!l=zK@A>1qTs zZ!2li8aFdfi34wesoKuFlMm^sPh=9VJ8arHd;J7x4j@Kp(39{=JeunB%FI|}~l}Q}3Y)Jg=OGYQjPWm?~NzVo@ zkvxtB5;*?2X~!+;&r1gTIps_VHPc*p$V|;B(7*u0tdk#~giHCg?T$=!zLODN6zJqL1h@%aRbeCJH{&d-yYB*5jLfgm(z~@Ej#`U>AzAX< z;|!uLB0?`v>JKg88?VQN>#2H4wc~Mb5{4+u(_^p2aLn`(nL^CP4Og&}jDPfcmbj|a zs8RCAB`?P$rs`9{GhQabM%%JTQkvvyg08+<69ZrwvD%&dc!g0;+nQzKY7*CdjxUvw zC;gj)qqLBwP$ZsHD66TyWRbG^1zUkYJ-bzGV62Mjo=yqCV_e9vY`!Vjq)(17kMAg; zi{z?70`}YcT#fgDo`Ou}-MEmSSfiA_&q@kR=1N~ea2KY(1TeoO7Ed3Z9EYQ$P$Jo- zd|^yOe^Hlf?% zzgE>u$KLUZ&WO%~G)F<0y?oh+Dr_8BCiP$j-kHnUVb< z_JtrBtg{1pukLUh8wpL-56!?UVfDa=Rj23H=yl#BYv|m%blI9)CUC5K-*ANSuLP?lPovd}meA_BK966^B}}noxDvHi`HV?vEru&>efL!`9P@%9;-elg zZ23iy`&u)x>bTy~9`B=^UW5Fb+Qt955(P!#GLgVb2NzgoJ53fq8t(Yh)**>DVE3+H4%0HTik>6nk%Bk&YF+I?((ur3~ZQ=k#wD3UBP9@{F ztXL;k2sUU3^=KPtWgDE8$yZ~qpCXJ?@b5K|w$igQpUjs%+<1~hh7=PP8Hms-N3~K# zQGstlxYDju?>UvsnFaeIT&(%Cl!d4smCAqHsfmtrN|6br5+5tV_zI7>(P_m^vL-Q^ zI+pM$+Cs2*saLt{$78|M%>L|V{d4asw??b&4;L&okc#~@k%g*0=dU_cUfUXfGAXcg z7Kbcd5N-Gj5&kB^q*~5AKI@g6F8C1)IG$hj;H~izi2XG@V6wujR#Xha;>-y%RGysj zug2X*GICfL)e4U9zCo3h9T5{UIm2;`t*O`&R9O`NEq_e288c8(EOacVmdP!I^^<<7 z5!oHy1MDbq&A~^omCfq2q#B-UN?us}3l8K%` zbgi;8Jz7x%<-R3LH?;U|O_>?blkV7195g6mQD+2c-Oyz7wrcQ?&m)!D-|5K`!*A%A z;cY^S5*g!53N&Tnh8MJn>@CsUu1Ib(y(Ky-k)u0bG`!D65E}T{Rm4O5bOm(0a+cNk|F3 zfrYnp9eR(@WrN30cpbeIWRWtAA*RZp5L!z?ofpmGvmM$rJi5Kl_Buak`F_HQ66PY7 zv*N>ZmFf>_HG<>jX+7>xNBOEM!jdkN91$dFYkT@cz$mh_KBPUey^$2=)DX~s9L0Gh z0=4qrC_(5*DNX_cA$qrF5?l9KsAZFmo-oSZMkk&oS$DDDz7a9C@xmR=yS=Ux* z#}Y0-^w*p#@Pj6_RU=6LJUpvA#3L7}3|FAP4J=F+vmpmuHJkIFwfMwBR0#|zJt&qQ z%ExD-yfIuu9?R{=PbzbpJ3)Y~c@uo?mK%S&!NK4y{i=dklGiIM%)X>J^Zh4K5A|4l zeYBCi?y*p12W1gaJN!q2xjn%0RNm<{)woDY{~^Iz>?^%p#v+)GK8%gQDgS4lk6$(n zW|!VA2)pD&4X20|`xPd_NOL1E8(T!nlWt zRy4$XD2rLrj%o0G#nCpD#P3by7}5w3tPcx|smtkF8mSPUXf|J<^Egxf!`dRniWH^? zu7k=Gkoe;qQM_xGo8;Uu0K;2O1cV|z?i8Gcq=vJVna1&rAI(*rX2_R_WbkHGC7OGY zaBqoR*ni{tM*WJD^E;)KEXAq5XLDV85yI%WRSM6l{e zq4C!nf%Tx8A_Sx;)c?-<|HJ+N|AHa@$NtacUriAP_J4v9|H=42zxeNs008)Zh862s literal 0 HcmV?d00001 diff --git a/tests/e2e/detectors/test_detectors.py b/tests/e2e/detectors/test_detectors.py index 2c6a5f55a..768ccb3f2 100644 --- a/tests/e2e/detectors/test_detectors.py +++ b/tests/e2e/detectors/test_detectors.py @@ -1714,6 +1714,11 @@ ALL_TESTS = [ "out_of_order_retryable.sol", "0.8.20", ), + Test( + all_detectors.ChronicleUncheckedPrice, + "chronicle_unchecked_price.sol", + "0.8.20", + ), # Test( # all_detectors.UnusedImport, # "ConstantContractLevelUsedInContractTest.sol", From 9c9818c24df73c20560b29dcbbfa7a311d051f8e Mon Sep 17 00:00:00 2001 From: Simone <79767264+smonicas@users.noreply.github.com> Date: Wed, 9 Oct 2024 20:19:13 +0200 Subject: [PATCH 15/18] Update tests/e2e/detectors/test_detectors.py Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- tests/e2e/detectors/test_detectors.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/e2e/detectors/test_detectors.py b/tests/e2e/detectors/test_detectors.py index 768ccb3f2..b42af234f 100644 --- a/tests/e2e/detectors/test_detectors.py +++ b/tests/e2e/detectors/test_detectors.py @@ -1718,7 +1718,7 @@ ALL_TESTS = [ all_detectors.ChronicleUncheckedPrice, "chronicle_unchecked_price.sol", "0.8.20", - ), + ), # Test( # all_detectors.UnusedImport, # "ConstantContractLevelUsedInContractTest.sol", From 1f3285568080479548d4e2d7e3b396292159818b Mon Sep 17 00:00:00 2001 From: Simone Date: Thu, 10 Oct 2024 10:45:18 +0200 Subject: [PATCH 16/18] Add StateVariable location --- slither/core/variables/state_variable.py | 14 ++++++++++++++ slither/solc_parsing/variables/state_variable.py | 15 +++++++++++++++ 2 files changed, 29 insertions(+) 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/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) From 9ce83021d7beb3b5f31c0e989759c12ca0d8d7ab Mon Sep 17 00:00:00 2001 From: Josselin Feist Date: Thu, 10 Oct 2024 11:48:21 +0200 Subject: [PATCH 17/18] Update gelato_unprotected_randomness.sol --- .../0.8.20/gelato_unprotected_randomness.sol | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 index 1a173f7d4..108859e9e 100644 --- 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 @@ -48,6 +48,7 @@ contract C is GelatoVRFConsumerBase { 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)); @@ -58,4 +59,4 @@ contract C is GelatoVRFConsumerBase { uint id = _requestRandomness(abi.encode(msg.sender)); } -} \ No newline at end of file +} From ecc20101dee3ac88fa6d5ae5d54539e4b02524f6 Mon Sep 17 00:00:00 2001 From: Josselin Feist Date: Thu, 10 Oct 2024 11:53:43 +0200 Subject: [PATCH 18/18] Update tests/e2e/detectors/test_detectors.py Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- tests/e2e/detectors/test_detectors.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/e2e/detectors/test_detectors.py b/tests/e2e/detectors/test_detectors.py index 2fa9dbbf2..d2f191a4d 100644 --- a/tests/e2e/detectors/test_detectors.py +++ b/tests/e2e/detectors/test_detectors.py @@ -1719,7 +1719,8 @@ ALL_TESTS = [ "gelato_unprotected_randomness.sol", "0.8.20", ), - Test( all_detectors.ChronicleUncheckedPrice, + Test( + all_detectors.ChronicleUncheckedPrice, "chronicle_unchecked_price.sol", "0.8.20", ),