From 08b47954cdf280aee415449594519f95efef572a Mon Sep 17 00:00:00 2001 From: Goran Vladika Date: Thu, 12 Sep 2024 12:56:57 +0200 Subject: [PATCH 01/16] Initial skeleton for unused custom errors detector --- slither/detectors/all_detectors.py | 1 + .../variables/unused_custom_errors.py | 45 ++++++++++++++++++ .../0.8.15/unused_custom_errors.sol | 11 +++++ .../unused_custom_errors.sol-0.8.15.zip | Bin 0 -> 1127 bytes tests/e2e/detectors/test_detectors.py | 5 ++ 5 files changed, 62 insertions(+) create mode 100644 slither/detectors/variables/unused_custom_errors.py create mode 100644 tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/unused_custom_errors.sol create mode 100644 tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/unused_custom_errors.sol-0.8.15.zip diff --git a/slither/detectors/all_detectors.py b/slither/detectors/all_detectors.py index 44a168c2b..3b1ecd150 100644 --- a/slither/detectors/all_detectors.py +++ b/slither/detectors/all_detectors.py @@ -19,6 +19,7 @@ from .reentrancy.reentrancy_no_gas import ReentrancyNoGas from .reentrancy.reentrancy_events import ReentrancyEvent from .variables.unused_state_variables import UnusedStateVars +from .variables.unused_custom_errors import UnusedCustomErrors from .variables.could_be_constant import CouldBeConstant from .variables.could_be_immutable import CouldBeImmutable from .statements.tx_origin import TxOrigin diff --git a/slither/detectors/variables/unused_custom_errors.py b/slither/detectors/variables/unused_custom_errors.py new file mode 100644 index 000000000..76bf0f2e1 --- /dev/null +++ b/slither/detectors/variables/unused_custom_errors.py @@ -0,0 +1,45 @@ +""" +Module detecting unused custom errors +""" +from typing import List, Dict + +from slither.core.compilation_unit import SlitherCompilationUnit +from slither.detectors.abstract_detector import ( + AbstractDetector, + DetectorClassification, + DETECTOR_INFO, +) +from slither.formatters.variables.unused_state_variables import custom_format +from slither.utils.output import Output + +class UnusedCustomErrors(AbstractDetector): + """ + Unused custom errors detector + """ + + ARGUMENT = "unused-custom-errors" + HELP = "Unused Custom Errors" + IMPACT = DetectorClassification.INFORMATIONAL + CONFIDENCE = DetectorClassification.HIGH + + WIKI = "https://github.com/crytic/slither/wiki/Detector-Documentation#unused-state-variable" + + WIKI_TITLE = "Unused state variable" + WIKI_DESCRIPTION = "Unused state variable." + WIKI_EXPLOIT_SCENARIO = "" + WIKI_RECOMMENDATION = "Remove unused state variables." + + def _detect(self) -> List[Output]: + """Detect unused state variables""" + results = [] + for c in self.compilation_unit.contracts_derived: + for custom_error in c.custom_errors: + info: DETECTOR_INFO = [custom_error.name, " is never used in ", c, "\n"] + json = self.generate_result(info) + results.append(json) + + return results + + @staticmethod + def _format(compilation_unit: SlitherCompilationUnit, result: Dict) -> None: + custom_format(compilation_unit, result) diff --git a/tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/unused_custom_errors.sol b/tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/unused_custom_errors.sol new file mode 100644 index 000000000..e337f8925 --- /dev/null +++ b/tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/unused_custom_errors.sol @@ -0,0 +1,11 @@ +//pragma solidity ^0.4.24; + +contract A { + error Unused1(); + error Unused2(); + error Unused3(); + + function x() public pure { + revert Unused1(); + } +} diff --git a/tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/unused_custom_errors.sol-0.8.15.zip b/tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/unused_custom_errors.sol-0.8.15.zip new file mode 100644 index 0000000000000000000000000000000000000000..61cc0dca55f66d3153dc0732a80e5fd0b31f1796 GIT binary patch literal 1127 zcmWIWW@fQxU}E57_?4^^>G-m0+FE7?hUdHt3_1)945fLc#i=Rr$)&|5`ML3_MMe2V z#d^j0Iq|6#1^GoK@rgyr8JT6NdRfK!d7Lb)46zIh4Gavn9UbY$e~b$kUOKnUns?68 zyiC>oa;?2VQg-s!i}#8dep+I1N#w<8JGs?wB-Lb{5;eBPu-Mnul>X{>_;^QVdj7lX zR|_^}U*Hn#v|O}X&RVoNY~EJ;rVYy;Nma@|FWc|7b*qWx4@;?dy$ z>yaJ9!H;F`##<&X>Q~as4z=f>DUvngylc&$R57*7#%8)Ff|mSwbX956`31K^HjCZO zG?kviuzaF4v-S64yM*kO=31=w$q#26K3iJYsqs7STS=7C_Pm2n&cFC>WgC)fe5PS@ zZLLXF2-_0Y;_rr;YoESco?=;B_C+%P=r-b~ ze75r9>e_JTHEnTrA5CBWKg~1o2u7g)7714C7P9s53KsWQ}lY4{w1lKhkVcO{S>s-IP_;u zsx$MOcHW?`lYjnDbiG(~>8|RZ{|g@exA0i^e&HFvYZW`CH%`7{v7+h6spm(0ZW*5B zG2QSyxsg%V(Q6`0_)Yeyl1rz~>$|qI_v8cZwG!Kz8zeP0{5i4Bb_&nUzb{i(Uow(v z{d_FskkBNXsY%ruLht^FZhd{jHbO2&&ZSc&n7V%@0DHb8~Auu#S`}zG1}7khdjhDK0RIfea-i_ z4aYXbUt#I0N|5RPy=so(Vv&VP$w7~9%v#R#+`s5t@4@dmqJhPQY+hoE`*wf-RlLJS zRqDyRTE344=l3M(e)v_Ke@tZQ3I^>J!Lcja!n(SAt{5u%zq{%*{{Snq*7C0@N&k1e zf7@_j>fw$jJAZgTkgbU8?yel|=iBWk;fn27aj7!bo`ppwmWB!>u`4GL`?a{@D zZ_1V##Qb6W(7jsStiW Date: Tue, 17 Sep 2024 12:17:26 +0200 Subject: [PATCH 02/16] Add function that checks if custom error is unused --- .../variables/unused_custom_errors.py | 34 ++++++++++++------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/slither/detectors/variables/unused_custom_errors.py b/slither/detectors/variables/unused_custom_errors.py index 76bf0f2e1..f0b61dd3e 100644 --- a/slither/detectors/variables/unused_custom_errors.py +++ b/slither/detectors/variables/unused_custom_errors.py @@ -4,12 +4,13 @@ from typing import List, Dict from slither.core.compilation_unit import SlitherCompilationUnit +from slither.core.declarations.custom_error import CustomError +from slither.core.declarations.solidity_variables import SolidityCustomRevert from slither.detectors.abstract_detector import ( AbstractDetector, DetectorClassification, DETECTOR_INFO, ) -from slither.formatters.variables.unused_state_variables import custom_format from slither.utils.output import Output class UnusedCustomErrors(AbstractDetector): @@ -22,24 +23,31 @@ class UnusedCustomErrors(AbstractDetector): IMPACT = DetectorClassification.INFORMATIONAL CONFIDENCE = DetectorClassification.HIGH - WIKI = "https://github.com/crytic/slither/wiki/Detector-Documentation#unused-state-variable" + WIKI = "https://github.com/crytic/slither/wiki/Detector-Documentation#unused-custom-error" - WIKI_TITLE = "Unused state variable" - WIKI_DESCRIPTION = "Unused state variable." + WIKI_TITLE = "Unused custom error" + WIKI_DESCRIPTION = "Unused custom error." WIKI_EXPLOIT_SCENARIO = "" - WIKI_RECOMMENDATION = "Remove unused state variables." + WIKI_RECOMMENDATION = "Remove unused custom errors." def _detect(self) -> List[Output]: - """Detect unused state variables""" + + + """Detect unused custom errors""" results = [] for c in self.compilation_unit.contracts_derived: for custom_error in c.custom_errors: - info: DETECTOR_INFO = [custom_error.name, " is never used in ", c, "\n"] - json = self.generate_result(info) - results.append(json) - + if not self._isCustomErrorUsed(custom_error): + info: DETECTOR_INFO = ["Unused custom error: ", custom_error.name, "\n"] + json = self.generate_result(info) + results.append(json) return results - @staticmethod - def _format(compilation_unit: SlitherCompilationUnit, result: Dict) -> None: - custom_format(compilation_unit, result) + def _isCustomErrorUsed(self, custom_error: CustomError) -> bool: + for c in self.compilation_unit.contracts_derived: + for f in c.functions: + for int_call in f._internal_calls: + print("Comparing: ", int_call.name, custom_error.name) + if type(int_call) is SolidityCustomRevert and custom_error.name in int_call.name: + return True + return False \ No newline at end of file From 0edd70c1a81f8fe52e4564e0f4edaa0cca585082 Mon Sep 17 00:00:00 2001 From: Goran Vladika Date: Tue, 17 Sep 2024 12:17:50 +0200 Subject: [PATCH 03/16] Add first sol contract for testing unused custom errors --- .../0.8.15/unused_custom_errors.sol | 13 +++++++++---- .../unused_custom_errors.sol-0.8.15.zip | Bin 1127 -> 2624 bytes 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/unused_custom_errors.sol b/tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/unused_custom_errors.sol index e337f8925..b8e24a971 100644 --- a/tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/unused_custom_errors.sol +++ b/tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/unused_custom_errors.sol @@ -2,10 +2,15 @@ contract A { error Unused1(); - error Unused2(); - error Unused3(); + error UsedError(address x); - function x() public pure { - revert Unused1(); + constructor() public {} + + function x() public view { + uint256 d = 7; + if (msg.sender == address(0)) { + d = 100; + revert UsedError(msg.sender); + } } } diff --git a/tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/unused_custom_errors.sol-0.8.15.zip b/tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/unused_custom_errors.sol-0.8.15.zip index 61cc0dca55f66d3153dc0732a80e5fd0b31f1796..f03b01102c4e89e91dc6633fbb2752406d997b14 100644 GIT binary patch delta 2315 zcmV+m3H0{o2*4B=P)h>@KL7#%4ggMjEm=K;b=rmr000{s=A$C$GHQ#-CkyuVNU_YRc30ykC=LDG|bYQ+vcH7y3q;o<5~? z=InuZeB)DcD(*xSe`8Ox-f4dtUui@>?3LbNW)Rl9o}OkM-C!x0C$cD+b8m) zA6-Cq{R7b(pG=pFp1&fZm^Mj_pyh5~h1%hJu!3gDI9hNPf3?0OB>O7X$a^tb?5S(n z834v)J4`3oOa@kUO{`|$_N}=4Iv_;)WycHv*=3CUk_};|=&_G=+TD<*;#@P3pBEAe zJsYVBo`Elbwu98LSVf#uLpHvtfh!s_jE*7bjh|rqYZ&iRT`&D9MW;nW;}N&T1RDfX z^Su!Rx41*af27_inwx+MNhD};xwI=|k$wO%oMr#cGcm~+EfkE@V0MARRBg*xF^HpT6f4?Vbr{{c!z|C$c!Z?~Vp@(>HX3n8^cCyLRJ9mavM-ilg}E zZ{MXa0ohCR+J8l`Y1dA2ccf-pcB;(%+2+jRNX|d@fBZJ47Ge{Cp2RU0RZ?PbZIz_N zLH)S6l<|9eDBg<@b3_5(Wc-VE zY3~+vy9MD+tOCO>&b6kO#OFw`cRqbJz7-A$Ru=YkYbbc-?m0ubVsPmbm}T0u&_Tp2 z(&Zr^gxpkbukWQ>0gccNh)-Kl#PQ*yG)_5BCl-=WahbCRwE8Cot=2>2I!dTNpElXOPIdAI}l+Mx0h_fi-djb2-n=T7!iSm zyM3}_vZ!OVuqJ^77A@&>omQa_u8q#83#Pmu+HTb3|#Jk4wyfS^ymo7-s zA2KfKcOtL_&meve*Hu;vCddKwwi#W11*62U$gzm-1T;;3RFT%n0gn3FkMX=D0-s7d z31U*Y~Rwne-BJCZ4gz>yP)b1ARo56#l#+rA&~2reiIMrARIE8 zmkB0p3AnFN70RvsFnj)GZKtD?z^YB4-tW*Q5se@ToPN9Q&D zYR*&c&s`BNFi9ZH&Te?NL#tl{x3H#*K{3F@KigVRQNldI!|N1BV;v!VUi90arikgxHmT+?~euhbc@qh-haEeI+sQ4ARgn*_HUhyLn2% zCWoED!e%+l8Cv2mYR|4Z-G(SAKT_lL3OvU*gCT$H?<^k-=Ne_khht476Ig1X;o z{M+j5h#Blamt+X%#CYl3#5ldp^d$!D&Bvl*Hza*HrbOVyu~hW!2e7Dz_{0h}nSCCJ z*h7jPqz4zvd}f52n%9cfEwjNI=CIEhy5G=}o#FQ&qcd=(XcKYqOYU8u)dE7q&!~{D z?6GODE`tzWb;dU4f3&@NrT&wVNZjIod9n&>d~2|UZMdW^gVE3xA5^LTB*dH?D^d&p zi9^sydHge=<|_)&#@B^Q`STV#QIMdCbju!9FT?Q?#+cepozfuBmfKYv2=6fO?Bc>oNL^FWfLU_Cj+t?r6AYn^m_#IC{oSf`1IC zj3F|~t^H@v{(v*swN)WqZ&bYvF;6N{x)7yS3E_*XI(N5dZ}zuGg0i^=1XJ!_KFnr7<4+K1@3$Vb4Tg4XD|FYF5tWZk< l0zU&k00ICG08V=?Sv`bx+J*@L02?lo5d=^M& delta 812 zcmV+{1JnG#6z2#SP)h>@KL7#%4gmUNELlP6dX}vN008F=u^0sbf5GI>==VB3RBbcN zfU|vlG8E5leRlL2Z^gC?e>sh{2mJg%Q_Sx8FIgLj}KfuCL(Z$Uuz?y zdwo{}tcG4b@ipoH%M6jp)b%s>$d)(sk>zU*s>QHGDnNYtFMo)8cVjciru*A)r_Zhv zy8U`%rgUAIBJ*6T%X^8<7=t5oz^eDT71wGn(i+*qP0hXXe^RwGRrzdXLj&xG4N~=! z`S>A4(Q?w=CH()O;r}>DuJ57DPS$w28?lqrIH-d7%ICyP+A+xtHL&MnfdMT+N|6Lt z*$0&wrIntJ*13(z;47^dw*!C~D6sseDY$$uA4s+=*S6rmzx zQsUT}rwr#$a?Xvx_iPnWb8!Yr7Nd^4_xf|VJS7_Bf9`z_@i5Q4VlD9cb8p5JrKkWa zs8e02hE|G-Ow=(UPwv%1pTGqJDyQ{iV*j}B?SRmg!-(X$QX9s|kv9<64B>*!)8K^u zSp;C}gS>7S%Pe7cq1jaQDAr@8v^dmaT7PILbM}Ls<<$Y^b|lM%g4w`>MY7Ku5c zM01(Ze^o8=BZwzxTSSw@BYJ-a{D8T0C`5$XuuFk~U)65a@}aB~aL&2i5ucj2KNI10001$)Q{}| From 82dfd33ec0590bda7a543e6997e32f611454d2c1 Mon Sep 17 00:00:00 2001 From: Goran Vladika Date: Tue, 17 Sep 2024 15:13:27 +0200 Subject: [PATCH 04/16] Refactor and include errors defined outside of contracts --- .../variables/unused_custom_errors.py | 50 ++++++++++++------- 1 file changed, 32 insertions(+), 18 deletions(-) diff --git a/slither/detectors/variables/unused_custom_errors.py b/slither/detectors/variables/unused_custom_errors.py index f0b61dd3e..d6dfaf41d 100644 --- a/slither/detectors/variables/unused_custom_errors.py +++ b/slither/detectors/variables/unused_custom_errors.py @@ -1,7 +1,7 @@ """ Module detecting unused custom errors """ -from typing import List, Dict +from typing import List, Set from slither.core.compilation_unit import SlitherCompilationUnit from slither.core.declarations.custom_error import CustomError @@ -13,6 +13,7 @@ ) from slither.utils.output import Output + class UnusedCustomErrors(AbstractDetector): """ Unused custom errors detector @@ -31,23 +32,36 @@ class UnusedCustomErrors(AbstractDetector): WIKI_RECOMMENDATION = "Remove unused custom errors." def _detect(self) -> List[Output]: + """Detect unused custom errors""" + defined_custom_errors: Set[CustomError] = set() + custom_reverts: Set[SolidityCustomRevert] = set() + unused_custom_errors: Set[CustomError] = set() + # Collect all custom errors defined in the contracts + for contract in self.compilation_unit.contracts: + for custom_error in contract.custom_errors: + defined_custom_errors.add(custom_error) + + # Add custom errors defined outside of contracts + for custom_error in self.compilation_unit.custom_errors: + defined_custom_errors.add(custom_error) + + # Collect all used custom errors + for contract in self.compilation_unit.contracts: + for function in contract.functions_and_modifiers: + for internal_call in function.internal_calls: + if isinstance(internal_call, SolidityCustomRevert): + custom_reverts.add(internal_call) + + # Find unused custom errors + for defined_error in defined_custom_errors: + if not any(defined_error.name in custom_revert.name for custom_revert in custom_reverts): + unused_custom_errors.add(defined_error) - """Detect unused custom errors""" results = [] - for c in self.compilation_unit.contracts_derived: - for custom_error in c.custom_errors: - if not self._isCustomErrorUsed(custom_error): - info: DETECTOR_INFO = ["Unused custom error: ", custom_error.name, "\n"] - json = self.generate_result(info) - results.append(json) - return results - - def _isCustomErrorUsed(self, custom_error: CustomError) -> bool: - for c in self.compilation_unit.contracts_derived: - for f in c.functions: - for int_call in f._internal_calls: - print("Comparing: ", int_call.name, custom_error.name) - if type(int_call) is SolidityCustomRevert and custom_error.name in int_call.name: - return True - return False \ No newline at end of file + for custom_error in unused_custom_errors: + info: DETECTOR_INFO = ["Unused custom error: # ", custom_error.full_name, "\n"] + json = self.generate_result(info) + results.append(json) + + return results \ No newline at end of file From a6b84cfc8a7fc157133128da76475029e915b112 Mon Sep 17 00:00:00 2001 From: Goran Vladika Date: Wed, 18 Sep 2024 14:29:09 +0200 Subject: [PATCH 05/16] Change format --- slither/detectors/variables/unused_custom_errors.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/slither/detectors/variables/unused_custom_errors.py b/slither/detectors/variables/unused_custom_errors.py index d6dfaf41d..3db6c021f 100644 --- a/slither/detectors/variables/unused_custom_errors.py +++ b/slither/detectors/variables/unused_custom_errors.py @@ -46,7 +46,7 @@ def _detect(self) -> List[Output]: for custom_error in self.compilation_unit.custom_errors: defined_custom_errors.add(custom_error) - # Collect all used custom errors + # Collect all custom errors used in revertsCustomError for contract in self.compilation_unit.contracts: for function in contract.functions_and_modifiers: for internal_call in function.internal_calls: @@ -59,9 +59,10 @@ def _detect(self) -> List[Output]: unused_custom_errors.add(defined_error) results = [] - for custom_error in unused_custom_errors: - info: DETECTOR_INFO = ["Unused custom error: # ", custom_error.full_name, "\n"] - json = self.generate_result(info) - results.append(json) + if len(unused_custom_errors) > 0: + info: DETECTOR_INFO = ["The following unused error(s) should be removed:"] + for custom_error in unused_custom_errors: + info += ["\n\t-", custom_error.full_name, " ", custom_error.compilation_unit, "\n"] + results.append(self.generate_result(info)) return results \ No newline at end of file From 0efbf5ccc08a5379193b4830e44cc0cb1aa40aaf Mon Sep 17 00:00:00 2001 From: Goran Vladika Date: Tue, 24 Sep 2024 11:54:22 +0200 Subject: [PATCH 06/16] Add filename to the output --- .../variables/unused_custom_errors.py | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/slither/detectors/variables/unused_custom_errors.py b/slither/detectors/variables/unused_custom_errors.py index 3db6c021f..913374438 100644 --- a/slither/detectors/variables/unused_custom_errors.py +++ b/slither/detectors/variables/unused_custom_errors.py @@ -1,7 +1,7 @@ """ Module detecting unused custom errors """ -from typing import List, Set +from typing import List, Tuple from slither.core.compilation_unit import SlitherCompilationUnit from slither.core.declarations.custom_error import CustomError @@ -33,36 +33,37 @@ class UnusedCustomErrors(AbstractDetector): def _detect(self) -> List[Output]: """Detect unused custom errors""" - defined_custom_errors: Set[CustomError] = set() - custom_reverts: Set[SolidityCustomRevert] = set() - unused_custom_errors: Set[CustomError] = set() + defined_custom_errors: List[Tuple[CustomError,str]] = [] + custom_reverts: List[SolidityCustomRevert] = [] + unused_custom_errors: List[Tuple[CustomError,str]] = [] # Collect all custom errors defined in the contracts for contract in self.compilation_unit.contracts: + contract.custom_errors_declared for custom_error in contract.custom_errors: - defined_custom_errors.add(custom_error) + defined_custom_errors.append((custom_error, custom_error.contract.file_scope.filename.short)) # Add custom errors defined outside of contracts for custom_error in self.compilation_unit.custom_errors: - defined_custom_errors.add(custom_error) + defined_custom_errors.append((custom_error, custom_error.file_scope.filename.short)) # Collect all custom errors used in revertsCustomError for contract in self.compilation_unit.contracts: for function in contract.functions_and_modifiers: for internal_call in function.internal_calls: if isinstance(internal_call, SolidityCustomRevert): - custom_reverts.add(internal_call) + custom_reverts.append(internal_call) # Find unused custom errors - for defined_error in defined_custom_errors: + for defined_error, file_name in defined_custom_errors: if not any(defined_error.name in custom_revert.name for custom_revert in custom_reverts): - unused_custom_errors.add(defined_error) + unused_custom_errors.append((defined_error, file_name)) results = [] if len(unused_custom_errors) > 0: info: DETECTOR_INFO = ["The following unused error(s) should be removed:"] - for custom_error in unused_custom_errors: - info += ["\n\t-", custom_error.full_name, " ", custom_error.compilation_unit, "\n"] + for custom_error, file_name in unused_custom_errors: + info += ["\n\t-", custom_error.full_name, " (", file_name, ")\n"] results.append(self.generate_result(info)) return results \ No newline at end of file From 53e896ce3bd01a75bd2d700c47c9b1332281bc82 Mon Sep 17 00:00:00 2001 From: Goran Vladika Date: Tue, 24 Sep 2024 12:01:15 +0200 Subject: [PATCH 07/16] Simplify fetching the file name --- .../variables/unused_custom_errors.py | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/slither/detectors/variables/unused_custom_errors.py b/slither/detectors/variables/unused_custom_errors.py index 913374438..f4262ac72 100644 --- a/slither/detectors/variables/unused_custom_errors.py +++ b/slither/detectors/variables/unused_custom_errors.py @@ -1,10 +1,9 @@ """ Module detecting unused custom errors """ -from typing import List, Tuple - -from slither.core.compilation_unit import SlitherCompilationUnit +from typing import List from slither.core.declarations.custom_error import CustomError +from slither.core.declarations.custom_error_top_level import CustomErrorTopLevel from slither.core.declarations.solidity_variables import SolidityCustomRevert from slither.detectors.abstract_detector import ( AbstractDetector, @@ -33,19 +32,19 @@ class UnusedCustomErrors(AbstractDetector): def _detect(self) -> List[Output]: """Detect unused custom errors""" - defined_custom_errors: List[Tuple[CustomError,str]] = [] + declared_custom_errors: List[CustomError] = [] custom_reverts: List[SolidityCustomRevert] = [] - unused_custom_errors: List[Tuple[CustomError,str]] = [] + unused_custom_errors: List[CustomError] = [] # Collect all custom errors defined in the contracts for contract in self.compilation_unit.contracts: contract.custom_errors_declared for custom_error in contract.custom_errors: - defined_custom_errors.append((custom_error, custom_error.contract.file_scope.filename.short)) + declared_custom_errors.append(custom_error) # Add custom errors defined outside of contracts for custom_error in self.compilation_unit.custom_errors: - defined_custom_errors.append((custom_error, custom_error.file_scope.filename.short)) + declared_custom_errors.append(custom_error) # Collect all custom errors used in revertsCustomError for contract in self.compilation_unit.contracts: @@ -55,15 +54,16 @@ def _detect(self) -> List[Output]: custom_reverts.append(internal_call) # Find unused custom errors - for defined_error, file_name in defined_custom_errors: + for defined_error in declared_custom_errors: if not any(defined_error.name in custom_revert.name for custom_revert in custom_reverts): - unused_custom_errors.append((defined_error, file_name)) + unused_custom_errors.append(defined_error) results = [] if len(unused_custom_errors) > 0: info: DETECTOR_INFO = ["The following unused error(s) should be removed:"] - for custom_error, file_name in unused_custom_errors: - info += ["\n\t-", custom_error.full_name, " (", file_name, ")\n"] + for custom_error in unused_custom_errors: + file_scope = custom_error.file_scope if isinstance(custom_error, CustomErrorTopLevel) else custom_error.contract.file_scope + info += ["\n\t-", custom_error.full_name, " (", file_scope.filename.short, ")\n"] results.append(self.generate_result(info)) return results \ No newline at end of file From acc542e3eee61b1907ddd004ffac190fdebc3f14 Mon Sep 17 00:00:00 2001 From: Goran Vladika Date: Tue, 24 Sep 2024 12:32:57 +0200 Subject: [PATCH 08/16] Add wiki docs --- .../variables/unused_custom_errors.py | 28 +++++++++++++++---- 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/slither/detectors/variables/unused_custom_errors.py b/slither/detectors/variables/unused_custom_errors.py index f4262ac72..6f9545544 100644 --- a/slither/detectors/variables/unused_custom_errors.py +++ b/slither/detectors/variables/unused_custom_errors.py @@ -19,16 +19,32 @@ class UnusedCustomErrors(AbstractDetector): """ ARGUMENT = "unused-custom-errors" - HELP = "Unused Custom Errors" + HELP = "Detects unused custom errors" IMPACT = DetectorClassification.INFORMATIONAL CONFIDENCE = DetectorClassification.HIGH - WIKI = "https://github.com/crytic/slither/wiki/Detector-Documentation#unused-custom-error" + WIKI = "https://github.com/crytic/slither/wiki/Detector-Documentation#unused-custom-errors" - WIKI_TITLE = "Unused custom error" - WIKI_DESCRIPTION = "Unused custom error." - WIKI_EXPLOIT_SCENARIO = "" - WIKI_RECOMMENDATION = "Remove unused custom errors." + WIKI_TITLE = "Unused Custom Errors" + WIKI_DESCRIPTION = "Declaring a custom error, but never using it might indicate a mistake." + + # region wiki_exploit_scenario + WIKI_EXPLOIT_SCENARIO = """ + ```solidity + contract A { + error ZeroAddressNotAllowed(); + + address public owner; + + constructor(address _owner) { + owner = _owner; + } + } + ``` + Custom error `ZeroAddressNotAllowed` is declared but never used. It shall be either invoked where needed, or removed if there's no need for it. + """ + # endregion wiki_exploit_scenario = "" + WIKI_RECOMMENDATION = "Remove the unused custom errors." def _detect(self) -> List[Output]: """Detect unused custom errors""" From cacb154802d049a527f237dc0500ad8ddd0fb16f Mon Sep 17 00:00:00 2001 From: Goran Vladika Date: Tue, 24 Sep 2024 14:06:47 +0200 Subject: [PATCH 09/16] Make test contract(s) more complete --- ...5_ContractToTestForCustomErrors_sol__0.txt | 15 +++++++ .../0.8.15/ContractToTestForCustomErrors.sol | 41 ++++++++++++++++++ ...ntractToTestForCustomErrors.sol-0.8.15.zip | Bin 0 -> 5968 bytes .../0.8.15/CustomErrorsLib.sol | 8 ++++ .../0.8.15/ErrorLibConsumer.sol | 17 ++++++++ .../0.8.15/unused_custom_errors.sol | 16 ------- .../unused_custom_errors.sol-0.8.15.zip | Bin 2624 -> 0 bytes tests/e2e/detectors/test_detectors.py | 2 +- 8 files changed, 82 insertions(+), 17 deletions(-) create mode 100644 tests/e2e/detectors/snapshots/detectors__detector_UnusedCustomErrors_0_8_15_ContractToTestForCustomErrors_sol__0.txt create mode 100644 tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/ContractToTestForCustomErrors.sol create mode 100644 tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/ContractToTestForCustomErrors.sol-0.8.15.zip create mode 100644 tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/CustomErrorsLib.sol create mode 100644 tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/ErrorLibConsumer.sol delete mode 100644 tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/unused_custom_errors.sol delete mode 100644 tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/unused_custom_errors.sol-0.8.15.zip diff --git a/tests/e2e/detectors/snapshots/detectors__detector_UnusedCustomErrors_0_8_15_ContractToTestForCustomErrors_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_UnusedCustomErrors_0_8_15_ContractToTestForCustomErrors_sol__0.txt new file mode 100644 index 000000000..c62cbd5ec --- /dev/null +++ b/tests/e2e/detectors/snapshots/detectors__detector_UnusedCustomErrors_0_8_15_ContractToTestForCustomErrors_sol__0.txt @@ -0,0 +1,15 @@ +The following unused error(s) should be removed: + -UnusedParentErr() (tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/ContractToTestForCustomErrors.sol) + + -UnusedParentErrWithArg(uint256) (tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/ContractToTestForCustomErrors.sol) + + -UnusedParentErr() (tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/ContractToTestForCustomErrors.sol) + + -UnusedParentErrWithArg(uint256) (tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/ContractToTestForCustomErrors.sol) + + -UnusedLibError() (tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/CustomErrorsLib.sol) + + -UnusedErrorLibConsumerErr() (tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/ErrorLibConsumer.sol) + + -UnusedTopLevelErr() (tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/ContractToTestForCustomErrors.sol) + diff --git a/tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/ContractToTestForCustomErrors.sol b/tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/ContractToTestForCustomErrors.sol new file mode 100644 index 000000000..c59f3505c --- /dev/null +++ b/tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/ContractToTestForCustomErrors.sol @@ -0,0 +1,41 @@ +// SPDX-License-Identifier: Unlicense +pragma solidity 0.8.15; + +error UnusedTopLevelErr(); +error UsedTopLevelErr(); + +import "./ErrorLibConsumer.sol"; + +contract ParentContract { + error UnusedParentErr(); + error UnusedParentErrWithArg(uint256 x); + error UsedParentErr(address x); + error UsedParentErrInChild(uint256 x); + + constructor() { + new ErrorLibConsumer(); + } + + function x() public view { + uint256 d = 7; + if (msg.sender == address(0)) { + d = 100; + revert UsedParentErr(msg.sender); + } + } +} + +contract ContractToTestForCustomErrors is ParentContract { + function y() public { + address f = msg.sender; + (bool s,) = f.call(""); + if (!s) { + revert UsedParentErrInChild(1); + } + revert UsedTopLevelErr(); + } + + function z() public { + revert Lib.UsedLibErrorB(8); + } +} diff --git a/tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/ContractToTestForCustomErrors.sol-0.8.15.zip b/tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/ContractToTestForCustomErrors.sol-0.8.15.zip new file mode 100644 index 0000000000000000000000000000000000000000..9efb0d8ebe806855ed34bfcefe63953e245a9094 GIT binary patch literal 5968 zcmbW*)k6~iqXqC0(#;6zZX`yHZjg}z(%m70(IF|_ozg9hbgPJr(MU*#G)Rq(`+fKQ zzT9&j&eQn^e%cT;bQu6DfB>NBE@mV~v!(lq8~`A#0|0~p0DuD2%?oB_*y>a%0x?!Wg0+;~+p#XqXOpF8Q z5#&)aGtn-AUohrJ-RSVMNRaqTU9z}1C1ZgF=vLmJ1e1gJRWofr@)vc(D@q5q0x5cP<6ua z5$CgFbu4jmF#EQGfSv6KjC0JE!<4Gs#kFs)C&0e*t;-PFlXPFmpVNZ8IOZ}MDQeNs zzOhiE#X+x{DxX+@!rjRPuT7WU>nviBnsfFNT?;13D z-C6DD6Nm%s^+vGt-z@pmqf(hARjyj4Xw85dUWb+$4b_ExxB>}Ge^NAfq^6tI&SEkmud99A^uCYbK_Aw@jTS;2$;5Dw>$H%JgNZs zh`IBpBci~M<5`~gb~7Hr(HAGEz_?+Rwmuxg0@`!kGOuCns^#%ghKLIA^^JAN$DV#o zkbza7uX67UClkM?&Mnz46E_zLbLaBuzQMMtGyw2Y2~$K_Y4??s~XltQXx zX1w_+88exctR!ccA_w^+QgLdMBaSl~-*X+@IAeEfH=h~QyZP3hFNeUewW!z`?V|AyICebbb&01pk z6b}retp@OTf`UqEVinodMnW5$urAHgD#(Q`K`_XDMu4V1f!>%0;(0E6yP-5XlPo&> z(62%E3#8d*+tN#aBb}wUr55I;U!g_o@KRs@?og6^s#5#1|HI=i&N_*WOt)Zs51MA#iRPS;8C>+bMDvh7t^b& zZ1_q3yoXymz5>W-BYXWK{Wu`t8~Ss>xtU<2c)`mEbzQdGOk9i8;32q@63!bM+&$ds z{df7x)a;F~erMoN$_xgP?A4J#iT6#k3_>Hs?7L};=ZmBAPY^vou14?ZqT8DKom7$9 zefI6|HpOtz^fTz-V@kCYKbYciA-YBRGG;{_VJJpxpa>E_KMoTcD5?pGG@l zh6%+>F+)Z@uj5j7GO5-N&l&_nrd4I+?+KUr)S?G=c><=fMBjw2?nMvB9)qAQSF9j{ zn8X6`HMxz6c)MUPs4q}_=7K+2`+h%@YnVXS!Sy7Z27%lSLPGI~hs=B|fDyY4q zdir9_h^Mm;)aGdvQ5O>S;29nTb-{(@MJsnChIU`4k6(kCV zybeRsb;wqWGRTIBo4n(ZWDw(mOw{T?D}r+raWWY~g>d5w14t`cknBXm8jFGjeA+1_hEfP@FL@YF5^uC8Mz%&t5RcXo8l?>d+)jOOwK3ht+aMPsl;)j=!;@_X*0p!0B`bSdPI)! ztmRx5e*AS$oq-^1pky6;e%(&n3Ivin-l1Zzu?dUkAIi~zxOywme@Cgjb4Pb|JoYNT z1(OCQt2IMO|2P}QRB($qPhq3*wym>j;$quG?(;%-7vG2(uC#dSC<$4`QP!YY@n#1xkgdZ}wStKF- z?iet=g&94hX*gm9IEtpsK$6k?Iz{cwWS>++(t&G3aMw3#<^%%sdf$WWMP!KBSiZX( z&QK8wfE2#%?zai*nAq#=P83P&)>wmzzK=zKeFqJA=X`4iKagGr6)Nd&p@=0E|X4Ex%{lr#%qXV7j8M z_wbpT>+lqQ4Np-gs}N2LKc(5g81MO|tZPFhe0h3=&tUo#)hb0Yp+BH;NG9b&k|0PO zJG*|+ha~Pd{%C#xE-cnrBrBX9H;{?T4?d+CzMQ;0og!vsv+CxVUgUU3dDnjVJFJ6Hdb#0e)yx}z-Bgr~81eWi8_AP&0685kNnhFL+|kxmgw)$QJp0FG$!aM1!EyP- zbdL7J<@hOH!lGpk{gpD}Tj&iR0rRM>Qk7V6@M<@bm7b=&h3SpgNlopmt-^NBGS%3l z1_C~@kQPnT!4t=@(8gmO)U`*Sh$P77(5Jk9-qq>1U2pkd(R45Jc7@HIzY@=;MN8Ab zve>A^;=XZMiZ{_dCoa>-Yq~;aUB-+ zVB1ChVQoCC#14S!gQj`1Sg{zbc;I=^ssHid9zOQ=1gZDz*|+|Bk*l8VUgwg^hEFc* zNLklg7T!tvmnz9%{hCdn2;hC(5mI4f71TzSuC)GP{pKcReIrch3d#13UEHrfNFrHA zaJ>kcB`Ajd0}q~7)3irHgtz-FPv#WJ*O=EPK)S*%tYf6F)hwI%=#;$H`i#NvHZl2Qd`e2}XH6?9ls}L9uMLUwFfQ&hd7Vrd%PZ z$ffv7Ea<&$S{%BGw=}Lf=RpfDmIs$AS!`cY%F$YYv|pFK-?!ec(NYxWy3yq6Xo)?J z$zU^7LeoM4F=^%{bA{=zLd&BJFRSJzJA>KAE1ReL-RsXj)A1@*;1@B5Tj z(nHRV@72`kAL;cACXCz)Ef#HyxYRDR@z6N(rMX?prYwjQc2mnsUA z{Ha7ycsW~ok49OO7;Ns!8wP!;OvU?pEY_?Y5C3{DyNc0}SMvzS7?>KOYAWt448TdV zxkfW&SJ+l9VJd%ClB~zSQ|d>%vzcYi3yF%37CXi>1iRXtqg+*irXOEdxb0gBG+|(2 z_I?gEX4MPr$WkYvbPiwFQ0olmH70;Hw8CoRMB7jhl_XU?hcg( z9rZxS6V3_S_R077?dr}O$!snV(OpqiKJ3&y3(JaIge{OKlIz7x@Mnu=*Y2RA#~ zUL`TTpg4*x)$Z5G+zJOluOvyO1%&HLC){IpHLBfxX_sZC1oU1>sxt=|UX*tQD{6Z$ zz$iXl=D=i`#DXcGJ=~x+#PPml$(e>#bx+9-235oH!CQZ$T!jlpAz^wev%S+Lv$Vrd z?4L$O_I9XfKF`?97$K26?Pw*2EKJ?6{W2?Nk4!9(?A%8N!AW)W*$FSjy5sgQVFU-8 zQq5;&p#m9d>PeQ_r@pmJDJ551NO_eA`uha#+O;`?*f9;svWc8l#-Af(gu3SVXEgdn zWZrmlN67C-5`uT1EX3bGP{~alNs}~X>0z-F_)v-M?sfqsIKCDwqLGzq-+c6M4lftl zAy6mZd#T_H!-t5Pryn|JS%8WE88r+yrHXM`pOm$C(@=`o5Do1g2*pzU(~_haAs|kx zw&LP(ncmUUvCjOtG8X#4U525Ni(8lzep*!1oFuDHF#gXz4vDRpK{|=x6oS5f$6C@D z+rK^NK(OFh=CN{JW%}K8jWG1X=f<{Js2wIyD7(2Fbw|>2Kuw`@hioWVT&up#i+Dy= z!F3yP%eG_?qe0;W_HEI()^M`yK+&Bp7`Epb%|{tLDi%1l=Q;qVP_Ua-NY^~{>qHiC zy@szu%mMXMzWo$FRP!SQ8QE-1Y$^_GT}S0vzcDe~M0hM8V~y=-D{cgO zPVPlUYHPvUy^4unN(_TRU*YTyJU49f)l+JeXW=_>^pLHYGepn$FpopapBu7Eex%dB+leG``dl80Z< zho9p23s0=@6e(6L5u&5BIcB;s^OT;{V_gl67;dGQJ`JsKgrH-Hht!*X@N)fT4#M3k zz8@x66S4h7_R<#L?c3a{cFM@1leqRu&RdD!M#aOZxxVk8MKs0y!mMS97At3X{-kvq zv-I5$a+5KkLeU90Nk$U!4%QPfF~)Pu~5Emx){q%IzVTGLtXJ_yP$iUW2-(lZ$P zO%V@_^mvsIm`x`@cbi2uwXZ~U`9d^Q4kAF@+?KJ(#l&iW6!Jyx(PqAZ@*+c>ev@}J zRo?n1H5gwQUM6&poDK{$8;d)6dN%N?NKB+zW(|dXq_py}X$W%IVh^(bI4^4;GGKozlO=Qa z64emiS5EHYq*@N@U$r+JWq3$VCPblz|SidPL(SqOCYnS z?kNq7+Tv#2U-;h#0_>4DP5HEXqdY$O?thlJP5TU^`##Q(ehw}@4IvS z&DgF_36?GWohB#3TEc}7ELz3((SVn99!kjCwX&|~k2|caK^FqF{CI0yj&VA--laqq zHD2|W41Jv~1(VSl`87go4Y`G=dDiI(%_6ICgsZ)u8ehIF`-tC$7e!!xw^kfPL?=pN zGM!k5(QoUgRc;38jkRo3b(=iOX{N74wrhXXKA3l3O8T*H0&rg`# zDxc!b`>{&<7To*p+FT7;GMWYo`1LznmaSH8NT0E-nQBPwxxNrFSd;j@$IjaCW0cIK zf4!^Eqk7`ht8J5`8(>8+;w0FLt;*(<#-l1bGN@~|Pk3P&&L~o+r8Uf$%g}Sq)pbJ< zUAwI(u^+q935_fYe9xj^bW10IW{U3VFPC)@rSN@kl*+~UFLV-UGEgGq;8@z)z!ioe z9`Ox76%x4Q{b&%+R=G|iaV?HQMLw-BhDr9U5J54yuOk!@1huXJV*GljHl+I4M9PbLHoa9`#-k*zX?V8@BF`*TO0BU TmM1pojS zh{GbiJP!I84`jC!9uVYvE2ru`+B%iN|DM*-jDmC0-HzGvg z_R=nHPM^-ktdEZc)uQO!0{8VwTO{>GNUlZoMzIv~{HX|urLc}m_4QhMyLvjFH?NIs z7W^IwwIp8In}-2!H^VG6>oQ8yWFaUQBT%lef6kD3)BzH#FsiAe*5g=yFh3^yaHwR; z`8D{npSE}b@H9i5O@9XqyM?OKRpBr~%;iIw6~XtF_5-&=-DDdBJHuGm)Oz`HR!{e( zx9jQJ(;U<;iDZsSK99Q+W~F&tlgTXe|3cXYe$me0^C&vlxzz`Hw6ku;@~dzbQxU=^Xv zwHSQVSEmq_bu(eamDe+VcTahs!{pjCX+{{U=o>f@Q}}wyj+upoBEtip1+})UVKuJ+fclU zt0+dD*g*P7xn^GAWUA9%6{C7eGbL4K`9blmeU#{iBV7GZ+5D-w=~scqE_QKtr(bOX zY>iE(UF9>%dHF>hgH+8CrjA#H_fIJ%1gnkfJ6HfWK&@tN@*+JCRX5^y&^ymag zOQ^R^Od(5)+n0pC7jbzX1|6m=)}C+KNae(i>V}eXKCn5ognhMqOiAiZD%)jZW-HWL zCT+H+?wV8l#_dAF%e~#hp7kG2GW+K(2aUE5bjaY_Lc}5(6zt-7J18KloMwF3)Regy z8L2drG{@V$yPPDupT4xk6B2Ev+5;@i*yJfp86YZuQVM4?I;GhC(EF?&%H9W_O%weK zbeUANr%&BCDD^?mF4Y_rKc|hm3pJXxdZeR+PnLz{spo$GV^EabcA?G3g<^PuR-!#I zQ@~*c^^$A6>Gg`0)AT^@K=x~R=8Eu?wz3P$h`y?--Jym8){JG=>mWJIZv!0K!ZBy} zf|UvxtS4vFmMkgZFoJfw$O$f2@HP&mgkN0OGktSAXhkRoIaNnDrm2oB%1FgcJ7cOh zD%@Ef(D@RtxI54`=gPF~t{j${+$MzcwmCpMbuUz>M05_ z>|nJOY@zED(5nZ7PkG3#8tzG2m${`jz`wF0Tj3QPqwIP0*oM`SdD(js&52Ms+km65 zk&aha(^9@W@MeGoqrDnFvxbE(u4g3XZVMXQo)TQypD!WjrcJvHHwMjBxBMdA*nXPw z#tnAaEyA^Cb+-nOQmHs|C#zFYShH_0X~Q7=o%>^r5Q71xas@ z05|n>BUCVnjjshKfAZIAU`IOEB6Md=-XEdcY@JGqQlcqkLo7$6&OuhfWhop(($S!C z7grvI9+s^JF?cjb8I96Yleo^Vb(!Ft@#zCg>fU79W~&@)v9&(eZD-Dp%|t${1i1=( z@z89tajZV`I4@d8gKVIK8|&$EgX~}rM-|kCjT%&Jc*Z|!w-Ts(way(TFJ1>Lxn52? z{m|;7zky{NFWwxEb~%34XW=b4PUyteyv&p>`bz|CG>CClci8-dwe?w%g5ZcrOpwWr zOpZpGP*nC4Wk^J&do4e-#xRK#xO6EkljrygQI3B7Il*GCP7MB5A`JC;h@c5{d=}S# z8;hPDX$9Fr1NdDAT1viwq@@~~yJ(UrGU=C@+Ov$+(|Tus&XBE2sUH6s5Y z1D=*W>U?&&WM+ddFOc5m0BQ8KDznQE=irdhv6K$!Ij?8~`Ue3EFCKVc{8e?b-I*%D zBt~*445FFF%qNMBF#M||0W^<*NH0)1%%DN6Qp7Blv=99Uoz{oo0uT)=>Hfh$e1qw8 z54J`MIoTN6Ni$-&**m;5cuQRswm?Y85N>WUipj~#N13wfAp@1j*H*jvN&IN%g5HjM z;s0dfsJRb=iBVU75ZoUPQ-6r6lR6|`>qA&#@0*&e;}_E$YyWh1vDT=p36>TvR*>Qd zT?=B*+1)PK(23lqjF7?0;>;Or?C@wtQ&tW^X@L-CT>HtwRPb2N#q8-B8>q1Z7*@L|-2sf;{vG?+E?uWY+GxmI&*&3JHa8-+Alxi@D9KFAROUaAdRb zaX8xW!cIaBv-Xa2(v8g4{4F2fgPV$x&o6bk#Wv6SA27bHCW@_H%=S9IM8mHzXy@Ao zw&Xe22Huz6)|DfiC!2lgs&U@>evDF?uuJ3C$t6f8PM9W1lpj0U7v&w7mp7jDN$vI9 zQ%uT`So19XVd314beU!l*;god(`FrBGRA5`8>&oywN~R-4I%O0#C7X1>)P&A{CSjN zIX!D9Ef+4 ztyiIY$?DS@y4xmL?RDQKpL~}_wg{C)r^ z@X<6wbS6(e2BE6dcpbLG0~#&52h#B5-8P$V@M=LUp(tw2 Date: Tue, 24 Sep 2024 14:17:19 +0200 Subject: [PATCH 10/16] Lint --- .../detectors/variables/unused_custom_errors.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/slither/detectors/variables/unused_custom_errors.py b/slither/detectors/variables/unused_custom_errors.py index 6f9545544..cde22cc00 100644 --- a/slither/detectors/variables/unused_custom_errors.py +++ b/slither/detectors/variables/unused_custom_errors.py @@ -27,7 +27,7 @@ class UnusedCustomErrors(AbstractDetector): WIKI_TITLE = "Unused Custom Errors" WIKI_DESCRIPTION = "Declaring a custom error, but never using it might indicate a mistake." - + # region wiki_exploit_scenario WIKI_EXPLOIT_SCENARIO = """ ```solidity @@ -71,15 +71,21 @@ def _detect(self) -> List[Output]: # Find unused custom errors for defined_error in declared_custom_errors: - if not any(defined_error.name in custom_revert.name for custom_revert in custom_reverts): + if not any( + defined_error.name in custom_revert.name for custom_revert in custom_reverts + ): unused_custom_errors.append(defined_error) results = [] if len(unused_custom_errors) > 0: info: DETECTOR_INFO = ["The following unused error(s) should be removed:"] for custom_error in unused_custom_errors: - file_scope = custom_error.file_scope if isinstance(custom_error, CustomErrorTopLevel) else custom_error.contract.file_scope + file_scope = ( + custom_error.file_scope + if isinstance(custom_error, CustomErrorTopLevel) + else custom_error.contract.file_scope + ) info += ["\n\t-", custom_error.full_name, " (", file_scope.filename.short, ")\n"] results.append(self.generate_result(info)) - return results \ No newline at end of file + return results From 3d68770e9f7ae4e0a355a593404c268d8d8f10a1 Mon Sep 17 00:00:00 2001 From: Goran Vladika Date: Tue, 24 Sep 2024 14:22:07 +0200 Subject: [PATCH 11/16] Move detector to another folder --- slither/detectors/all_detectors.py | 2 +- .../detectors/{variables => statements}/unused_custom_errors.py | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename slither/detectors/{variables => statements}/unused_custom_errors.py (100%) diff --git a/slither/detectors/all_detectors.py b/slither/detectors/all_detectors.py index 3b1ecd150..9bfdc5cd4 100644 --- a/slither/detectors/all_detectors.py +++ b/slither/detectors/all_detectors.py @@ -19,7 +19,6 @@ from .reentrancy.reentrancy_no_gas import ReentrancyNoGas from .reentrancy.reentrancy_events import ReentrancyEvent from .variables.unused_state_variables import UnusedStateVars -from .variables.unused_custom_errors import UnusedCustomErrors from .variables.could_be_constant import CouldBeConstant from .variables.could_be_immutable import CouldBeImmutable from .statements.tx_origin import TxOrigin @@ -98,5 +97,6 @@ from .statements.tautological_compare import TautologicalCompare from .statements.return_bomb import ReturnBomb from .functions.out_of_order_retryable import OutOfOrderRetryable +from .statements.unused_custom_errors import UnusedCustomErrors # from .statements.unused_import import UnusedImport diff --git a/slither/detectors/variables/unused_custom_errors.py b/slither/detectors/statements/unused_custom_errors.py similarity index 100% rename from slither/detectors/variables/unused_custom_errors.py rename to slither/detectors/statements/unused_custom_errors.py From 3c18062b0697cddda8ca14c8a7461f491301a507 Mon Sep 17 00:00:00 2001 From: Goran Vladika Date: Tue, 24 Sep 2024 14:26:59 +0200 Subject: [PATCH 12/16] Update comments --- slither/detectors/statements/unused_custom_errors.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/slither/detectors/statements/unused_custom_errors.py b/slither/detectors/statements/unused_custom_errors.py index cde22cc00..d73111a1f 100644 --- a/slither/detectors/statements/unused_custom_errors.py +++ b/slither/detectors/statements/unused_custom_errors.py @@ -62,7 +62,7 @@ def _detect(self) -> List[Output]: for custom_error in self.compilation_unit.custom_errors: declared_custom_errors.append(custom_error) - # Collect all custom errors used in revertsCustomError + # Collect all custom errors invoked in revert statements for contract in self.compilation_unit.contracts: for function in contract.functions_and_modifiers: for internal_call in function.internal_calls: @@ -70,11 +70,11 @@ def _detect(self) -> List[Output]: custom_reverts.append(internal_call) # Find unused custom errors - for defined_error in declared_custom_errors: + for declared_error in declared_custom_errors: if not any( - defined_error.name in custom_revert.name for custom_revert in custom_reverts + declared_error.name in custom_revert.name for custom_revert in custom_reverts ): - unused_custom_errors.append(defined_error) + unused_custom_errors.append(declared_error) results = [] if len(unused_custom_errors) > 0: From 415e65e910266bf41e5083ac87660595947c3ddf Mon Sep 17 00:00:00 2001 From: Goran Vladika Date: Tue, 24 Sep 2024 15:17:11 +0200 Subject: [PATCH 13/16] Remove leftovers --- slither/detectors/statements/unused_custom_errors.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/slither/detectors/statements/unused_custom_errors.py b/slither/detectors/statements/unused_custom_errors.py index d73111a1f..b3873ec57 100644 --- a/slither/detectors/statements/unused_custom_errors.py +++ b/slither/detectors/statements/unused_custom_errors.py @@ -54,7 +54,6 @@ def _detect(self) -> List[Output]: # Collect all custom errors defined in the contracts for contract in self.compilation_unit.contracts: - contract.custom_errors_declared for custom_error in contract.custom_errors: declared_custom_errors.append(custom_error) @@ -76,6 +75,7 @@ def _detect(self) -> List[Output]: ): unused_custom_errors.append(declared_error) + # Format results results = [] if len(unused_custom_errors) > 0: info: DETECTOR_INFO = ["The following unused error(s) should be removed:"] From b63e62a0f0f6871749dc973e72a6c9d7902a2dbd Mon Sep 17 00:00:00 2001 From: Goran Vladika Date: Thu, 26 Sep 2024 14:00:51 +0200 Subject: [PATCH 14/16] Use sets to avoid duplicates --- .../detectors/statements/unused_custom_errors.py | 16 ++++++++-------- ...8_15_ContractToTestForCustomErrors_sol__0.txt | 10 +++------- 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/slither/detectors/statements/unused_custom_errors.py b/slither/detectors/statements/unused_custom_errors.py index b3873ec57..0f0a73640 100644 --- a/slither/detectors/statements/unused_custom_errors.py +++ b/slither/detectors/statements/unused_custom_errors.py @@ -1,7 +1,7 @@ """ Module detecting unused custom errors """ -from typing import List +from typing import List, Set from slither.core.declarations.custom_error import CustomError from slither.core.declarations.custom_error_top_level import CustomErrorTopLevel from slither.core.declarations.solidity_variables import SolidityCustomRevert @@ -48,32 +48,32 @@ class UnusedCustomErrors(AbstractDetector): def _detect(self) -> List[Output]: """Detect unused custom errors""" - declared_custom_errors: List[CustomError] = [] - custom_reverts: List[SolidityCustomRevert] = [] - unused_custom_errors: List[CustomError] = [] + declared_custom_errors: Set[CustomError] = set() + custom_reverts: Set[SolidityCustomRevert] = set() + unused_custom_errors: Set[CustomError] = set() # Collect all custom errors defined in the contracts for contract in self.compilation_unit.contracts: for custom_error in contract.custom_errors: - declared_custom_errors.append(custom_error) + declared_custom_errors.add(custom_error) # Add custom errors defined outside of contracts for custom_error in self.compilation_unit.custom_errors: - declared_custom_errors.append(custom_error) + declared_custom_errors.add(custom_error) # Collect all custom errors invoked in revert statements for contract in self.compilation_unit.contracts: for function in contract.functions_and_modifiers: for internal_call in function.internal_calls: if isinstance(internal_call, SolidityCustomRevert): - custom_reverts.append(internal_call) + custom_reverts.add(internal_call) # Find unused custom errors for declared_error in declared_custom_errors: if not any( declared_error.name in custom_revert.name for custom_revert in custom_reverts ): - unused_custom_errors.append(declared_error) + unused_custom_errors.add(declared_error) # Format results results = [] diff --git a/tests/e2e/detectors/snapshots/detectors__detector_UnusedCustomErrors_0_8_15_ContractToTestForCustomErrors_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_UnusedCustomErrors_0_8_15_ContractToTestForCustomErrors_sol__0.txt index c62cbd5ec..4c7643166 100644 --- a/tests/e2e/detectors/snapshots/detectors__detector_UnusedCustomErrors_0_8_15_ContractToTestForCustomErrors_sol__0.txt +++ b/tests/e2e/detectors/snapshots/detectors__detector_UnusedCustomErrors_0_8_15_ContractToTestForCustomErrors_sol__0.txt @@ -1,15 +1,11 @@ The following unused error(s) should be removed: - -UnusedParentErr() (tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/ContractToTestForCustomErrors.sol) - - -UnusedParentErrWithArg(uint256) (tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/ContractToTestForCustomErrors.sol) + -UnusedLibError() (tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/CustomErrorsLib.sol) - -UnusedParentErr() (tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/ContractToTestForCustomErrors.sol) + -UnusedErrorLibConsumerErr() (tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/ErrorLibConsumer.sol) -UnusedParentErrWithArg(uint256) (tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/ContractToTestForCustomErrors.sol) - -UnusedLibError() (tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/CustomErrorsLib.sol) - - -UnusedErrorLibConsumerErr() (tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/ErrorLibConsumer.sol) + -UnusedParentErr() (tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/ContractToTestForCustomErrors.sol) -UnusedTopLevelErr() (tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/ContractToTestForCustomErrors.sol) From 44de39b8452a92ad20307c49b8e60eb5a351b6e8 Mon Sep 17 00:00:00 2001 From: Goran Vladika Date: Thu, 26 Sep 2024 14:54:28 +0200 Subject: [PATCH 15/16] Use sorted list to get deterministic order in output --- slither/detectors/statements/unused_custom_errors.py | 7 +++++-- ...mErrors_0_8_15_ContractToTestForCustomErrors_sol__0.txt | 6 +++--- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/slither/detectors/statements/unused_custom_errors.py b/slither/detectors/statements/unused_custom_errors.py index 0f0a73640..a1d64b4b3 100644 --- a/slither/detectors/statements/unused_custom_errors.py +++ b/slither/detectors/statements/unused_custom_errors.py @@ -50,7 +50,7 @@ def _detect(self) -> List[Output]: """Detect unused custom errors""" declared_custom_errors: Set[CustomError] = set() custom_reverts: Set[SolidityCustomRevert] = set() - unused_custom_errors: Set[CustomError] = set() + unused_custom_errors: List[CustomError] = [] # Collect all custom errors defined in the contracts for contract in self.compilation_unit.contracts: @@ -73,7 +73,10 @@ def _detect(self) -> List[Output]: if not any( declared_error.name in custom_revert.name for custom_revert in custom_reverts ): - unused_custom_errors.add(declared_error) + unused_custom_errors.append(declared_error) + + # Sort by error name + unused_custom_errors = sorted(unused_custom_errors, key=lambda err: err.name) # Format results results = [] diff --git a/tests/e2e/detectors/snapshots/detectors__detector_UnusedCustomErrors_0_8_15_ContractToTestForCustomErrors_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_UnusedCustomErrors_0_8_15_ContractToTestForCustomErrors_sol__0.txt index 4c7643166..5b3870de4 100644 --- a/tests/e2e/detectors/snapshots/detectors__detector_UnusedCustomErrors_0_8_15_ContractToTestForCustomErrors_sol__0.txt +++ b/tests/e2e/detectors/snapshots/detectors__detector_UnusedCustomErrors_0_8_15_ContractToTestForCustomErrors_sol__0.txt @@ -1,11 +1,11 @@ The following unused error(s) should be removed: - -UnusedLibError() (tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/CustomErrorsLib.sol) - -UnusedErrorLibConsumerErr() (tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/ErrorLibConsumer.sol) - -UnusedParentErrWithArg(uint256) (tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/ContractToTestForCustomErrors.sol) + -UnusedLibError() (tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/CustomErrorsLib.sol) -UnusedParentErr() (tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/ContractToTestForCustomErrors.sol) + -UnusedParentErrWithArg(uint256) (tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/ContractToTestForCustomErrors.sol) + -UnusedTopLevelErr() (tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/ContractToTestForCustomErrors.sol) From 9af60e669cf8a16eb056dc0d725ad01cdc970655 Mon Sep 17 00:00:00 2001 From: Goran Vladika Date: Thu, 10 Oct 2024 13:10:03 +0200 Subject: [PATCH 16/16] internal_call.function is instance of SolidityCustomRevert --- slither/detectors/statements/unused_custom_errors.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/slither/detectors/statements/unused_custom_errors.py b/slither/detectors/statements/unused_custom_errors.py index a1d64b4b3..356c2c6a1 100644 --- a/slither/detectors/statements/unused_custom_errors.py +++ b/slither/detectors/statements/unused_custom_errors.py @@ -65,8 +65,8 @@ def _detect(self) -> List[Output]: for contract in self.compilation_unit.contracts: for function in contract.functions_and_modifiers: for internal_call in function.internal_calls: - if isinstance(internal_call, SolidityCustomRevert): - custom_reverts.add(internal_call) + if isinstance(internal_call.function, SolidityCustomRevert): + custom_reverts.add(internal_call.function) # Find unused custom errors for declared_error in declared_custom_errors: