Generated via:
python tests/e2e/detectors/test_detectors.py --compile
pytest -k DivideBeforeMultiply-0.8.20-divide_before_multiply_reassignment --insta update
All 5 DivideBeforeMultiply test cases pass with the fix applied.
The `_explore` worklist in `slither/detectors/statements/divide_before_multiply.py`
tracks the set of currently-division-tainted IR variables in a `divisions` dict.
The Assignment handler only propagates taint from rvalue to lvalue when the
rvalue is already in `divisions`. There is no else branch, so when the rvalue
is a clean (non-divided) value, the lvalue's previous taint is never cleared.
A variable that was previously assigned a division result and is then fully
overwritten with an unrelated value keeps its stale taint, and the next
multiplication using it fires:
uint256 a = x / y; // divisions[a] = [node15]
a = z; // rvalue clean; divisions[a] never cleared
return a * 100; // FP: `a` here is `z`, not x/y
Clear the stale taint in an `else` branch on Assignment so the dataflow kill
matches the join semantics of full overwrite.
Adds tests/e2e/detectors/test_data/divide-before-multiply/0.8.20/divide_before_multiply_reassignment.sol
covering two FP shapes (direct reassign, reassign through a temporary) plus a
tripwire `genuineDivThenMul` that must still fire. After this change the
snapshot contains only the tripwire; the two FPs are gone.
All four existing DivideBeforeMultiply fixtures still pass with identical
output.
The change does not touch the multiplication-check itself, so the
intentional-precision-loss firings (`a / b * c` in fixed-point math) are
unchanged.
Generated via:
python tests/e2e/detectors/test_detectors.py --compile
pytest -k NamingConvention-0.8.20-naming_convention_yul_local --insta update
All 9 NamingConvention test cases pass with the fix applied.
The Yul parser mangles every local identifier to <source_name>__<scope_chain>
(solc_parsing/yul/parse_yul.py:130), so a Yul parameter `base_` declared
inside `unsafeExtend`'s 0th assembly block ends up named
`base__unsafeExtend_asm_0_extendInline`. The naming-convention detector
iterates `contract.functions_declared`, which includes these synthetic Yul-
local functions, and runs `is_mixed_case_with_underscore` on the mangled
name. The mangled name is a parser artifact, not an identifier the developer
wrote.
Synthetic Yul-local functions carry `Function.internal_scope` non-empty. Skip
them in the naming-convention loop so neither the synthetic function name nor
its parser-mangled parameters are checked.
Adds tests/e2e/detectors/test_data/naming-convention/0.8.20/naming_convention_yul_local.sol
covering two Yul-local function declarations (extendInline, memcpy) plus a
tripwire `tripwire(uint256 Bad_Param)` that uses a deliberately non-mixedCase
Solidity-level parameter. The snapshot after this change contains exactly the
tripwire finding; the Yul-local FPs are gone.
All 8 existing NamingConvention fixtures still pass with identical output.
Closes#1815.
is_revert checked ir.function.name in ["revert()", "revert(string"], where
the second literal is missing the closing paren. The real SolidityFunction
name for revert("msg") is "revert(string)" (see solidity_variables.py:55),
so the membership check never matched. Custom-error reverts are also missed
because SolidityCustomRevert builds its name as "revert " + signature.
Switch to explicit checks for "revert()" and "revert(string)" plus a
startswith("revert ") branch that covers any SolidityCustomRevert.name.
Adds tests/e2e/detectors/test_data/incorrect-modifier/0.8.20/modifier_revert_variants.sol
covering revert(), revert("msg"), revert MyError(), revert MyError(args)
and one negative case that should still fire.
register_detector iterates over self.compilation_units to create one
detector instance per compilation unit. register_printer was missing
this loop, creating a single instance regardless of the number of
compilation units.
Update register_printer to mirror register_detector by wrapping
instantiation in a for-loop over self.compilation_units.
Also update AbstractPrinter.__init__ to accept a compilation_unit
parameter (mirroring AbstractDetector), and update unregister_printer
to remove all instances of a given printer class (not just the first),
since multiple instances may now be registered.
Fixes: https://github.com/crytic/slither/issues/1780
The unprotected-upgrade detector whitelists `onlyProxy` initializers
because the modifier prevents calling the implementation contract
directly. `onlyDelegateCall` is a common community-defined synonym
that provides the same protection (e.g. minimal-proxy / ERC-1167
templates) and was reported as a false positive in #1136.
Add `onlyDelegateCall` to the modifier whitelist alongside
`onlyProxy` and extend the existing whitelisted.sol e2e fixtures
across all solc versions to cover it.
Closes#1136
The call-graph printer has a pre-existing bug where passing a filename
ending in .dot doesn't properly initialize all_contracts_filename,
causing FileNotFoundError. Changed test to use filename without .dot
suffix to avoid triggering this issue.
- Skip external call processing for FunctionTopLevel as they cannot make
high-level calls to contracts (only low-level calls)
- Remove unrelated changes to abstract_detector.py and abstract_checks.py
(--exclude hint should be in separate PR)
- Add test assertions for contract-to-top-level call edges
Addresses review feedback from @dguido
Previously, the call-graph printer would crash with an AttributeError when
analyzing contracts that use top-level functions (FunctionTopLevel), as it
assumed all functions have a contract_declarer attribute.
This commit:
- Adds support for FunctionTopLevel in the call-graph printer
- Creates a separate "[Top Level]" cluster for top-level functions
- Properly handles internal calls from/to top-level functions
- Adds a test to verify the fix
Fixes#1437
Virtual functions with implementations that are overridden by derived
contracts should not be flagged as dead code. These functions are part
of an inheritance design pattern where the base implementation provides
a default behavior that derived contracts can override.
Fixes#2500
When a function's return values are directly used in another function's
return statement (e.g., return A()), the detector was incorrectly
flagging them as unused.
The issue occurred because when tracking tuple return values, both the
tuple variable itself and its individual element indices were added to
the tracking list. When the tuple was used as a whole in a return
statement, only the tuple variable entry was removed, leaving the
individual element entries which caused false positives.
The fix ensures that when a tuple is used as a whole (not unpacked),
all individual element entries are also removed from the tracking list.
Fixes#2762
* fix: exclude library functions from dead-code detector
The dead-code detector incorrectly flagged library functions as dead code.
This fix updates the type annotation and set comprehension to properly
track library function canonical names.
Also adds test case and snapshot for library function usage.
Fixes#1265
* fix: address PR review feedback
- Remove unused Contract import from dead_code.py
- Fix misleading comment in test file that claimed unused library
function should be flagged (library functions are excluded from
dead-code detection)
Co-Authored-By: Claude Opus 4.5 <[email protected]>
---------
Co-authored-by: ep0chzer0 <[email protected]>
Co-authored-by: Dan Guido <[email protected]>
Co-authored-by: Claude Opus 4.5 <[email protected]>
* feat: add support for constant and immutable variables in slither-read-storage
This adds the --include-immutable flag to slither-read-storage which enables
display of constant and immutable variables alongside regular storage variables.
Features:
- New --include-immutable CLI flag
- Constant values extracted from expressions using ConstantFolding
- Immutable values retrieved via RPC getter calls for public variables
- Variables displayed with (constant) or (immutable) type suffix
- Slot shows -1 for non-storage variables
Fixes#1614
* fix: address code review findings for immutable/constant support
- Fix pre-existing bug: lambda filter used x[1].name but x is StateVariable, not tuple
- Use specific exception types instead of catching all Exception
- Handle None type by showing "unknown" instead of "None"
- Add debug log for private/internal immutables (cannot retrieve via RPC)
- Add tests for --variable-name filter with immutable/constant variables
* fix: add solc version to test fixture for CI compatibility
* fix: address remaining code review findings for immutable/constant support
- Remove unused `contract` parameter from `_get_immutable_value` method
- Remove unused `Path` import from test file
- Change test assertions from `>=` to `==` for exact count validation
Co-Authored-By: Claude Opus 4.5 <[email protected]>
---------
Co-authored-by: ep0chzer0 <[email protected]>
Co-authored-by: Dan Guido <[email protected]>
Co-authored-by: Claude Opus 4.5 <[email protected]>
* fix: handle unhashable types in using_for lookup
When the destination type `t` is a list (unhashable), the check
`t in using_for` raises TypeError. This can happen with certain
contract patterns involving tuple types.
Added try/except to gracefully handle unhashable types instead of
crashing.
Fixes#2140
* style: apply ruff formatting
* fix: add helper function and fix second unhashable type location
- Add _type_in_using_for() helper to centralize hashability checks
- Replace inline try/except in propagate_types() with helper call
- Fix unprotected t in using_for check in convert_to_library_or_top_level()
- Add debug logging for unhashable type encounters
- Add regression tests for unhashable type handling
Co-Authored-By: Claude Opus 4.5 <[email protected]>
* test: expand mutation testing for unhashable type fix
Add comprehensive tests that use actual Type objects instead of strings
and verify mutation-catching behavior:
- Test with ElementaryType objects
- Test unhashable list of types (simulating tuple returns)
- Test wildcard "*" fallthrough behavior
- Test that `return True` mutation would fail
- Add edge cases (None, empty dict, type equality)
Verified: 7/12 tests fail when `return False` is mutated to `return True`
Co-Authored-By: Claude Opus 4.5 <[email protected]>
---------
Co-authored-by: ep0chzer0 <[email protected]>
Co-authored-by: Dan Guido <[email protected]>
Co-authored-by: Claude Opus 4.5 <[email protected]>
* feat: add C3 linearization printer for inheritance analysis
Add a new printer that displays the C3 linearization order for contract
inheritance. This helps developers understand the method resolution order
(MRO), constructor execution order, and super call resolution.
Features:
- Shows linearization order with contract type labels (SELF/BASE/INHERITED)
- Displays source file locations
- Shows constructor presence for each contract
- Lists constructor execution order (reverse of linearization)
- Outputs both human-readable format and structured JSON
Fixes#2784
* fix: access linearizations from additional_fields in test
* fix: update WIKI URL to crytic, fix IndexError and PEP 8 style
- Update WIKI URL from trailofbits to crytic
- Check source_mapping.lines is non-empty before accessing [0]
- Replace lambda with named function for identity (PEP 8 E731)
Addresses review feedback from @dguido
* style: apply ruff formatting
* fix: address code review issues in C3 linearization printer
- Add defensive null check for `filename` before accessing `.short`
- Extract identity function outside loop for cleaner code
Co-Authored-By: Claude Opus 4.5 <[email protected]>
---------
Co-authored-by: ep0chzer0 <[email protected]>
Co-authored-by: Dan Guido <[email protected]>
Co-authored-by: Claude Opus 4.5 <[email protected]>
* feat: add --warn-unused-ignores flag for detecting unnecessary slither-disable comments
Implements issue #1680 - Warn when slither-disable comment is no longer necessary.
This feature adds the ability to detect and report slither-disable comments that
do not actually suppress any findings, helping maintain clean codebases.
Changes:
- Add tracking infrastructure in SlitherCore for all ignore comments
- Track which ignore comments are actually used when suppressing findings
- Add --warn-unused-ignores CLI flag to enable warning output
- Report unused ignore comments via logger.warning()
- Add unit tests for the feature
The implementation handles:
- slither-disable-next-line comments
- slither-disable-start/end block comments
- Multiple detectors on the same comment
- The "all" keyword
- Empty detector lists (filtered out)
- Whitespace in detector lists (stripped)
Closes#1680
* fix: add JSON output support and fix whitespace stripping
- Add unused ignore directives to JSON output (structured output first)
- Fix whitespace stripping in has_ignore_comment to match parse_ignore_comments
- Both parsing and checking now consistently strip whitespace from detector names
Addresses review feedback from @dguido
* style: apply ruff formatting
* fix: address test review feedback
- Move tempfile and os imports to module level
- Add whitespace to test_whitespace_in_detector_list_handled to actually
test whitespace handling in detector lists
Co-Authored-By: Claude Opus 4.5 <[email protected]>
---------
Co-authored-by: ep0chzer0 <[email protected]>
Co-authored-by: Dan Guido <[email protected]>
Co-authored-by: Claude Opus 4.5 <[email protected]>
* fix: add ArrayType and complex type support in _find_struct_var_slot
Fixes issue #1615 - slither-read-storage now properly handles arrays
and other complex types inside structs.
Changes to _find_struct_var_slot:
- Add ArrayType handling (both dynamic and fixed arrays)
- Add UserDefinedType handling with proper distinction between:
- Structure: starts on new slot boundary
- Enum/Contract: packs like elementary type
- Add MappingType handling (takes 1 slot)
The fix ensures correct slot calculations when iterating through
struct members to find storage locations.
Closes#1615
* fix: move imports to top of test file, add test location comment
- Move tempfile and os imports to module level (PEP 8 compliance)
- Remove unused imports (Path, pytest)
- Add comment explaining test file location rationale
Addresses review feedback from @dguido
* style: apply ruff formatting
* fix: use actual struct size and add explicit Enum/Contract checks
- Change hardcoded size=256 to var_type.storage_size[0] * 8 for nested
structs in _find_struct_var_slot
- Add explicit isinstance checks for Enum and Contract types instead of
implicit else branch
- Strengthen test assertions to verify actual slot values, sizes, and
offsets instead of just checking len(slot_info) > 0
Co-Authored-By: Claude Opus 4.5 <[email protected]>
---------
Co-authored-by: ep0chzer0 <[email protected]>
Co-authored-by: Dan Guido <[email protected]>
Co-authored-by: Claude Opus 4.5 <[email protected]>
* feat: add msg-value-in-nonpayable detector
Implements #2781. Detects uses of msg.value in functions that cannot be
reached from any payable entry point.
- Uses call graph analysis via all_reachable_from_functions
- Properly formats output using Slither objects for JSON/SARIF
- Includes test cases for internal function call chains
- Limited to Solidity (LANGUAGE = "solidity")
Note: Solidity 0.8+ prevents direct msg.value use in non-payable
public/external functions at compile time. This detector catches
internal functions that use msg.value but are only reachable from
non-payable entry points.
* fix: update type annotations and add test artifacts
- Replace deprecated typing.List/Tuple with builtin list/tuple (ruff UP035/UP006)
- Add from __future__ import annotations for PEP 604 support
- Add missing test zip artifact and snapshot file
* fix: correct snapshot order for msg-value-in-nonpayable detector
---------
Co-authored-by: ep0chzer0 <[email protected]>
Co-authored-by: Dan Guido <[email protected]>
* fix: classify external calls in yul/vyper
* fix: Apply ruff formatting to test file
Co-Authored-By: Claude Opus 4.5 <[email protected]>
---------
Co-authored-by: Dan Guido <[email protected]>
Co-authored-by: Claude Opus 4.5 <[email protected]>
* Add --exclude-location flag to hide file locations in detector messages
This adds a new CLI flag `--exclude-location` that allows users to exclude
file location information (filename and line numbers) from detector output.
This is useful for cleaner output when locations are not needed.
Changes:
- Add OutputConfig class with EXCLUDE_LOCATION setting (matches colors.py pattern)
- Add _format_with_location helper function for consistent formatting
- Update _convert_to_description to use helper for conditional location
- Add CLI flag --exclude-location in __main__.py
- Add default to command_line.py
- Add unit tests for the new functionality
Also includes inheritance printer fixes (issue #1835):
- Remove extra newlines causing malformed output
- Fix bug storing `immediate` in `not_immediate` key
- Use PEP8 style `child not in` instead of `not child in`
Fixes#2222
* style: fix ruff formatting in test_output.py
* test: add fixture to reset OutputConfig between tests
Prevents test pollution if test order changes.
Co-Authored-By: Claude Opus 4.5 <[email protected]>
* test: improve test_output.py with real objects and add coverage
- Replace fragile MagicMock with real Slither objects from compilation
- Add coverage for Node, Function, Contract paths in _convert_to_description
- Add PrinterInheritance test to verify text printer output structure
- Rename test_inheritance_printer to test_inheritance_graph_printer for clarity
Co-Authored-By: Claude Opus 4.5 <[email protected]>
* fix: use lenient threshold for timing test on Windows
Windows timer resolution can cause time.sleep(0.01) to return slightly
early (~0.0092s), causing test flakiness. Lower threshold to 0.008.
Co-Authored-By: Claude Opus 4.5 <[email protected]>
---------
Co-authored-by: ep0chzer0 <[email protected]>
Co-authored-by: Dan Guido <[email protected]>
Co-authored-by: Claude Opus 4.5 <[email protected]>
Add 42 tests covering:
- All 26 IR operation types in copy_ir()
- SSA variable conversion (local, state, temporary, reference, tuple)
- Phi node generation for control flow (if/else, loops, nested)
- Critical edge cases (LibraryCall subclass preservation, None lvalues)
- Snapshot tests for SSA output stability
These tests provide a safety net for future refactoring of the
SSA conversion code.
Co-authored-by: Claude Opus 4.5 <[email protected]>
* perf: optimize Type.__eq__ methods with direct attribute access
Replace property access with direct attribute access in __eq__ methods
for ElementaryType, ArrayType, MappingType, FunctionType, and TypeInformation.
Changes:
- Use `type(other) is X` instead of `isinstance(other, X)` (no subclasses exist)
- Access private attributes directly instead of through properties
Microbenchmark shows 2.4x speedup for ElementaryType.__eq__ alone.
ElementaryType.__eq__ is called 41M times on the optimism codebase.
Co-Authored-By: Claude Opus 4.5 <[email protected]>
* test: add unit tests for Type.__eq__ methods
Verify correctness of __eq__ optimizations for ElementaryType,
ArrayType, MappingType, FunctionType, and TypeInformation.
Co-Authored-By: Claude Opus 4.5 <[email protected]>
---------
Co-authored-by: Claude Opus 4.5 <[email protected]>
* perf: cache Literal.__str__ conversion for numeric types
Optimize Literal.__str__() by caching the result of convert_string_to_int()
during construction. This avoids repeated expensive conversions when the
same literal is stringified multiple times during detector execution.
Changes:
- Add module-level _NUMERIC_TYPES frozenset for O(1) type lookup
- Cache converted string in __init__ for numeric string types
- Skip caching when subdenomination is set (uses converted_value instead)
- Preserve original behavior for ElementaryType inputs (no conversion)
Benchmark: ~16% faster on optimism codebase (50.7s vs ~61s)
Co-Authored-By: Claude Opus 4.5 <[email protected]>
* fix: use lenient threshold for timing test on Windows
Windows has lower timer resolution (~15.6ms default) and time.sleep()
can return slightly early. Lower the assertion threshold from 0.01 to
0.005 to account for this variance.
Co-Authored-By: Claude Opus 4.5 <[email protected]>
* fix: address PR review feedback for Literal caching
- Add unit tests for Literal class covering caching behavior
- Restore comment explaining non-numeric type fallback path
- Note: test_timing.py threshold lowered for Windows timer resolution
Co-Authored-By: Claude Opus 4.5 <[email protected]>
---------
Co-authored-by: Claude Opus 4.5 <[email protected]>
Simone