Skip to content

Commit 204452f

Browse files
committed
Add detector for unused custom error definitions
Implements a new detector (unused-error) that identifies custom error definitions that are declared but never used in revert statements. Features: - Detects unused contract-level custom errors - Detects unused top-level custom errors - Handles errors with and without parameters - Skips interface contracts (signature-only) Fixes #2587
1 parent 87c97d9 commit 204452f

File tree

4 files changed

+228
-0
lines changed

4 files changed

+228
-0
lines changed

slither/detectors/all_detectors.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
from .reentrancy.reentrancy_no_gas import ReentrancyNoGas
1919
from .reentrancy.reentrancy_events import ReentrancyEvent
2020
from .variables.unused_state_variables import UnusedStateVars
21+
from .variables.unused_custom_errors import UnusedCustomErrors
2122
from .variables.could_be_constant import CouldBeConstant
2223
from .variables.could_be_immutable import CouldBeImmutable
2324
from .statements.tx_origin import TxOrigin
Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,164 @@
1+
"""
2+
Module detecting unused custom errors
3+
"""
4+
5+
from slither.core.declarations import Function, Contract
6+
from slither.core.declarations.custom_error import CustomError
7+
from slither.core.declarations.custom_error_contract import CustomErrorContract
8+
from slither.core.declarations.custom_error_top_level import CustomErrorTopLevel
9+
from slither.core.declarations.solidity_variables import SolidityCustomRevert
10+
from slither.detectors.abstract_detector import (
11+
AbstractDetector,
12+
DetectorClassification,
13+
DETECTOR_INFO,
14+
)
15+
from slither.slithir.operations import SolidityCall
16+
from slither.utils.output import Output
17+
18+
19+
def _detect_unused_custom_errors_in_contract(
20+
contract: Contract,
21+
) -> list[CustomErrorContract]:
22+
"""
23+
Detect unused custom errors declared in a contract.
24+
25+
Args:
26+
contract: The contract to analyze
27+
28+
Returns:
29+
List of unused custom errors declared in the contract
30+
"""
31+
# Get all custom errors declared in this contract
32+
declared_errors = set(contract.custom_errors_declared)
33+
34+
if not declared_errors:
35+
return []
36+
37+
# Find all custom errors used in this contract and its derived contracts
38+
used_errors: set[CustomError] = set()
39+
40+
# Check all functions in the contract (including inherited)
41+
all_functions = [
42+
f
43+
for f in contract.all_functions_called + list(contract.modifiers)
44+
if isinstance(f, Function)
45+
]
46+
47+
for func in all_functions:
48+
for node in func.nodes:
49+
for ir in node.all_slithir_operations():
50+
if isinstance(ir, SolidityCall) and isinstance(
51+
ir.function, SolidityCustomRevert
52+
):
53+
used_errors.add(ir.function.custom_error)
54+
55+
# Return unused errors
56+
return [error for error in declared_errors if error not in used_errors]
57+
58+
59+
def _detect_unused_custom_errors_top_level(
60+
compilation_unit,
61+
) -> list[CustomErrorTopLevel]:
62+
"""
63+
Detect unused top-level custom errors.
64+
65+
Args:
66+
compilation_unit: The compilation unit to analyze
67+
68+
Returns:
69+
List of unused top-level custom errors
70+
"""
71+
# Get all top-level custom errors
72+
top_level_errors = set(compilation_unit.custom_errors)
73+
74+
if not top_level_errors:
75+
return []
76+
77+
# Find all custom errors used across all contracts
78+
used_errors: set[CustomError] = set()
79+
80+
for contract in compilation_unit.contracts:
81+
all_functions = [
82+
f
83+
for f in contract.all_functions_called + list(contract.modifiers)
84+
if isinstance(f, Function)
85+
]
86+
87+
for func in all_functions:
88+
for node in func.nodes:
89+
for ir in node.all_slithir_operations():
90+
if isinstance(ir, SolidityCall) and isinstance(
91+
ir.function, SolidityCustomRevert
92+
):
93+
used_errors.add(ir.function.custom_error)
94+
95+
# Also check top-level functions
96+
for func in compilation_unit.functions_top_level:
97+
for node in func.nodes:
98+
for ir in node.all_slithir_operations():
99+
if isinstance(ir, SolidityCall) and isinstance(
100+
ir.function, SolidityCustomRevert
101+
):
102+
used_errors.add(ir.function.custom_error)
103+
104+
# Return unused top-level errors
105+
return [error for error in top_level_errors if error not in used_errors]
106+
107+
108+
class UnusedCustomErrors(AbstractDetector):
109+
"""
110+
Detector for unused custom error definitions
111+
"""
112+
113+
ARGUMENT = "unused-error"
114+
HELP = "Unused custom error definitions"
115+
IMPACT = DetectorClassification.INFORMATIONAL
116+
CONFIDENCE = DetectorClassification.HIGH
117+
118+
WIKI = "https://github.com/crytic/slither/wiki/Detector-Documentation#unused-custom-error"
119+
120+
WIKI_TITLE = "Unused custom error"
121+
WIKI_DESCRIPTION = "Detects custom error definitions that are never used. Unused custom errors may indicate missing error handling logic or dead code that should be removed."
122+
123+
WIKI_EXPLOIT_SCENARIO = """
124+
```solidity
125+
contract VendingMachine {
126+
error Unauthorized(); // Defined but never used
127+
address payable owner = payable(msg.sender);
128+
129+
function withdraw() public {
130+
// Missing: if (msg.sender != owner) revert Unauthorized();
131+
owner.transfer(address(this).balance);
132+
}
133+
}
134+
```
135+
The `Unauthorized` error is defined but never used, suggesting the developer may have forgotten to add access control checks."""
136+
137+
WIKI_RECOMMENDATION = "Use the custom error in a `revert` statement, or remove the error definition if it is not needed."
138+
139+
def _detect(self) -> list[Output]:
140+
"""Detect unused custom errors"""
141+
results: list[Output] = []
142+
143+
# Check for unused custom errors in each contract
144+
for contract in self.compilation_unit.contracts_derived:
145+
if contract.is_signature_only():
146+
continue
147+
148+
unused_errors = _detect_unused_custom_errors_in_contract(contract)
149+
for error in unused_errors:
150+
info: DETECTOR_INFO = [
151+
error,
152+
" is declared but never used in ",
153+
contract,
154+
"\n",
155+
]
156+
results.append(self.generate_result(info))
157+
158+
# Check for unused top-level custom errors
159+
unused_top_level = _detect_unused_custom_errors_top_level(self.compilation_unit)
160+
for error in unused_top_level:
161+
info = [error, " is declared but never used\n"]
162+
results.append(self.generate_result(info))
163+
164+
return results
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
// SPDX-License-Identifier: GPL-3.0
2+
pragma solidity ^0.8.4;
3+
4+
// Top-level error - unused
5+
error TopLevelUnused();
6+
7+
// Top-level error - used
8+
error TopLevelUsed(address account);
9+
10+
contract UnusedErrorTest {
11+
// Contract-level error - unused
12+
error Unauthorized();
13+
14+
// Contract-level error - used
15+
error InsufficientBalance(uint256 available, uint256 required);
16+
17+
// Contract-level error - unused with parameters
18+
error InvalidAmount(uint256 amount);
19+
20+
address payable owner = payable(msg.sender);
21+
uint256 public balance;
22+
23+
function withdraw(uint256 amount) public {
24+
if (msg.sender != owner) {
25+
// Using InsufficientBalance but not Unauthorized
26+
revert InsufficientBalance(balance, amount);
27+
}
28+
owner.transfer(amount);
29+
}
30+
31+
function topLevelCheck(address account) public pure {
32+
// Using top-level error
33+
revert TopLevelUsed(account);
34+
}
35+
}
36+
37+
// Contract that uses all its errors (no findings expected)
38+
contract NoUnusedErrors {
39+
error AccessDenied();
40+
error InvalidInput(string reason);
41+
42+
function checkAccess(bool allowed) public pure {
43+
if (!allowed) {
44+
revert AccessDenied();
45+
}
46+
}
47+
48+
function validateInput(string memory input) public pure {
49+
if (bytes(input).length == 0) {
50+
revert InvalidInput("empty input");
51+
}
52+
}
53+
}
54+
55+
// Interface - should not report (signature only)
56+
interface IErrors {
57+
error InterfaceError();
58+
}

tests/e2e/detectors/test_detectors.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1944,6 +1944,11 @@ def id_test(test_item: Test):
19441944
# "C.sol",
19451945
# "0.8.16",
19461946
# ),
1947+
Test(
1948+
all_detectors.UnusedCustomErrors,
1949+
"unused_custom_error.sol",
1950+
"0.8.4",
1951+
),
19471952
]
19481953

19491954
GENERIC_PATH = "/GENERIC_PATH"

0 commit comments

Comments
 (0)