Fix external_calls_as_expressions to exclude builtins and library calls #2906
+169
−15
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Author
|
Hello maintainers! This fixes the bug where |
ep0chzer0
force-pushed
the
fix/external-calls-filtering
branch
2 times, most recently
from
9e4e5ef to
395d75b
Compare
Add a helpful line after the Reference URL showing how to exclude the detector using --exclude flag. This helps users quickly suppress specific findings without needing to look up the detector name. Example output: Reference: https://github.com/crytic/slither/wiki/...#timestamp Use --exclude timestamp to suppress this detector. Also updated the upgradeability checks for consistency. Fixes crytic#1316
The dead-code detector had a bug where it tried to index a Function object as if it were a tuple. The `all_library_calls()` method returns LibraryCall objects, and `item.function` extracts a Function, not a tuple of (Contract, Function). This fix: - Corrects the type annotation from `list[tuple[Contract, Function]]` to `list[Function]` - Fixes the set comprehension to access `f.canonical_name` directly instead of `lib[1].canonical_name` This ensures library functions used via `using X for Y` syntax are properly tracked and not incorrectly reported as dead code. Fixes crytic#1265
The function-summary printer was incorrectly reporting all member access expressions as external calls, including: - Solidity builtins (abi.encode, msg.sender, block.timestamp) - Library calls (Address.sendValue, SafeMath.add) - Struct field accesses This fix adds a helper function `_is_external_call()` that properly filters calls by checking: 1. If the called expression is an Identifier -> internal call 2. If it's a MemberAccess where the expression is a SolidityVariable -> builtin call (not external) 3. If it's a MemberAccess where the expression is a library Contract -> library call (not external) The fix is applied consistently to all three parsers: - slither/solc_parsing/cfg/node.py - slither/vyper_parsing/cfg/node.py - slither/solc_parsing/yul/parse_yul.py Fixes crytic#2073
ep0chzer0
force-pushed
the
fix/external-calls-filtering
branch
from
395d75b to
2016144
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
--print function-summaryincorrectly reporting builtins and library calls as external callsDetails
The function-summary printer was incorrectly reporting all member access expressions as external calls. This includes:
abi.encode,msg.sender,block.timestampAddress.sendValue,SafeMath.addRoot cause
The original logic was:
This only checked if the call target was an
Identifier(internal call), but didn't account forMemberAccessexpressions that could be builtins or library calls.Fix
Added a helper function
_is_external_call()that properly classifies calls:Identifier-> internal call (not external)MemberAccesswhere expression isSolidityVariable-> builtin (not external)MemberAccesswhere expression is a libraryContract-> library call (not external)Test plan
Fixes #2073
🤖 Generated with Claude Code