Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

resolve available definitions from import by reference ID #2403

Merged
merged 6 commits into from
Apr 7, 2024
Merged
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
4 changes: 2 additions & 2 deletions slither/core/declarations/contract.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

from slither.core.cfg.scope import Scope
from slither.core.source_mapping.source_mapping import SourceMapping
from slither.utils.using_for import USING_FOR, _merge_using_for
from slither.utils.using_for import USING_FOR, merge_using_for
from slither.core.declarations.function import Function, FunctionType, FunctionLanguage
from slither.utils.erc import (
ERC20_signatures,
Expand Down Expand Up @@ -342,7 +342,7 @@ def using_for_complete(self) -> USING_FOR:
result = self.using_for
top_level_using_for = self.file_scope.using_for_directives
for uftl in top_level_using_for:
result = _merge_using_for(result, uftl.using_for)
result = merge_using_for(result, uftl.using_for)
self._using_for_complete = result
return self._using_for_complete

Expand Down
3 changes: 1 addition & 2 deletions slither/core/declarations/function.py
Original file line number Diff line number Diff line change
Expand Up @@ -355,8 +355,7 @@ def is_checked(self) -> bool:
@property
def id(self) -> Optional[str]:
"""
Return the ID of the funciton. For Solidity with compact-AST the ID is the reference ID
For other, the ID is None
Return the reference ID of the function, if available.

:return:
:rtype:
Expand Down
5 changes: 4 additions & 1 deletion slither/core/declarations/function_contract.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,10 @@ def is_declared_by(self, contract: "Contract") -> bool:

@property
def file_scope(self) -> "FileScope":
return self.contract.file_scope
# This is the contract declarer's file scope because inherited functions have access
# to the file scope which their declared in. This scope may contain references not
# available in the child contract's scope. See inherited_function_scope.sol for an example.
return self.contract_declarer.file_scope

# endregion
###################################################################################
Expand Down
4 changes: 2 additions & 2 deletions slither/core/declarations/function_top_level.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

from slither.core.declarations import Function
from slither.core.declarations.top_level import TopLevel
from slither.utils.using_for import USING_FOR, _merge_using_for
from slither.utils.using_for import USING_FOR, merge_using_for

if TYPE_CHECKING:
from slither.core.compilation_unit import SlitherCompilationUnit
Expand All @@ -32,7 +32,7 @@ def using_for_complete(self) -> USING_FOR:
if self._using_for_complete is None:
result = {}
for uftl in self.file_scope.using_for_directives:
result = _merge_using_for(result, uftl.using_for)
result = merge_using_for(result, uftl.using_for)
self._using_for_complete = result
return self._using_for_complete

Expand Down
2 changes: 1 addition & 1 deletion slither/core/declarations/import_directive.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ class Import(SourceMapping):
def __init__(self, filename: Path, scope: "FileScope") -> None:
super().__init__()
self._filename: Path = filename
self._alias: Optional[str] = None
self.scope: "FileScope" = scope
self._alias: Optional[str] = None
# Map local name -> original name
self.renaming: Dict[str, str] = {}

Expand Down
5 changes: 2 additions & 3 deletions slither/core/declarations/using_for_top_level.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
from typing import TYPE_CHECKING, List, Dict, Union
from typing import TYPE_CHECKING

from slither.core.solidity_types.type import Type
from slither.core.declarations.top_level import TopLevel
from slither.utils.using_for import USING_FOR

Expand All @@ -11,7 +10,7 @@
class UsingForTopLevel(TopLevel):
def __init__(self, scope: "FileScope") -> None:
super().__init__()
self._using_for: Dict[Union[str, Type], List[Type]] = {}
self._using_for: USING_FOR = {}
self.file_scope: "FileScope" = scope

@property
Expand Down
47 changes: 17 additions & 30 deletions slither/core/scope/scope.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ class FileScope:
def __init__(self, filename: Filename) -> None:
self.filename = filename
self.accessible_scopes: List[FileScope] = []
self.exported_symbols: Set[int] = set()

self.contracts: Dict[str, Contract] = {}
# Custom error are a list instead of a dict
Expand Down Expand Up @@ -56,53 +57,39 @@ def __init__(self, filename: Filename) -> None:
# Name -> type alias
self.type_aliases: Dict[str, TypeAlias] = {}

def add_accesible_scopes(self) -> bool: # pylint: disable=too-many-branches
def add_accessible_scopes(self) -> bool: # pylint: disable=too-many-branches
"""
Add information from accessible scopes. Return true if new information was obtained

:return:
:rtype:
"""

learn_something = False

for new_scope in self.accessible_scopes:
if not _dict_contain(new_scope.contracts, self.contracts):
self.contracts.update(new_scope.contracts)
learn_something = True
if not new_scope.custom_errors.issubset(self.custom_errors):
self.custom_errors |= new_scope.custom_errors
learn_something = True
if not _dict_contain(new_scope.enums, self.enums):
self.enums.update(new_scope.enums)
learn_something = True
if not new_scope.events.issubset(self.events):
self.events |= new_scope.events
learn_something = True
if not new_scope.functions.issubset(self.functions):
self.functions |= new_scope.functions
learn_something = True
# To support using for directives on user defined types and user defined functions,
# we need to propagate the using for directives from the imported file to the importing file
# since it is not reflected in the "exportedSymbols" field of the AST.
if not new_scope.using_for_directives.issubset(self.using_for_directives):
self.using_for_directives |= new_scope.using_for_directives
learn_something = True
if not new_scope.imports.issubset(self.imports):
self.imports |= new_scope.imports
learn_something = True
if not new_scope.pragmas.issubset(self.pragmas):
self.pragmas |= new_scope.pragmas
if not _dict_contain(new_scope.type_aliases, self.type_aliases):
self.type_aliases.update(new_scope.type_aliases)
learn_something = True
if not _dict_contain(new_scope.structures, self.structures):
self.structures.update(new_scope.structures)
if not new_scope.functions.issubset(self.functions):
self.functions |= new_scope.functions
learn_something = True
if not _dict_contain(new_scope.variables, self.variables):
self.variables.update(new_scope.variables)

# To get around this bug for aliases https://github.com/ethereum/solidity/pull/11881,
# we propagate the exported_symbols from the imported file to the importing file
# See tests/e2e/solc_parsing/test_data/top-level-nested-import-0.7.1.sol
if not new_scope.exported_symbols.issubset(self.exported_symbols):
self.exported_symbols |= new_scope.exported_symbols
learn_something = True

# This is need to support aliasing when we do a late lookup using SolidityImportPlaceholder
if not _dict_contain(new_scope.renaming, self.renaming):
self.renaming.update(new_scope.renaming)
learn_something = True
if not _dict_contain(new_scope.type_aliases, self.type_aliases):
self.type_aliases.update(new_scope.type_aliases)
learn_something = True

return learn_something

Expand Down
71 changes: 60 additions & 11 deletions slither/slither.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
import logging
from typing import Union, List, ValuesView, Type, Dict, Optional
from typing import Union, List, Type, Dict, Optional

from crytic_compile import CryticCompile, InvalidCompilation

# pylint: disable= no-name-in-module
from slither.core.compilation_unit import SlitherCompilationUnit
from slither.core.scope.scope import FileScope
from slither.core.slither_core import SlitherCore
from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification
from slither.exceptions import SlitherError
Expand All @@ -27,32 +26,71 @@ def _check_common_things(
) -> None:

if not issubclass(cls, base_cls) or cls is base_cls:
raise Exception(
raise SlitherError(
f"You can't register {cls!r} as a {thing_name}. You need to pass a class that inherits from {base_cls.__name__}"
)

if any(type(obj) == cls for obj in instances_list): # pylint: disable=unidiomatic-typecheck
raise Exception(f"You can't register {cls!r} twice.")
raise SlitherError(f"You can't register {cls!r} twice.")


def _update_file_scopes(candidates: ValuesView[FileScope]):
def _update_file_scopes(
sol_parser: SlitherCompilationUnitSolc,
): # pylint: disable=too-many-branches
"""
Because solc's import allows cycle in the import
We iterate until we aren't adding new information to the scope

Since all definitions in a file are exported by default, including definitions from its (transitive) dependencies,
we can identify all top level items that could possibly be referenced within the file from its exportedSymbols.
It is not as straightforward for user defined types and functions as well as aliasing. See add_accessible_scopes for more details.
"""
candidates = sol_parser.compilation_unit.scopes.values()
learned_something = False
# Because solc's import allows cycle in the import graph, iterate until we aren't adding new information to the scope.
while True:
for candidate in candidates:
learned_something |= candidate.add_accesible_scopes()
learned_something |= candidate.add_accessible_scopes()
if not learned_something:
break
learned_something = False

for scope in candidates:
for refId in scope.exported_symbols:
if refId in sol_parser.contracts_by_id:
contract = sol_parser.contracts_by_id[refId]
scope.contracts[contract.name] = contract
elif refId in sol_parser.functions_by_id:
functions = sol_parser.functions_by_id[refId]
assert len(functions) == 1
scope.functions.add(functions[0])
elif refId in sol_parser.imports_by_id:
import_directive = sol_parser.imports_by_id[refId]
scope.imports.add(import_directive)
elif refId in sol_parser.top_level_variables_by_id:
top_level_variable = sol_parser.top_level_variables_by_id[refId]
scope.variables[top_level_variable.name] = top_level_variable
elif refId in sol_parser.top_level_events_by_id:
top_level_event = sol_parser.top_level_events_by_id[refId]
scope.events.add(top_level_event)
elif refId in sol_parser.top_level_structures_by_id:
top_level_struct = sol_parser.top_level_structures_by_id[refId]
scope.structures[top_level_struct.name] = top_level_struct
elif refId in sol_parser.top_level_type_aliases_by_id:
top_level_type_alias = sol_parser.top_level_type_aliases_by_id[refId]
scope.type_aliases[top_level_type_alias.name] = top_level_type_alias
elif refId in sol_parser.top_level_enums_by_id:
top_level_enum = sol_parser.top_level_enums_by_id[refId]
scope.enums[top_level_enum.name] = top_level_enum
elif refId in sol_parser.top_level_errors_by_id:
top_level_custom_error = sol_parser.top_level_errors_by_id[refId]
scope.custom_errors.add(top_level_custom_error)
else:
logger.error(
f"Failed to resolved name for reference id {refId} in {scope.filename.absolute}."
)


class Slither(
SlitherCore
): # pylint: disable=too-many-instance-attributes,too-many-locals,too-many-statements
): # pylint: disable=too-many-instance-attributes,too-many-locals,too-many-statements,too-many-branches
def __init__(self, target: Union[str, CryticCompile], **kwargs) -> None:
"""
Args:
Expand Down Expand Up @@ -118,7 +156,18 @@ def __init__(self, target: Union[str, CryticCompile], **kwargs) -> None:
sol_parser.parse_top_level_items(ast, path)
self.add_source_code(path)

_update_file_scopes(compilation_unit_slither.scopes.values())
for contract in sol_parser._underlying_contract_to_parser:
if contract.name.startswith("SlitherInternalTopLevelContract"):
raise SlitherError(
# region multi-line-string
"""Your codebase has a contract named 'SlitherInternalTopLevelContract'.
Please rename it, this name is reserved for Slither's internals"""
# endregion multi-line
)
sol_parser._contracts_by_id[contract.id] = contract
sol_parser._compilation_unit.contracts.append(contract)

_update_file_scopes(sol_parser)

if kwargs.get("generate_patches", False):
self.generate_patches = True
Expand Down
25 changes: 11 additions & 14 deletions slither/slithir/convert.py
Original file line number Diff line number Diff line change
Expand Up @@ -1502,7 +1502,6 @@ def convert_to_pop(ir: HighLevelCall, node: "Node") -> List[Operation]:


def look_for_library_or_top_level(
contract: Contract,
ir: HighLevelCall,
using_for,
t: Union[
Expand Down Expand Up @@ -1536,8 +1535,9 @@ def look_for_library_or_top_level(
lib_contract = None
if isinstance(destination, FunctionContract) and destination.contract.is_library:
lib_contract = destination.contract
elif not isinstance(destination, FunctionTopLevel):
lib_contract = contract.file_scope.get_contract_from_name(str(destination))
elif isinstance(destination, UserDefinedType) and isinstance(destination.type, Contract):
lib_contract = destination.type

if lib_contract:
lib_call = LibraryCall(
lib_contract,
Expand All @@ -1561,20 +1561,14 @@ def look_for_library_or_top_level(
def convert_to_library_or_top_level(
ir: HighLevelCall, node: "Node", using_for
) -> Optional[Union[LibraryCall, InternalCall,]]:
# We use contract_declarer, because Solidity resolve the library
# before resolving the inheritance.
# Though we could use .contract as libraries cannot be shadowed
contract = (
node.function.contract_declarer if isinstance(node.function, FunctionContract) else None
)
t = ir.destination.type
if t in using_for:
new_ir = look_for_library_or_top_level(contract, ir, using_for, t)
new_ir = look_for_library_or_top_level(ir, using_for, t)
if new_ir:
return new_ir

if "*" in using_for:
new_ir = look_for_library_or_top_level(contract, ir, using_for, "*")
new_ir = look_for_library_or_top_level(ir, using_for, "*")
if new_ir:
return new_ir

Expand All @@ -1585,7 +1579,7 @@ def convert_to_library_or_top_level(
and UserDefinedType(node.function.contract) in using_for
):
new_ir = look_for_library_or_top_level(
contract, ir, using_for, UserDefinedType(node.function.contract)
ir, using_for, UserDefinedType(node.function.contract)
)
if new_ir:
return new_ir
Expand Down Expand Up @@ -1740,7 +1734,10 @@ def convert_type_of_high_and_internal_level_call(
]

for import_statement in contract.file_scope.imports:
if import_statement.alias and import_statement.alias == ir.contract_name:
if (
import_statement.alias is not None
and import_statement.alias == ir.contract_name
):
imported_scope = contract.compilation_unit.get_scope(import_statement.filename)
candidates += [
f
Expand Down Expand Up @@ -1771,7 +1768,7 @@ def convert_type_of_high_and_internal_level_call(
if func is None and candidates:
func = _find_function_from_parameter(ir.arguments, candidates, False)

# lowlelvel lookup needs to be done at last step
# low level lookup needs to be done as last step
if not func:
if can_be_low_level(ir):
return convert_to_low_level(ir)
Expand Down
4 changes: 3 additions & 1 deletion slither/solc_parsing/declarations/contract.py
Original file line number Diff line number Diff line change
Expand Up @@ -585,7 +585,9 @@ def _analyze_params_elements( # pylint: disable=too-many-arguments,too-many-loc
element.is_shadowed = True
accessible_elements[element.full_name].shadows = True
except (VariableNotFound, KeyError) as e:
self.log_incorrect_parsing(f"Missing params {e}")
self.log_incorrect_parsing(
f"Missing params {e} {self._contract.source_mapping.to_detailed_str()}"
)
return all_elements

def analyze_constant_state_variables(self) -> None:
Expand Down
6 changes: 4 additions & 2 deletions slither/solc_parsing/declarations/function.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,12 @@ def __init__(
# Only present if compact AST
if self.is_compact_ast:
self._function.name = function_data["name"]
if "id" in function_data:
self._function.id = function_data["id"]
else:
self._function.name = function_data["attributes"][self.get_key()]

if "id" in function_data:
self._function.id = function_data["id"]

self._functionNotParsed = function_data
self._returnsNotParsed: List[dict] = []
self._params_was_analyzed = False
Expand Down
5 changes: 4 additions & 1 deletion slither/solc_parsing/expressions/find_variable.py
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,7 @@ def _find_variable_init(
else:
assert isinstance(underlying_function, FunctionContract)
scope = underlying_function.contract.file_scope

elif isinstance(caller_context, StructureTopLevelSolc):
direct_contracts = []
direct_functions_parser = []
Expand Down Expand Up @@ -505,4 +506,6 @@ def find_variable(
if ret:
return ret, False

raise VariableNotFound(f"Variable not found: {var_name} (context {contract})")
raise VariableNotFound(
f"Variable not found: {var_name} (context {contract} {contract.source_mapping.to_detailed_str()})"
)
Loading
Loading