diff --git a/README.md b/README.md index 97522379f..f7267339c 100644 --- a/README.md +++ b/README.md @@ -60,7 +60,7 @@ Num | Detector | What it Detects | Impact | Confidence 11 | `low-level-calls` | Low level calls | Informational | High 12 | `naming-convention` | Conformance to Solidity naming conventions | Informational | High 13 | `pragma` | If different pragma directives are used | Informational | High -14 | `solc-version` | If an old version of Solidity used (<0.4.23) | Informational | High +14 | `solc-version` | Old versions of Solidity (< 0.4.23) | Informational | High 15 | `unused-state` | Unused state variables | Informational | High [Contact us](https://www.trailofbits.com/contact/) to get access to additional detectors. diff --git a/scripts/travis_test.sh b/scripts/travis_test.sh index 82cf650b2..918342a97 100755 --- a/scripts/travis_test.sh +++ b/scripts/travis_test.sh @@ -30,7 +30,7 @@ test_slither tests/complex_func.sol "complex-function" 3 test_slither tests/inline_assembly_contract.sol "assembly" 1 test_slither tests/inline_assembly_library.sol "assembly" 2 test_slither tests/low_level_calls.sol "low-level-calls" 1 -test_slither tests/const_state_variables.sol "const-candidates-state" 2 +test_slither tests/const_state_variables.sol "constable-states" 2 test_slither tests/external_function.sol "external-function" 4 test_slither tests/naming_convention.sol "naming-convention" 12 diff --git a/slither/analyses/taint/common.py b/slither/analyses/taint/common.py index 484f5f2d2..944f65bab 100644 --- a/slither/analyses/taint/common.py +++ b/slither/analyses/taint/common.py @@ -1,4 +1,4 @@ -from slither.slithir.operations import (Index, Member) +from slither.slithir.operations import (Index, Member, Length, Balance) def iterate_over_irs(irs, transfer_func, taints): refs = {} @@ -6,6 +6,9 @@ def iterate_over_irs(irs, transfer_func, taints): if isinstance(ir, (Index, Member)): refs[ir.lvalue] = ir.variable_left + if isinstance(ir, (Length, Balance)): + refs[ir.lvalue] = ir.value + if isinstance(ir, Index): read = [ir.variable_left] else: diff --git a/slither/analyses/write/are_variables_written.py b/slither/analyses/write/are_variables_written.py index f76ca4874..d644e3ac2 100644 --- a/slither/analyses/write/are_variables_written.py +++ b/slither/analyses/write/are_variables_written.py @@ -4,7 +4,7 @@ from slither.core.cfg.node import NodeType from slither.core.declarations import SolidityFunction from slither.slithir.operations import (Index, Member, OperationWithLValue, - SolidityCall) + SolidityCall, Length, Balance) from slither.slithir.variables import ReferenceVariable @@ -27,6 +27,8 @@ def _visit(node, visited, variables_written, variables_to_write): continue if isinstance(ir, (Index, Member)): refs[ir.lvalue] = ir.variable_left + if isinstance(ir, (Length, Balance)): + refs[ir.lvalue] = ir.value variables_written = variables_written + [ir.lvalue] lvalue = ir.lvalue diff --git a/slither/core/cfg/node.py b/slither/core/cfg/node.py index 03aec416f..dbbbb522d 100644 --- a/slither/core/cfg/node.py +++ b/slither/core/cfg/node.py @@ -3,27 +3,24 @@ """ import logging +from slither.core.children.child_function import ChildFunction +from slither.core.declarations import Contract +from slither.core.declarations.solidity_variables import (SolidityFunction, + SolidityVariable) from slither.core.source_mapping.source_mapping import SourceMapping -from slither.core.variables.variable import Variable from slither.core.variables.state_variable import StateVariable - +from slither.core.variables.variable import Variable +from slither.slithir.convert import convert_expression +from slither.slithir.operations import (Balance, HighLevelCall, Index, + InternalCall, Length, LibraryCall, + LowLevelCall, Member, + OperationWithLValue, SolidityCall) +from slither.slithir.variables import (Constant, ReferenceVariable, + TemporaryVariable, TupleVariable) from slither.visitors.expression.expression_printer import ExpressionPrinter from slither.visitors.expression.read_var import ReadVar from slither.visitors.expression.write_var import WriteVar -from slither.core.children.child_function import ChildFunction - -from slither.core.declarations.solidity_variables import SolidityVariable, SolidityFunction - -from slither.slithir.convert import convert_expression - -from slither.slithir.operations import OperationWithLValue, Index, Member, LowLevelCall, SolidityCall, HighLevelCall, InternalCall, LibraryCall - - -from slither.slithir.variables import Constant, ReferenceVariable, TemporaryVariable, TupleVariable - -from slither.core.declarations import Contract - logger = logging.getLogger("Node") class NodeType: @@ -370,11 +367,10 @@ class Node(SourceMapping, ChildFunction): def is_slithir_var(var): return isinstance(var, (Constant, ReferenceVariable, TemporaryVariable, TupleVariable)) - for ir in self.irs: self._vars_read += [v for v in ir.read if not is_slithir_var(v)] if isinstance(ir, OperationWithLValue): - if isinstance(ir, (Index, Member)): + if isinstance(ir, (Index, Member, Length, Balance)): continue # Don't consider Member and Index operations -> ReferenceVariable var = ir.lvalue # If its a reference, we loop until finding the origin @@ -413,5 +409,3 @@ class Node(SourceMapping, ChildFunction): self._high_level_calls = list(set(self._high_level_calls)) self._low_level_calls = list(set(self._low_level_calls)) - - diff --git a/slither/core/declarations/solidity_variables.py b/slither/core/declarations/solidity_variables.py index 777b5ce83..36f0795f1 100644 --- a/slither/core/declarations/solidity_variables.py +++ b/slither/core/declarations/solidity_variables.py @@ -22,8 +22,7 @@ SOLIDITY_VARIABLES_COMPOSED = {"block.coinbase":"address", "msg.sig":"bytes4", "msg.value":"uint256", "tx.gasprice":"uint256", - "tx.origin":"address", - "this.balance":"uint256"} + "tx.origin":"address"} SOLIDITY_FUNCTIONS = {"gasleft()":['uint256'], diff --git a/slither/core/variables/variable.py b/slither/core/variables/variable.py index 64373b33a..0ab0a99ca 100644 --- a/slither/core/variables/variable.py +++ b/slither/core/variables/variable.py @@ -3,7 +3,8 @@ """ from slither.core.source_mapping.source_mapping import SourceMapping - +from slither.core.solidity_types.type import Type +from slither.core.solidity_types.elementary_type import ElementaryType class Variable(SourceMapping): @@ -73,6 +74,9 @@ class Variable(SourceMapping): return self._visibility def set_type(self, t): + if isinstance(t, str): + t = ElementaryType(t) + assert isinstance(t, (Type, list)) or t is None self._type = t def __str__(self): diff --git a/slither/detectors/attributes/old_solc.py b/slither/detectors/attributes/old_solc.py index f322deb1c..a14a1df50 100644 --- a/slither/detectors/attributes/old_solc.py +++ b/slither/detectors/attributes/old_solc.py @@ -12,7 +12,7 @@ class OldSolc(AbstractDetector): """ ARGUMENT = 'solc-version' - HELP = 'If an old version of Solidity used (<0.4.23)' + HELP = 'Old versions of Solidity (< 0.4.23)' IMPACT = DetectorClassification.INFORMATIONAL CONFIDENCE = DetectorClassification.HIGH diff --git a/slither/detectors/functions/arbitrary_send.py b/slither/detectors/functions/arbitrary_send.py index ae115ede3..920351d41 100644 --- a/slither/detectors/functions/arbitrary_send.py +++ b/slither/detectors/functions/arbitrary_send.py @@ -27,7 +27,7 @@ class ArbitrarySend(AbstractDetector): """ ARGUMENT = 'arbitrary-send' - HELP = 'Functions that send ether to an arbitrary destination' + HELP = 'Functions that send ether to arbitrary destinations' IMPACT = DetectorClassification.HIGH CONFIDENCE = DetectorClassification.MEDIUM diff --git a/slither/detectors/functions/external_function.py b/slither/detectors/functions/external_function.py index 9cb19909b..99ffe40fe 100644 --- a/slither/detectors/functions/external_function.py +++ b/slither/detectors/functions/external_function.py @@ -12,7 +12,7 @@ class ExternalFunction(AbstractDetector): """ ARGUMENT = 'external-function' - HELP = 'Detect public function that could be declared as external' + HELP = 'Public function that could be declared as external' IMPACT = DetectorClassification.INFORMATIONAL CONFIDENCE = DetectorClassification.HIGH diff --git a/slither/detectors/variables/uninitialized_state_variables.py b/slither/detectors/variables/uninitialized_state_variables.py index 427e3ad4a..86f54b048 100644 --- a/slither/detectors/variables/uninitialized_state_variables.py +++ b/slither/detectors/variables/uninitialized_state_variables.py @@ -38,10 +38,11 @@ class UninitializedStateVarsDetection(AbstractDetector): if isinstance(ir, LibraryCall) \ or isinstance(ir, InternalCall): idx = 0 - for param in ir.function.parameters: - if param.location == 'storage': - ret.append(ir.arguments[idx]) - idx = idx+1 + if ir.function: + for param in ir.function.parameters: + if param.location == 'storage': + ret.append(ir.arguments[idx]) + idx = idx+1 return ret diff --git a/slither/slithir/convert.py b/slither/slithir/convert.py index ecffadfdd..3748f4403 100644 --- a/slither/slithir/convert.py +++ b/slither/slithir/convert.py @@ -14,7 +14,7 @@ from slither.slithir.operations import (Assignment, Binary, BinaryType, Call, NewStructure, OperationWithLValue, Push, Return, Send, SolidityCall, Transfer, TypeConversion, Unary, - Unpack) + Unpack, Length, Balance) from slither.slithir.tmp_operations.argument import Argument, ArgumentType from slither.slithir.tmp_operations.tmp_call import TmpCall from slither.slithir.tmp_operations.tmp_new_array import TmpNewArray @@ -189,7 +189,10 @@ def convert_to_low_level(ir): call = SolidityFunction('abi.{}()'.format(ir.function_name)) new_ir = SolidityCall(call, ir.nbr_arguments, ir.lvalue, ir.type_call) new_ir.arguments = ir.arguments - new_ir.lvalue.set_type(call.return_type) + if isinstance(call.return_type, list) and len(call.return_type) == 1: + new_ir.lvalue.set_type(call.return_type[0]) + else: + new_ir.lvalue.set_type(call.return_type) return new_ir elif ir.function_name in ['call', 'delegatecall', 'callcode']: new_ir = LowLevelCall(ir.destination, @@ -254,9 +257,10 @@ def convert_to_library(ir, node, using_for): contract = node.function.contract t = ir.destination.type - new_ir = look_for_library(contract, ir, node, using_for, t) - if new_ir: - return new_ir + if t in using_for: + new_ir = look_for_library(contract, ir, node, using_for, t) + if new_ir: + return new_ir if '*' in using_for: new_ir = look_for_library(contract, ir, node, using_for, '*') @@ -353,7 +357,12 @@ def convert_type_of_high_level_call(ir, contract): return_type = return_type[0] else: # otherwise its a variable (getter) - return_type = func.type + if isinstance(func.type, MappingType): + return_type = func.type.type_to + elif isinstance(func.type, ArrayType): + return_type = func.type.type + else: + return_type = func.type if return_type: ir.lvalue.set_type(return_type) else: @@ -365,6 +374,7 @@ def propagate_types(ir, node): # propagate the type using_for = node.function.contract.using_for if isinstance(ir, OperationWithLValue): + # Force assignment in case of missing previous correct type if not ir.lvalue.type: if isinstance(ir, Assignment): ir.lvalue.set_type(ir.rvalue.type) @@ -386,7 +396,7 @@ def propagate_types(ir, node): return # convert library - if t in using_for: + if t in using_for or '*' in using_for: new_ir = convert_to_library(ir, node, using_for) if new_ir: return new_ir @@ -427,7 +437,7 @@ def propagate_types(ir, node): if return_type: if len(return_type) == 1: ir.lvalue.set_type(return_type[0]) - else: + elif len(return_type)>1: ir.lvalue.set_type(return_type) else: ir.lvalue = None @@ -446,6 +456,11 @@ def propagate_types(ir, node): # This should not happen assert False elif isinstance(ir, Member): + # TODO we should convert the reference to a temporary if the member is a length or a balance + if ir.variable_right == 'length' and isinstance(ir.variable_left.type, (ElementaryType, ArrayType)): + return Length(ir.variable_left, ir.lvalue) + if ir.variable_right == 'balance' and isinstance(ir.variable_left.type, ElementaryType): + return Balance(ir.variable_left, ir.lvalue) left = ir.variable_left if isinstance(left, (Variable, SolidityVariable)): t = ir.variable_left.type @@ -483,7 +498,7 @@ def propagate_types(ir, node): return_type = ir.function.return_type if len(return_type) == 1: ir.lvalue.set_type(return_type[0]) - else: + elif len(return_type)>1: ir.lvalue.set_type(return_type) elif isinstance(ir, TypeConversion): ir.lvalue.set_type(ir.type) diff --git a/slither/slithir/operations/__init__.py b/slither/slithir/operations/__init__.py index 13200f273..ed59260e2 100644 --- a/slither/slithir/operations/__init__.py +++ b/slither/slithir/operations/__init__.py @@ -26,3 +26,5 @@ from .transfer import Transfer from .type_conversion import TypeConversion from .unary import Unary, UnaryType from .unpack import Unpack +from .length import Length +from .balance import Balance diff --git a/slither/slithir/operations/assignment.py b/slither/slithir/operations/assignment.py index ad4b9d7fe..0440a1697 100644 --- a/slither/slithir/operations/assignment.py +++ b/slither/slithir/operations/assignment.py @@ -36,4 +36,4 @@ class Assignment(OperationWithLValue): return self._rvalue def __str__(self): - return '{} := {}'.format(self.lvalue, self.rvalue) + return '{}({}) := {}({})'.format(self.lvalue, self.lvalue.type, self.rvalue, self.rvalue.type) diff --git a/slither/slithir/operations/balance.py b/slither/slithir/operations/balance.py new file mode 100644 index 000000000..3dd6560fd --- /dev/null +++ b/slither/slithir/operations/balance.py @@ -0,0 +1,26 @@ +import logging +from slither.slithir.operations.lvalue import OperationWithLValue +from slither.core.declarations import Function +from slither.core.variables.variable import Variable +from slither.slithir.utils.utils import is_valid_lvalue, is_valid_rvalue +from slither.core.solidity_types.elementary_type import ElementaryType + +class Balance(OperationWithLValue): + + def __init__(self, value, lvalue): + assert is_valid_rvalue(value) + assert is_valid_lvalue(lvalue) + self._value = value + self._lvalue = lvalue + lvalue.set_type(ElementaryType('uint256')) + + @property + def read(self): + return [self._value] + + @property + def value(self): + return self._value + + def __str__(self): + return "{} -> BALANCE {}".format(self.lvalue, self.value) diff --git a/slither/slithir/operations/binary.py b/slither/slithir/operations/binary.py index 1485f01b0..733b6596b 100644 --- a/slither/slithir/operations/binary.py +++ b/slither/slithir/operations/binary.py @@ -2,6 +2,7 @@ import logging from slither.slithir.operations.lvalue import OperationWithLValue from slither.core.variables.variable import Variable from slither.slithir.utils.utils import is_valid_lvalue, is_valid_rvalue +from slither.core.solidity_types import ElementaryType logger = logging.getLogger("BinaryOperationIR") @@ -135,7 +136,7 @@ class Binary(OperationWithLValue): self._type = operation_type self._lvalue = result if BinaryType.return_bool(operation_type): - result.set_type('bool') + result.set_type(ElementaryType('bool')) else: result.set_type(left_variable.type) diff --git a/slither/slithir/operations/event_call.py b/slither/slithir/operations/event_call.py index acffe5970..0c7214d60 100644 --- a/slither/slithir/operations/event_call.py +++ b/slither/slithir/operations/event_call.py @@ -14,7 +14,15 @@ class EventCall(Call): @property def read(self): - return list(self.arguments) + def unroll(l): + ret = [] + for x in l: + if not isinstance(x, list): + ret += [x] + else: + ret += unroll(x) + return ret + return unroll(self.arguments) def __str__(self): args = [str(a) for a in self.arguments] diff --git a/slither/slithir/operations/high_level_call.py b/slither/slithir/operations/high_level_call.py index 1681bb263..fd54a1113 100644 --- a/slither/slithir/operations/high_level_call.py +++ b/slither/slithir/operations/high_level_call.py @@ -58,7 +58,16 @@ class HighLevelCall(Call, OperationWithLValue): @property def read(self): - all_read = [self.destination, self.call_gas, self.call_value] + self.arguments + # if array inside the parameters + def unroll(l): + ret = [] + for x in l: + if not isinstance(x, list): + ret += [x] + else: + ret += unroll(x) + return ret + all_read = [self.destination, self.call_gas, self.call_value] + unroll(self.arguments) # remove None return [x for x in all_read if x] diff --git a/slither/slithir/operations/init_array.py b/slither/slithir/operations/init_array.py index bbd939c65..e85299dba 100644 --- a/slither/slithir/operations/init_array.py +++ b/slither/slithir/operations/init_array.py @@ -23,7 +23,16 @@ class InitArray(OperationWithLValue): @property def read(self): - return list(self.init_values) + # if array inside the init values + def unroll(l): + ret = [] + for x in l: + if not isinstance(x, list): + ret += [x] + else: + ret += unroll(x) + return ret + return unroll(self.init_values) @property def init_values(self): diff --git a/slither/slithir/operations/internal_call.py b/slither/slithir/operations/internal_call.py index e8bbb3db7..606059ca6 100644 --- a/slither/slithir/operations/internal_call.py +++ b/slither/slithir/operations/internal_call.py @@ -16,7 +16,16 @@ class InternalCall(Call, OperationWithLValue): @property def read(self): - return list(self.arguments) + # if array inside the parameters + def unroll(l): + ret = [] + for x in l: + if not isinstance(x, list): + ret += [x] + else: + ret += unroll(x) + return ret + return list(unroll(self.arguments)) @property def function(self): diff --git a/slither/slithir/operations/internal_dynamic_call.py b/slither/slithir/operations/internal_dynamic_call.py index e39fc9d65..5e40d3e57 100644 --- a/slither/slithir/operations/internal_dynamic_call.py +++ b/slither/slithir/operations/internal_dynamic_call.py @@ -19,7 +19,17 @@ class InternalDynamicCall(Call, OperationWithLValue): @property def read(self): - return list(self.arguments) + [self.function] + # if array inside the parameters + def unroll(l): + ret = [] + for x in l: + if not isinstance(x, list): + ret += [x] + else: + ret += unroll(x) + return ret + + return unroll(self.arguments) + [self.function] @property def function(self): diff --git a/slither/slithir/operations/length.py b/slither/slithir/operations/length.py new file mode 100644 index 000000000..10b16ff80 --- /dev/null +++ b/slither/slithir/operations/length.py @@ -0,0 +1,26 @@ +import logging +from slither.slithir.operations.lvalue import OperationWithLValue +from slither.core.declarations import Function +from slither.core.variables.variable import Variable +from slither.slithir.utils.utils import is_valid_lvalue, is_valid_rvalue +from slither.core.solidity_types.elementary_type import ElementaryType + +class Length(OperationWithLValue): + + def __init__(self, value, lvalue): + assert is_valid_rvalue(value) + assert is_valid_lvalue(lvalue) + self._value = value + self._lvalue = lvalue + lvalue.set_type(ElementaryType('uint256')) + + @property + def read(self): + return [self._value] + + @property + def value(self): + return self._value + + def __str__(self): + return "{} -> LENGTH {}".format(self.lvalue, self.value) diff --git a/slither/slithir/operations/new_array.py b/slither/slithir/operations/new_array.py index 91383b00c..213a5c487 100644 --- a/slither/slithir/operations/new_array.py +++ b/slither/slithir/operations/new_array.py @@ -18,7 +18,16 @@ class NewArray(Call, OperationWithLValue): @property def read(self): - return list(self.arguments) + # if array inside the parameters + def unroll(l): + ret = [] + for x in l: + if not isinstance(x, list): + ret += [x] + else: + ret += unroll(x) + return ret + return unroll(self.arguments) @property def depth(self): diff --git a/slither/slithir/operations/new_contract.py b/slither/slithir/operations/new_contract.py index 2299351ef..0646a70db 100644 --- a/slither/slithir/operations/new_contract.py +++ b/slither/slithir/operations/new_contract.py @@ -39,7 +39,17 @@ class NewContract(Call, OperationWithLValue): @property def read(self): - return list(self.arguments) + # if array inside the parameters + def unroll(l): + ret = [] + for x in l: + if not isinstance(x, list): + ret += [x] + else: + ret += unroll(x) + return ret + return unroll(self.arguments) + def __str__(self): value = '' if self.call_value: diff --git a/slither/slithir/operations/new_structure.py b/slither/slithir/operations/new_structure.py index 6c41aaaac..302ac7ff6 100644 --- a/slither/slithir/operations/new_structure.py +++ b/slither/slithir/operations/new_structure.py @@ -17,7 +17,16 @@ class NewStructure(Call, OperationWithLValue): @property def read(self): - return list(self.arguments) + # if array inside the parameters + def unroll(l): + ret = [] + for x in l: + if not isinstance(x, list): + ret += [x] + else: + ret += unroll(x) + return ret + return unroll(self.arguments) @property def structure(self): diff --git a/slither/slithir/variables/constant.py b/slither/slithir/variables/constant.py index 296dd7f25..f16d686a6 100644 --- a/slither/slithir/variables/constant.py +++ b/slither/slithir/variables/constant.py @@ -1,4 +1,5 @@ from slither.core.variables.variable import Variable +from slither.core.solidity_types.elementary_type import ElementaryType class Constant(Variable): @@ -6,10 +7,10 @@ class Constant(Variable): super(Constant, self).__init__() assert isinstance(val, str) if val.isdigit(): - self._type = 'uint256' + self._type = ElementaryType('uint256') self._val = int(val) else: - self._type = 'string' + self._type = ElementaryType('string') self._val = val @property diff --git a/slither/slithir/variables/tuple.py b/slither/slithir/variables/tuple.py index 7abe34a25..1b9e73353 100644 --- a/slither/slithir/variables/tuple.py +++ b/slither/slithir/variables/tuple.py @@ -1,6 +1,7 @@ from slither.core.variables.variable import Variable +from slither.core.solidity_types.type import Type class TupleVariable(Variable): COUNTER = 0 diff --git a/slither/solc_parsing/expressions/expression_parsing.py b/slither/solc_parsing/expressions/expression_parsing.py index 688482177..02d095cc8 100644 --- a/slither/solc_parsing/expressions/expression_parsing.py +++ b/slither/solc_parsing/expressions/expression_parsing.py @@ -152,11 +152,11 @@ def parse_call(expression, caller_context): type_info = children[0] expression_to_parse = children[1] assert type_info['name'] in ['ElementaryTypenameExpression', - 'ElementaryTypeNameExpression', - 'Identifier', - 'TupleExpression', - 'IndexAccess', - 'MemberAccess'] + 'ElementaryTypeNameExpression', + 'Identifier', + 'TupleExpression', + 'IndexAccess', + 'MemberAccess'] expression = parse_expression(expression_to_parse, caller_context) t = TypeConversion(expression, type_call)