fix: classify using-for library calls as internal (#2073)#3010
Open
MycCellium420 wants to merge 2 commits intocrytic:masterfrom
Open
fix: classify using-for library calls as internal (#2073)#3010MycCellium420 wants to merge 2 commits intocrytic:masterfrom
MycCellium420 wants to merge 2 commits intocrytic:masterfrom
Conversation
Add TestClassifyCalls covering the false-positive cases reported in crytic#2073: - abi.encode() and other SolidityVariable member calls → internal - direct library calls (MyLib.foo()) → internal - this.foo() → external - state/local variable member calls → external - is_external_identifier predicate override - empty list and mixed-call scenarios Tests run without a Solidity compiler, using mock Expression objects so they are fast and environment-independent. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
`addr.sendValue(amount)` via `using Address for address` was incorrectly reported as an external call in the function-summary printer because classify_calls ran before using-for directives were parsed. Changes: - Add `_is_using_for_library_call` helper that checks whether a variable's type has a matching library function in the using-for map - Add optional `using_for` parameter to `classify_calls` - Re-classify calls in `_analyze_using_for` after directives are available, fixing the timing issue - Add 16 unit tests covering all classification scenarios Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
19f9ed0 to
28fabd4
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
Fixes #2073
addr.sendValue(amount)viausing Address for addresswas incorrectly reported as an external call in thefunction-summaryprinter. Root cause:classify_callsran duringanalyze_expressions— before_analyze_using_forhad parsed theusing … fordirectives.Changes
slither/visitors/expression/call_classification.py_is_using_for_library_call()helper checks if a variable's type has a matching library function viausing forclassify_calls()gains an optionalusing_forparameterslither/solc_parsing/slither_compilation_unit_solc.py_analyze_using_for, after directives are availableslither/solc_parsing/cfg/node.pytests/unit/core/test_external_calls_classification.pyVerified with real Solidity (solc 0.8.24)
r.sendValue()(using-for)a.add(b)(using-for)token.transfer()(real)Test plan
TestClassifyCalls+TestUsingForClassification)slither test.sol --print function-summaryverified with solc 0.8.24 on Linuxabi.encode, direct library calls,this.foo()all still classified correctly🤖 Generated with Claude Code