Merge branch 'dev' into dev-ssa

pull/87/head
Josselin 6 years ago
commit 9c2a8c1da3
  1. 51
      scripts/tests_generate_expected_json_4.sh
  2. 16
      scripts/tests_generate_expected_json_5.sh
  3. 1
      scripts/travis_test_4.sh
  4. 3
      scripts/travis_test_5.sh
  5. 83
      slither/core/declarations/contract.py
  6. 12
      slither/core/declarations/function.py
  7. 138
      slither/detectors/functions/external_function.py
  8. 50
      slither/slithir/convert.py
  9. 5
      slither/slithir/operations/return_operation.py
  10. 6
      slither/solc_parsing/declarations/function.py
  11. 39
      slither/solc_parsing/expressions/expression_parsing.py
  12. 19
      slither/solc_parsing/slitherSolc.py
  13. 1
      tests/expected_json/external_function_2.external-function.json
  14. 2
      tests/external_function.sol
  15. 55
      tests/external_function_2.sol
  16. 2
      tests/external_function_import.sol

@ -14,28 +14,29 @@ generate_expected_json(){
}
generate_expected_json tests/uninitialized.sol "uninitialized-state"
generate_expected_json tests/backdoor.sol "backdoor"
generate_expected_json tests/backdoor.sol "suicidal"
generate_expected_json tests/pragma.0.4.24.sol "pragma"
generate_expected_json tests/old_solc.sol.json "solc-version"
generate_expected_json tests/reentrancy.sol "reentrancy"
generate_expected_json tests/uninitialized_storage_pointer.sol "uninitialized-storage"
generate_expected_json tests/tx_origin.sol "tx-origin"
generate_expected_json tests/unused_state.sol "unused-state"
generate_expected_json tests/locked_ether.sol "locked-ether"
generate_expected_json tests/arbitrary_send.sol "arbitrary-send"
generate_expected_json tests/inline_assembly_contract.sol "assembly"
generate_expected_json tests/inline_assembly_library.sol "assembly"
generate_expected_json tests/low_level_calls.sol "low-level-calls"
generate_expected_json tests/const_state_variables.sol "constable-states"
generate_expected_json tests/external_function.sol "external-function"
generate_expected_json tests/naming_convention.sol "naming-convention"
generate_expected_json tests/uninitialized_local_variable.sol "uninitialized-local"
generate_expected_json tests/controlled_delegatecall.sol "controlled-delegatecall"
generate_expected_json tests/constant.sol "constant-function"
generate_expected_json tests/unused_return.sol "unused-return"
generate_expected_json tests/shadowing_state_variable.sol "shadowing-state"
generate_expected_json tests/shadowing_abstract.sol "shadowing-abstract"
generate_expected_json tests/timestamp.sol "timestamp"
generate_expected_json tests/multiple_calls_in_loop.sol "calls-loop"
#generate_expected_json tests/uninitialized.sol "uninitialized-state"
#generate_expected_json tests/backdoor.sol "backdoor"
#generate_expected_json tests/backdoor.sol "suicidal"
#generate_expected_json tests/pragma.0.4.24.sol "pragma"
#generate_expected_json tests/old_solc.sol.json "solc-version"
#generate_expected_json tests/reentrancy.sol "reentrancy"
#generate_expected_json tests/uninitialized_storage_pointer.sol "uninitialized-storage"
#generate_expected_json tests/tx_origin.sol "tx-origin"
#generate_expected_json tests/unused_state.sol "unused-state"
#generate_expected_json tests/locked_ether.sol "locked-ether"
#generate_expected_json tests/arbitrary_send.sol "arbitrary-send"
#generate_expected_json tests/inline_assembly_contract.sol "assembly"
#generate_expected_json tests/inline_assembly_library.sol "assembly"
#generate_expected_json tests/low_level_calls.sol "low-level-calls"
#generate_expected_json tests/const_state_variables.sol "constable-states"
#generate_expected_json tests/external_function.sol "external-function"
generate_expected_json tests/external_function_2.sol "external-function"
#generate_expected_json tests/naming_convention.sol "naming-convention"
#generate_expected_json tests/uninitialized_local_variable.sol "uninitialized-local"
#generate_expected_json tests/controlled_delegatecall.sol "controlled-delegatecall"
#generate_expected_json tests/constant.sol "constant-function"
#generate_expected_json tests/unused_return.sol "unused-return"
#generate_expected_json tests/shadowing_state_variable.sol "shadowing-state"
#generate_expected_json tests/shadowing_abstract.sol "shadowing-abstract"
#generate_expected_json tests/timestamp.sol "timestamp"
#generate_expected_json tests/multiple_calls_in_loop.sol "calls-loop"

@ -14,16 +14,16 @@ generate_expected_json(){
}
generate_expected_json tests/uninitialized-0.5.1.sol "uninitialized-state"
#generate_expected_json tests/uninitialized-0.5.1.sol "uninitialized-state"
#generate_expected_json tests/backdoor.sol "backdoor"
#generate_expected_json tests/backdoor.sol "suicidal"
#generate_expected_json tests/pragma.0.4.24.sol "pragma"
#generate_expected_json tests/old_solc.sol.json "solc-version"
generate_expected_json tests/reentrancy-0.5.1.sol "reentrancy"
#generate_expected_json tests/reentrancy-0.5.1.sol "reentrancy"
#generate_expected_json tests/uninitialized_storage_pointer.sol "uninitialized-storage"
generate_expected_json tests/tx_origin-0.5.1.sol "tx-origin"
generate_expected_json tests/locked_ether-0.5.1.sol "locked-ether"
generate_expected_json tests/arbitrary_send-0.5.1.sol "arbitrary-send"
generate_expected_json tests/inline_assembly_contract-0.5.1.sol "assembly"
generate_expected_json tests/inline_assembly_library-0.5.1.sol "assembly"
generate_expected_json tests/constant-0.5.1.sol "constant-function"
#generate_expected_json tests/tx_origin-0.5.1.sol "tx-origin"
#generate_expected_json tests/locked_ether-0.5.1.sol "locked-ether"
#generate_expected_json tests/arbitrary_send-0.5.1.sol "arbitrary-send"
#generate_expected_json tests/inline_assembly_contract-0.5.1.sol "assembly"
#generate_expected_json tests/inline_assembly_library-0.5.1.sol "assembly"
#generate_expected_json tests/constant-0.5.1.sol "constant-function"

@ -81,6 +81,7 @@ test_slither tests/inline_assembly_library.sol "assembly"
test_slither tests/low_level_calls.sol "low-level-calls"
test_slither tests/const_state_variables.sol "constable-states"
test_slither tests/external_function.sol "external-function"
test_slither tests/external_function_2.sol "external-function"
test_slither tests/naming_convention.sol "naming-convention"
#test_slither tests/complex_func.sol "complex-function"
test_slither tests/controlled_delegatecall.sol "controlled-delegatecall"

@ -79,8 +79,9 @@ test_slither tests/inline_assembly_library-0.5.1.sol "assembly"
test_slither tests/low_level_calls.sol "low-level-calls"
test_slither tests/const_state_variables.sol "constable-states"
test_slither tests/external_function.sol "external-function"
test_slither tests/external_function_2.sol "external-function"
test_slither tests/naming_convention.sol "naming-convention"
#test_slither tests/complex_func.sol "complex-function"
##test_slither tests/complex_func.sol "complex-function"
test_slither tests/controlled_delegatecall.sol "controlled-delegatecall"
test_slither tests/constant-0.5.1.sol "constant-function"
test_slither tests/unused_return.sol "unused-return"

@ -70,6 +70,14 @@ class Contract(ChildSlither, SourceMapping):
def setInheritance(self, inheritance):
self._inheritance = inheritance
@property
def derived_contracts(self):
'''
list(Contract): Return the list of contracts derived from self
'''
candidates = self.slither.contracts
return [c for c in candidates if self in c.inheritance]
@property
def structures(self):
'''
@ -87,12 +95,6 @@ class Contract(ChildSlither, SourceMapping):
def enums_as_dict(self):
return self._enums
@property
def modifiers(self):
'''
list(Modifier): List of the modifiers
'''
return list(self._modifiers.values())
def modifiers_as_dict(self):
return self._modifiers
@ -115,6 +117,75 @@ class Contract(ChildSlither, SourceMapping):
'''
return [f for f in self.functions if f.contract != self]
@property
def functions_not_inherited(self):
'''
list(Function): List of the functions defined within the contract (not inherited)
'''
return [f for f in self.functions if f.contract == self]
@property
def functions_entry_points(self):
'''
list(Functions): List of public and external functions
'''
return [f for f in self.functions if f.visibility in ['public', 'external']]
@property
def modifiers(self):
'''
list(Modifier): List of the modifiers
'''
return list(self._modifiers.values())
@property
def modifiers_inherited(self):
'''
list(Modifier): List of the inherited modifiers
'''
return [m for m in self.modifiers if m.contract != self]
@property
def modifiers_not_inherited(self):
'''
list(Modifier): List of the modifiers defined within the contract (not inherited)
'''
return [m for m in self.modifiers if m.contract == self]
@property
def functions_and_modifiers(self):
'''
list(Function|Modifier): List of the functions and modifiers
'''
return self.functions + self.modifiers
@property
def functions_and_modifiers_inherited(self):
'''
list(Function|Modifier): List of the inherited functions and modifiers
'''
return self.functions_inherited + self.modifiers_inherited
@property
def functions_and_modifiers_not_inherited(self):
'''
list(Function|Modifier): List of the functions and modifiers defined within the contract (not inherited)
'''
return self.functions_not_inherited + self.modifiers_not_inherited
def get_functions_overridden_by(self, function):
'''
Return the list of functions overriden by the function
Args:
(core.Function)
Returns:
list(core.Function)
'''
candidates = [c.functions_not_inherited for c in self.inheritance]
candidates = [candidate for sublist in candidates for candidate in sublist]
return [f for f in candidates if f.full_name == function.full_name]
@property
def all_functions_called(self):
'''

@ -327,6 +327,18 @@ class Function(ChildContract, SourceMapping):
name, parameters, _ = self.signature
return name+'('+','.join(parameters)+')'
@property
def functions_shadowed(self):
'''
Return the list of functions shadowed
Returns:
list(core.Function)
'''
candidates = [c.functions_not_inherited for c in self.contract.inheritance]
candidates = [candidate for sublist in candidates for candidate in sublist]
return [f for f in candidates if f.full_name == self.full_name]
@property
def slither(self):

@ -27,7 +27,10 @@ class ExternalFunction(AbstractDetector):
(list): List of all InternallCall, SolidityCall
"""
result = []
# Obtain all functions reachable by this contract.
for func in contract.all_functions_called:
# Loop through all nodes in the function, add all calls to a list.
for node in func.nodes:
for ir in node.irs:
if isinstance(ir, (InternalCall, SolidityCall)):
@ -36,6 +39,12 @@ class ExternalFunction(AbstractDetector):
@staticmethod
def _contains_internal_dynamic_call(contract):
"""
Checks if a contract contains a dynamic call either in a direct definition, or through inheritance.
Returns:
(boolean): True if this contract contains a dynamic call (including through inheritance).
"""
for func in contract.all_functions_called:
for node in func.nodes:
for ir in node.irs:
@ -43,31 +52,126 @@ class ExternalFunction(AbstractDetector):
return True
return False
@staticmethod
def get_base_most_function(function):
"""
Obtains the base function definition for the provided function. This could be used to obtain the original
definition of a function, if the provided function is an override.
Returns:
(function): Returns the base-most function of a provided function. (The original definition).
"""
# Loop through the list of inherited contracts and this contract, to find the first function instance which
# matches this function's signature. Note here that `inheritance` is in order from most basic to most extended.
for contract in function.contract.inheritance + [function.contract]:
# Loop through the functions not inherited (explicitly defined in this contract).
for f in contract.functions_not_inherited:
# If it matches names, this is the base most function.
if f.full_name == function.full_name:
return f
# Somehow we couldn't resolve it, which shouldn't happen, as the provided function should be found if we could
# not find some any more basic.
raise Exception("Could not resolve the base-most function for the provided function.")
@staticmethod
def get_all_function_definitions(base_most_function):
"""
Obtains all function definitions given a base-most function. This includes the provided function, plus any
overrides of that function.
Returns:
(list): Returns any the provided function and any overriding functions defined for it.
"""
# We assume the provided function is the base-most function, so we check all derived contracts
# for a redefinition
return [base_most_function] + [function for derived_contract in base_most_function.contract.derived_contracts
for function in derived_contract.functions
if function.full_name == base_most_function.full_name]
def detect(self):
results = []
public_function_calls = []
all_info = ''
for contract in self.slither.contracts_derived:
# Create a set to track contracts with dynamic calls. All contracts with dynamic calls could potentially be
# calling functions internally, and thus we can't assume any function in such contracts isn't called by them.
dynamic_call_contracts = set()
# Create a completed functions set to skip over functions already processed (any functions which are the base
# of, or override hierarchically are processed together).
completed_functions = set()
# First we build our set of all contracts with dynamic calls
for contract in self.contracts:
if self._contains_internal_dynamic_call(contract):
dynamic_call_contracts.add(contract)
# Loop through all not-inherited contracts.
for contract in self.slither.contracts_derived:
# Filter false-positives: Immediately filter this contract if it's in blacklist
if contract in dynamic_call_contracts:
continue
func_list = self.detect_functions_called(contract)
public_function_calls.extend(func_list)
for func in [f for f in contract.functions if f.visibility == 'public' and\
not f in public_function_calls and\
not f.is_constructor]:
txt = "{}.{} ({}) should be declared external\n"
info = txt.format(func.contract.name,
func.name,
func.source_mapping_str)
all_info += info
json = self.generate_json_result(info)
self.add_function_to_json(func, json)
results.append(json)
# Next we'll want to loop through all functions defined directly in this contract.
for function in contract.functions_not_inherited:
# If the function is a constructor, or is public, we skip it.
if function.is_constructor or function.visibility != "public":
continue
# Optimization: If this function has already been processed, we stop.
if function in completed_functions:
continue
# Get the base-most function to know our origin of this function.
base_most_function = self.get_base_most_function(function)
# Get all possible contracts which can call this function (or an override).
all_possible_sources = [base_most_function.contract] + base_most_function.contract.derived_contracts
# Get all function signatures (overloaded and not), mark as completed and we process them now.
# Note: We mark all function definitions as the same, as they must all share visibility to override.
all_function_definitions = set(self.get_all_function_definitions(base_most_function))
completed_functions = completed_functions.union(all_function_definitions)
# Filter false-positives: Determine if any of these sources have dynamic calls, if so, flag all of these
# function definitions, and then flag all functions in all contracts that make dynamic calls.
sources_with_dynamic_calls = set(all_possible_sources) & dynamic_call_contracts
if sources_with_dynamic_calls:
functions_in_dynamic_call_sources = set([f for dyn_contract in sources_with_dynamic_calls
for f in dyn_contract if not f.is_constructor])
completed_functions = completed_functions.union(functions_in_dynamic_call_sources)
continue
# Detect all functions called in each source, if any match our current signature, we skip
# otherwise, this is a candidate (in all sources) to be changed visibility for.
is_called = False
for possible_source in all_possible_sources:
functions_called = self.detect_functions_called(possible_source)
if set(functions_called) & all_function_definitions:
is_called = True
break
# If any of this function's definitions are called, we skip it.
if is_called:
continue
# Loop for each function definition, and recommend it be declared external.
for function_definition in all_function_definitions:
txt = "{}.{} ({}) should be declared external\n"
info = txt.format(function_definition.contract.name,
function_definition.name,
function_definition.source_mapping_str)
all_info += info
json = self.generate_json_result(info)
self.add_function_to_json(function_definition, json)
results.append(json)
if all_info != '':
self.log(all_info)
return results

@ -316,10 +316,19 @@ def get_type(t):
return str(t)
def get_sig(ir):
'''
Return a list of potential signature
It is a list, as Constant variables can be converted to int256
Args:
ir (slithIR.operation)
Returns:
list(str)
'''
sig = '{}({})'
name = ir.function_name
args = []
# list of list of arguments
argss = [[]]
for arg in ir.arguments:
if isinstance(arg, (list,)):
type_arg = '{}[{}]'.format(get_type(arg[0].type), len(arg))
@ -327,14 +336,31 @@ def get_sig(ir):
type_arg = arg.signature_str
else:
type_arg = get_type(arg.type)
args.append(type_arg)
return sig.format(name, ','.join(args))
if isinstance(arg, Constant) and arg.type == ElementaryType('uint256'):
# If it is a constant
# We dupplicate the existing list
# And we add uint256 and int256 cases
# There is no potential collision, as the compiler
# Prevent it with a
# "not unique after argument-dependent loopkup" issue
argss_new = [list(args) for args in argss]
for args in argss:
args.append(str(ElementaryType('uint256')))
for args in argss_new:
args.append(str(ElementaryType('int256')))
argss = argss + argss_new
else:
for args in argss:
args.append(type_arg)
return [sig.format(name, ','.join(args)) for args in argss]
def convert_type_library_call(ir, lib_contract):
sig = get_sig(ir)
func = lib_contract.get_function_from_signature(sig)
if not func:
func = lib_contract.get_state_variable_from_name(ir.function_name)
sigs = get_sig(ir)
func = None
for sig in sigs:
func = lib_contract.get_function_from_signature(sig)
if not func:
func = lib_contract.get_state_variable_from_name(ir.function_name)
# In case of multiple binding to the same type
if not func:
# specific lookup when the compiler does implicit conversion
@ -363,10 +389,12 @@ def convert_type_library_call(ir, lib_contract):
return ir
def convert_type_of_high_level_call(ir, contract):
sig = get_sig(ir)
func = contract.get_function_from_signature(sig)
if not func:
func = contract.get_state_variable_from_name(ir.function_name)
func = None
sigs = get_sig(ir)
for sig in sigs:
func = contract.get_function_from_signature(sig)
if not func:
func = contract.get_state_variable_from_name(ir.function_name)
if not func:
# specific lookup when the compiler does implicit conversion
# for example

@ -18,6 +18,11 @@ class Return(Operation):
else:
values = [values]
else:
# Remove None
# Prior Solidity 0.5
# return (0,)
# was valid for returns(uint)
values = [v for v in values if not v is None]
self._valid_value(values)
super(Return, self).__init__()
self._values = values

@ -196,9 +196,9 @@ class FunctionSolc(Function):
if loop_expression:
node_LoopExpression = self._parse_statement(loop_expression, node_body)
link_nodes(node_LoopExpression, node_startLoop)
link_nodes(node_LoopExpression, node_condition)
else:
link_nodes(node_body, node_startLoop)
link_nodes(node_body, node_condition)
if not condition:
if not loop_expression:
@ -291,7 +291,7 @@ class FunctionSolc(Function):
if not hasCondition and not hasLoopExpression:
link_nodes(node, node_endLoop)
link_nodes(node_LoopExpression, node_startLoop)
link_nodes(node_LoopExpression, node_condition)
return node_endLoop

@ -237,6 +237,35 @@ def filter_name(value):
value = value[:idx+1]
return value
def convert_subdenomination(value, sub):
if sub is None:
return value
# to allow 0.1 ether conversion
value = float(value)
if sub == 'wei':
return int(value)
if sub == 'szabo':
return int(value * int(1e12))
if sub == 'finney':
return int(value * int(1e15))
if sub == 'ether':
return int(value * int(1e18))
if sub == 'seconds':
return int(value)
if sub == 'minutes':
return int(value * 60)
if sub == 'hours':
return int(value * 60 * 60)
if sub == 'days':
return int(value * 60 * 60 * 24)
if sub == 'weeks':
return int(value * 60 * 60 * 24 * 7)
if sub == 'years':
return int(value * 60 * 60 * 24 * 7 * 365)
logger.error('Subdemoniation not found {}'.format(sub))
return int(value)
def parse_expression(expression, caller_context):
"""
@ -380,11 +409,17 @@ def parse_expression(expression, caller_context):
if is_compact_ast:
value = expression['value']
if not value and value != "":
if value:
if 'subdenomination' in expression and expression['subdenomination']:
value = str(convert_subdenomination(value, expression['subdenomination']))
elif not value and value != "":
value = '0x'+expression['hexValue']
else:
value = expression['attributes']['value']
if value is None:
if value:
if 'subdenomination' in expression['attributes'] and expression['attributes']['subdenomination']:
value = str(convert_subdenomination(value, expression['attributes']['subdenomination']))
elif value is None:
# for literal declared as hex
# see https://solidity.readthedocs.io/en/v0.4.25/types.html?highlight=hex#hexadecimal-literals
assert 'hexvalue' in expression['attributes']

@ -11,6 +11,8 @@ from slither.core.declarations.pragma_directive import Pragma
from slither.core.declarations.import_directive import Import
from slither.analyses.data_depencency.data_depency import compute_dependency
from slither.utils.colors import red
class SlitherSolc(Slither):
def __init__(self, filename):
@ -158,11 +160,18 @@ class SlitherSolc(Slither):
for contract in self._contractsNotParsed:
# remove the first elem in linearizedBaseContracts as it is the contract itself
fathers = []
for i in contract.linearizedBaseContracts[1:]:
if i in contract.remapping:
fathers.append(self.get_contract_from_name(contract.remapping[i]))
else:
fathers.append(self._contracts_by_id[i])
try:
for i in contract.linearizedBaseContracts[1:]:
if i in contract.remapping:
fathers.append(self.get_contract_from_name(contract.remapping[i]))
else:
fathers.append(self._contracts_by_id[i])
except KeyError:
logger.error(red('A contract was not find, it is likely that your codebase contains muliple contracts with the same name'))
logger.error(red('Truffle does not handle this case during compilation'))
logger.error(red('Please read https://github.com/trailofbits/slither/wiki#keyerror-or-nonetype-error'))
logger.error(red('And update your code to remove the dupplicate'))
exit(-1)
contract.setInheritance(fathers)
contracts_to_be_analyzed = self.contracts

@ -1,6 +1,6 @@
//pragma solidity ^0.4.24;
import "./external_function_test_2.sol";
import "./external_function_import.sol";
contract ContractWithFunctionCalledSuper is ContractWithFunctionCalled {
function callWithSuper() public {

@ -0,0 +1,55 @@
// This tests against false-positives. This test should output no recommendations from the external-function detector.
contract ContractWithBaseFunctionCalled {
function getsCalledByBase() public;
function callsOverrideMe() external {
getsCalledByBase();
}
}
contract DerivingContractWithBaseCalled is ContractWithBaseFunctionCalled {
function getsCalledByBase() public {
// This should not be recommended to be marked external because it is called by the base class.
}
}
// All the contracts below should not recommend changing to external since inherited contracts have dynamic calls.
contract ContractWithDynamicCall {
function() returns(uint) ptr;
function test1() public returns(uint){
return 1;
}
function test2() public returns(uint){
return 2;
}
function setTest1() external{
ptr = test1;
}
function setTest2() external{
ptr = test2;
}
function exec() external returns(uint){
return ptr();
}
}
contract DerivesFromDynamicCall is ContractWithDynamicCall{
function getsCalledDynamically() public returns (uint){
// This should not be recommended because it is called dynamically.
return 3;
}
function setTest3() public {
// This should not be recommended because we inherit from a contract that calls dynamically, and we cannot be
// sure it did not somehow call this function.
ptr = getsCalledDynamically;
}
}

@ -1,4 +1,4 @@
//pragma solidity ^0.4.24;
// This file is imported by external_function.sol
contract ContractWithFunctionCalled {
function funcCalled() external {
Loading…
Cancel
Save