Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions slither/core/declarations/function_contract.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,16 @@ def get_summary(
Return the function summary
Returns:
(str, str, str, list(str), list(str), listr(str), list(str), list(str);
contract_name, name, visibility, modifiers, vars read, vars written, internal_calls, external_calls_as_expressions
contract_name, name, visibility, modifiers, vars read, vars written, internal_calls, external_calls
"""
# Get true external calls by excluding library calls from high-level calls
# This is more accurate than external_calls_as_expressions which is set at parse time
# and cannot distinguish library calls via "using X for Y" syntax
library_call_irs = set(self.library_calls)
external_calls = [
(contract, ir) for contract, ir in self.high_level_calls if ir not in library_call_irs
]

return (
self.contract_declarer.name,
self.full_name,
Expand All @@ -113,7 +121,7 @@ def get_summary(
[str(x) for x in self.state_variables_read + self.solidity_variables_read],
[str(x) for x in self.state_variables_written],
[str(x) for x in self.internal_calls],
[str(x) for x in self.external_calls_as_expressions],
[f"{contract.name}.{ir.function_name}()" for contract, ir in external_calls],
compute_cyclomatic_complexity(self),
)

Expand Down
12 changes: 10 additions & 2 deletions slither/core/declarations/function_top_level.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,16 @@ def get_summary(
Return the function summary
Returns:
(str, str, str, list(str), list(str), listr(str), list(str), list(str);
contract_name, name, visibility, modifiers, vars read, vars written, internal_calls, external_calls_as_expressions
contract_name, name, visibility, modifiers, vars read, vars written, internal_calls, external_calls
"""
# Get true external calls by excluding library calls from high-level calls
# This is more accurate than external_calls_as_expressions which is set at parse time
# and cannot distinguish library calls via "using X for Y" syntax
library_call_irs = set(self.library_calls)
external_calls = [
(contract, ir) for contract, ir in self.high_level_calls if ir not in library_call_irs
]

return (
"",
self.full_name,
Expand All @@ -84,7 +92,7 @@ def get_summary(
[str(x) for x in self.state_variables_read + self.solidity_variables_read],
[str(x) for x in self.state_variables_written],
[str(x) for x in self.internal_calls],
[str(x) for x in self.external_calls_as_expressions],
[f"{contract.name}.{ir.function_name}()" for contract, ir in external_calls],
)

# endregion
Expand Down
80 changes: 73 additions & 7 deletions slither/solc_parsing/cfg/node.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
from typing import Union, Optional, Dict, TYPE_CHECKING
from typing import Union, Optional, Dict, List, Tuple, TYPE_CHECKING

from slither.core.cfg.node import Node
from slither.core.cfg.node import NodeType
from slither.core.declarations import Contract
from slither.core.declarations.solidity_variables import SolidityVariable
from slither.core.expressions.assignment_operation import (
AssignmentOperation,
AssignmentOperationType,
)
from slither.core.expressions.call_expression import CallExpression
from slither.core.expressions.identifier import Identifier
from slither.core.expressions.member_access import MemberAccess
from slither.solc_parsing.expressions.expression_parsing import parse_expression
from slither.visitors.expression.find_calls import FindCalls
from slither.visitors.expression.read_var import ReadVar
Expand All @@ -17,6 +21,67 @@
from slither.solc_parsing.declarations.modifier import ModifierSolc


def _classify_calls(
calls: List[CallExpression],
) -> Tuple[List[CallExpression], List[CallExpression]]:
"""
Classify call expressions into internal and external calls.

External calls are calls to external contracts (e.g., token.transfer()).
Internal calls include:
- Direct function calls (e.g., myFunc())
- Solidity built-in calls (e.g., abi.encode(), abi.decode())
- Library calls (e.g., SafeMath.add())

Args:
calls: List of CallExpression to classify

Returns:
Tuple of (internal_calls, external_calls)
"""
internal_calls: List[CallExpression] = []
external_calls: List[CallExpression] = []

for call in calls:
called = call.called

if isinstance(called, Identifier):
# Direct function call like myFunc() or require()
# These are internal calls (including Solidity built-ins accessed directly)
internal_calls.append(call)

elif isinstance(called, MemberAccess):
# Member access like x.foo()
# Need to determine if x is:
# - A Solidity built-in (abi, msg, block, etc.) -> internal
# - A library contract -> internal
# - An external contract/address -> external
base_expr = called.expression

if isinstance(base_expr, Identifier):
base_value = base_expr.value

# Check if it's a Solidity built-in variable (abi, msg, block, tx, etc.)
# Note: "this" is a SolidityVariable but this.foo() is an external call
# (uses CALL opcode), so we exclude it from internal calls
if isinstance(base_value, SolidityVariable) and base_value.name != "this":
internal_calls.append(call)
# Check if it's a library contract
elif isinstance(base_value, Contract) and base_value.is_library:
internal_calls.append(call)
else:
# External contract call
external_calls.append(call)
else:
# Complex expression like getContract().foo() - treat as external
external_calls.append(call)
else:
# Other cases (e.g., complex expressions) - treat as external to be safe
external_calls.append(call)

return internal_calls, external_calls


class NodeSolc:
def __init__(self, node: Node) -> None:
self._unparsed_expression: Optional[Dict] = None
Expand Down Expand Up @@ -62,9 +127,10 @@ def analyze_expressions(self, caller_context: Union["FunctionSolc", "ModifierSol

find_call = FindCalls(expression)
self._node.calls_as_expression = find_call.result()
self._node.external_calls_as_expressions = [
c for c in self._node.calls_as_expression if not isinstance(c.called, Identifier)
]
self._node.internal_calls_as_expressions = [
c for c in self._node.calls_as_expression if isinstance(c.called, Identifier)
]

# Classify calls into internal and external
# Internal: direct calls, Solidity built-ins (abi.encode, etc.), library calls
# External: calls to external contracts
internal, external = _classify_calls(self._node.calls_as_expression)
self._node.internal_calls_as_expressions = internal
self._node.external_calls_as_expressions = external
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

// Test case for issue #2073: function-summary printer incorrectly reports
// abi.encode, library calls, and struct field access as external calls

// Library for testing library calls
library SafeMath {
function add(uint256 a, uint256 b) internal pure returns (uint256) {
return a + b;
}

function sub(uint256 a, uint256 b) internal pure returns (uint256) {
return a - b;
}
}

// Interface for external calls
interface IExternalContract {
function externalFunction() external returns (uint256);
function anotherExternal(uint256 x) external returns (uint256);
}

// Contract to test Solidity built-in calls (should NOT be external calls)
contract TestBuiltins {
function useAbiEncode(uint256 a, address b) external pure returns (bytes memory) {
return abi.encode(a, b);
}

function useAbiEncodePacked(uint256 a) external pure returns (bytes memory) {
return abi.encodePacked(a);
}

function useAbiEncodeWithSelector(bytes4 selector, uint256 a) external pure returns (bytes memory) {
return abi.encodeWithSelector(selector, a);
}

function useAbiDecode(bytes memory data) external pure returns (uint256, address) {
return abi.decode(data, (uint256, address));
}

function useKeccak256(bytes memory data) external pure returns (bytes32) {
return keccak256(data);
}
}

// Contract to test library calls (should NOT be external calls)
contract TestLibraryCalls {
using SafeMath for uint256;

function useLibraryWithUsing(uint256 a, uint256 b) external pure returns (uint256) {
return a.add(b);
}

function useLibraryDirect(uint256 a, uint256 b) external pure returns (uint256) {
return SafeMath.add(a, b);
}

function useMultipleLibraryCalls(uint256 a, uint256 b, uint256 c) external pure returns (uint256) {
uint256 sum = SafeMath.add(a, b);
return SafeMath.sub(sum, c);
}
}

// Contract to test struct field access (should NOT be external calls)
contract TestStructAccess {
struct Data {
uint256 value;
address owner;
}

Data private data;

function setData(uint256 v, address o) external {
data.value = v;
data.owner = o;
}

function getValue() external view returns (uint256) {
return data.value;
}

function getOwner() external view returns (address) {
return data.owner;
}
}

// Contract to test TRUE external calls (SHOULD be external calls)
contract TestExternalCalls {
IExternalContract private externalContract;

constructor(IExternalContract _external) {
externalContract = _external;
}

function callExternal() external returns (uint256) {
return externalContract.externalFunction();
}

function callExternalWithArg(uint256 x) external returns (uint256) {
return externalContract.anotherExternal(x);
}

function callMultipleExternal(uint256 x) external returns (uint256) {
uint256 a = externalContract.externalFunction();
uint256 b = externalContract.anotherExternal(x);
return a + b;
}
}

// Mixed contract testing all patterns
contract TestMixed {
using SafeMath for uint256;

IExternalContract private externalContract;

struct Config {
uint256 threshold;
}

Config private config;

constructor(IExternalContract _external) {
externalContract = _external;
}

// Should have 1 external call (externalContract.externalFunction)
// Should NOT count: abi.encode, SafeMath.add, config.threshold
function mixedOperations(uint256 a, uint256 b) external returns (bytes memory) {
uint256 sum = SafeMath.add(a, b);
uint256 external_val = externalContract.externalFunction();
uint256 threshold = config.threshold;
return abi.encode(sum, external_val, threshold);
}
}
94 changes: 94 additions & 0 deletions tests/e2e/printers/test_function_summary.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
"""
Tests for the function-summary printer, specifically for issue #2073.

Issue #2073: The function-summary printer incorrectly reports all statements
with '.' as external calls, including:
- Solidity built-ins like abi.encode(), abi.decode()
- Library calls like SafeMath.add()
- Struct field access

This test verifies that only true external contract calls are reported
in the "External Calls" column of the function-summary printer.
"""

from pathlib import Path

from crytic_compile import CryticCompile
from crytic_compile.platform.solc_standard_json import SolcStandardJson

from slither import Slither


TEST_DATA_DIR = Path(__file__).resolve().parent / "test_data" / "test_function_summary"


def _get_external_calls_from_summary(func):
"""Get external calls from function summary (index 7 in the tuple)."""
summary = func.get_summary()
return summary[7] # external_calls is at index 7


def _assert_no_external_calls_in_summary(contract, contract_name):
"""Helper to assert a contract's functions have no external calls in summary."""
for func in contract.functions:
if func.is_constructor:
continue
external_calls = _get_external_calls_from_summary(func)
assert len(external_calls) == 0, (
f"{contract_name}.{func.name} should have no external calls, "
f"but found: {external_calls}"
)


def test_function_summary_external_calls(solc_binary_path) -> None:
"""
Test that the function-summary correctly classifies external calls.

- Solidity built-ins (abi.encode, etc.) should NOT be external calls
- Library calls (SafeMath.add, etc.) should NOT be external calls
- Struct field access should NOT be external calls
- True external contract calls SHOULD be external calls
"""
solc_path = solc_binary_path("0.8.0")
standard_json = SolcStandardJson()
standard_json.add_source_file(Path(TEST_DATA_DIR, "external_calls.sol").as_posix())
compilation = CryticCompile(standard_json, solc=solc_path)
slither = Slither(compilation)

# Test TestBuiltins contract - should have NO external calls
test_builtins = slither.get_contract_from_name("TestBuiltins")[0]
_assert_no_external_calls_in_summary(test_builtins, "TestBuiltins")

# Test TestLibraryCalls contract - should have NO external calls
test_library = slither.get_contract_from_name("TestLibraryCalls")[0]
_assert_no_external_calls_in_summary(test_library, "TestLibraryCalls")

# Test TestStructAccess contract - should have NO external calls
test_struct = slither.get_contract_from_name("TestStructAccess")[0]
_assert_no_external_calls_in_summary(test_struct, "TestStructAccess")

# Test TestExternalCalls contract - should have external calls
test_external = slither.get_contract_from_name("TestExternalCalls")[0]

call_external = test_external.get_function_from_signature("callExternal()")
ext_calls = _get_external_calls_from_summary(call_external)
assert len(ext_calls) == 1, "TestExternalCalls.callExternal should have 1 external call"

call_with_arg = test_external.get_function_from_signature("callExternalWithArg(uint256)")
ext_calls = _get_external_calls_from_summary(call_with_arg)
assert len(ext_calls) == 1, "TestExternalCalls.callExternalWithArg should have 1 external call"

call_multiple = test_external.get_function_from_signature("callMultipleExternal(uint256)")
ext_calls = _get_external_calls_from_summary(call_multiple)
assert (
len(ext_calls) == 2
), "TestExternalCalls.callMultipleExternal should have 2 external calls"

# Test TestMixed contract - mixedOperations should have exactly 1 external call
test_mixed = slither.get_contract_from_name("TestMixed")[0]
mixed_ops = test_mixed.get_function_from_signature("mixedOperations(uint256,uint256)")
ext_calls = _get_external_calls_from_summary(mixed_ops)
assert len(ext_calls) == 1, (
f"TestMixed.mixedOperations should have 1 external call, "
f"but found {len(ext_calls)}: {ext_calls}"
)