diff --git a/slither/detectors/all_detectors.py b/slither/detectors/all_detectors.py index a30d2b3c0..437222346 100644 --- a/slither/detectors/all_detectors.py +++ b/slither/detectors/all_detectors.py @@ -104,5 +104,6 @@ from .functions.chainlink_feed_registry import ChainlinkFeedRegistry from .functions.pyth_deprecated_functions import PythDeprecatedFunctions from .functions.optimism_deprecation import OptimismDeprecation +from .statements.unused_custom_errors import UnusedCustomErrors # from .statements.unused_import import UnusedImport diff --git a/slither/detectors/statements/unused_custom_errors.py b/slither/detectors/statements/unused_custom_errors.py new file mode 100644 index 000000000..356c2c6a1 --- /dev/null +++ b/slither/detectors/statements/unused_custom_errors.py @@ -0,0 +1,94 @@ +""" +Module detecting unused custom errors +""" +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 +from slither.detectors.abstract_detector import ( + AbstractDetector, + DetectorClassification, + DETECTOR_INFO, +) +from slither.utils.output import Output + + +class UnusedCustomErrors(AbstractDetector): + """ + Unused custom errors detector + """ + + ARGUMENT = "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-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""" + declared_custom_errors: Set[CustomError] = set() + custom_reverts: Set[SolidityCustomRevert] = set() + unused_custom_errors: List[CustomError] = [] + + # 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.add(custom_error) + + # Add custom errors defined outside of contracts + for custom_error in self.compilation_unit.custom_errors: + 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.function, SolidityCustomRevert): + custom_reverts.add(internal_call.function) + + # 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) + + # Sort by error name + unused_custom_errors = sorted(unused_custom_errors, key=lambda err: err.name) + + # Format results + 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 + ) + info += ["\n\t-", custom_error.full_name, " (", file_scope.filename.short, ")\n"] + results.append(self.generate_result(info)) + + return 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 new file mode 100644 index 000000000..5b3870de4 --- /dev/null +++ b/tests/e2e/detectors/snapshots/detectors__detector_UnusedCustomErrors_0_8_15_ContractToTestForCustomErrors_sol__0.txt @@ -0,0 +1,11 @@ +The following unused error(s) should be removed: + -UnusedErrorLibConsumerErr() (tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/ErrorLibConsumer.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) + 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 000000000..9efb0d8eb Binary files /dev/null and b/tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/ContractToTestForCustomErrors.sol-0.8.15.zip differ diff --git a/tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/CustomErrorsLib.sol b/tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/CustomErrorsLib.sol new file mode 100644 index 000000000..7c81c5026 --- /dev/null +++ b/tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/CustomErrorsLib.sol @@ -0,0 +1,8 @@ +// SPDX-License-Identifier: Unlicense +pragma solidity 0.8.15; + +library Lib { + error UnusedLibError(); + error UsedLibErrorA(); + error UsedLibErrorB(uint256 x); +} \ No newline at end of file diff --git a/tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/ErrorLibConsumer.sol b/tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/ErrorLibConsumer.sol new file mode 100644 index 000000000..9f173a721 --- /dev/null +++ b/tests/e2e/detectors/test_data/unused-custom-errors/0.8.15/ErrorLibConsumer.sol @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: Unlicense +pragma solidity 0.8.15; + +import "./CustomErrorsLib.sol"; + +contract ErrorLibConsumer { + error UnusedErrorLibConsumerErr(); + error UsedErrorLibConsumerErr(); + + constructor() { + revert Lib.UsedLibErrorA(); + } + + function a() public pure { + revert UsedErrorLibConsumerErr(); + } +} diff --git a/tests/e2e/detectors/test_detectors.py b/tests/e2e/detectors/test_detectors.py index d2f191a4d..74a68cf63 100644 --- a/tests/e2e/detectors/test_detectors.py +++ b/tests/e2e/detectors/test_detectors.py @@ -401,6 +401,11 @@ def id_test(test_item: Test): "unused_state.sol", "0.7.6", ), + Test( + all_detectors.UnusedCustomErrors, + "ContractToTestForCustomErrors.sol", + "0.8.15", + ), Test(all_detectors.LockedEther, "locked_ether.sol", "0.4.25"), Test(all_detectors.LockedEther, "locked_ether.sol", "0.5.16"), Test(all_detectors.LockedEther, "locked_ether.sol", "0.6.11"),