Merge branch 'dev' into dev-fix-external-function-detector

pull/257/head
Josselin 6 years ago
commit b79846a12c
  1. 32
      scripts/tests_generate_expected_json_5.sh
  2. 10
      slither/core/cfg/node.py
  3. 8
      slither/core/declarations/contract.py
  4. 10
      slither/core/source_mapping/source_mapping.py
  5. 4
      slither/printers/summary/variable_order.py
  6. 10
      slither/slithir/convert.py
  7. 2
      slither/solc_parsing/declarations/contract.py
  8. 19
      slither/solc_parsing/declarations/function.py
  9. 10
      slither/utils/expression_manipulations.py
  10. 15
      tests/expected_json/deprecated_calls.deprecated-standards.json
  11. 2
      tests/expected_json/deprecated_calls.deprecated-standards.txt
  12. 36
      tests/expected_json/incorrect_equality.incorrect-equality.json
  13. 14
      tests/expected_json/reentrancy.reentrancy-eth.json
  14. 2
      tests/expected_json/reentrancy.reentrancy-eth.txt
  15. 14
      tests/expected_json/tx_origin-0.5.1.tx-origin.json
  16. 2
      tests/expected_json/tx_origin-0.5.1.tx-origin.txt
  17. 14
      tests/expected_json/tx_origin.tx-origin.json
  18. 2
      tests/expected_json/tx_origin.tx-origin.txt
  19. 22
      tests/source_mapping.sol

@ -20,20 +20,20 @@ generate_expected_json(){
sed "s|$CURRENT_PATH|$TRAVIS_PATH|g" "$output_filename_txt" -i
}
generate_expected_json tests/solc_version_incorrect_05.ast.json "solc-version"
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/old_solc.sol.json "solc-version"
generate_expected_json tests/reentrancy-0.5.1.sol "reentrancy-eth"
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/incorrect_equality.sol "incorrect-equality"
generate_expected_json tests/too_many_digits.sol "too-many-digits"
generate_expected_json tests/unchecked_lowlevel-0.5.1.sol "unchecked-lowlevel"
generate_expected_json tests/unchecked_send-0.5.1.sol "unchecked-send"
#generate_expected_json tests/solc_version_incorrect_05.ast.json "solc-version"
#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/old_solc.sol.json "solc-version"
#generate_expected_json tests/reentrancy-0.5.1.sol "reentrancy-eth"
#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/incorrect_equality.sol "incorrect-equality"
#generate_expected_json tests/too_many_digits.sol "too-many-digits"
#generate_expected_json tests/unchecked_lowlevel-0.5.1.sol "unchecked-lowlevel"
#generate_expected_json tests/unchecked_send-0.5.1.sol "unchecked-send"

@ -20,6 +20,7 @@ from slither.slithir.operations import (Balance, HighLevelCall, Index,
from slither.slithir.variables import (Constant, LocalIRVariable,
ReferenceVariable, StateIRVariable,
TemporaryVariable, TupleVariable)
from slither.all_exceptions import SlitherException
logger = logging.getLogger("Node")
@ -44,9 +45,9 @@ class NodeType:
# Merging nodes
# Can have phi IR operation
ENDIF = 0x50
STARTLOOP = 0x51
ENDLOOP = 0x52
ENDIF = 0x50 # ENDIF node source mapping points to the if/else body
STARTLOOP = 0x51 # STARTLOOP node source mapping points to the entire loop body
ENDLOOP = 0x52 # ENDLOOP node source mapping points to the entire loop body
# Below the nodes have no expression
# But are used to expression CFG structure
@ -715,7 +716,10 @@ class Node(SourceMapping, ChildFunction):
elif ir.destination == SolidityVariable('this'):
self._high_level_calls.append((self.function.contract, ir.function))
else:
try:
self._high_level_calls.append((ir.destination.type.type, ir.function))
except AttributeError:
raise SlitherException(f'Function not found on {ir}. Please try compiling with a recent Solidity version.')
elif isinstance(ir, LibraryCall):
assert isinstance(ir.destination, Contract)
self._high_level_calls.append((ir.destination, ir.function))

@ -33,6 +33,7 @@ class Contract(ChildSlither, SourceMapping):
self._structures = {}
self._events = {}
self._variables = {}
self._variables_ordered = [] # contain also shadowed variables
self._modifiers = {}
self._functions = {}
@ -196,6 +197,13 @@ class Contract(ChildSlither, SourceMapping):
'''
return list(self._variables.values())
@property
def state_variables_ordered(self):
'''
list(StateVariable): List of the state variables by order of declaration. Contains also shadowed variables
'''
return list(self._variables_ordered)
@property
def state_variables_inherited(self):
'''

@ -20,6 +20,7 @@ class SourceMapping(Context):
Not done in an efficient way
"""
source_code = source_code.encode('utf-8')
total_length = len(source_code)
source_code = source_code.splitlines(True)
counter = 0
@ -29,17 +30,18 @@ class SourceMapping(Context):
ending_column = None
while counter < total_length:
# Determine the length of the line, and advance the line number
lineLength = len(source_code[i])
line_content = source_code[i]
line_length = len(line_content)
i = i + 1
# Determine our column numbers.
if starting_column is None and counter + lineLength > start:
if starting_column is None and counter + line_length > start:
starting_column = (start - counter) + 1
if starting_column is not None and ending_column is None and counter + lineLength > start + length:
if starting_column is not None and ending_column is None and counter + line_length > start + length:
ending_column = ((start + length) - counter) + 1
# Advance the current position counter, and determine line numbers.
counter += lineLength
counter += line_length
if counter > start:
lines.append(i)

@ -23,9 +23,9 @@ class VariableOrder(AbstractPrinter):
for contract in self.slither.contracts_derived:
txt += '\n{}:\n'.format(contract.name)
table = PrettyTable(['Name', 'Type'])
for variable in contract.state_variables:
for variable in contract.state_variables_ordered:
if not variable.is_constant:
table.add_row([variable.name, str(variable.type)])
table.add_row([variable.canonical_name, str(variable.type)])
txt += str(table) + '\n'
self.info(txt)

@ -781,21 +781,21 @@ def convert_type_of_high_and_internal_level_call(ir, contract):
sigs = get_canonical_names(ir, ir.function_name, ir.contract_name)
for sig in sigs:
func = contract.get_function_from_canonical_name(sig)
if not func:
func = contract.get_state_variable_from_name(ir.function_name)
if func:
# stop to explore if func is found (prevent dupplicate issue)
break
if not func:
func = contract.get_state_variable_from_name(ir.function_name)
else:
assert isinstance(ir, HighLevelCall)
sigs = get_sig(ir, ir.function_name)
for sig in sigs:
func = contract.get_function_from_canonical_name(sig)
if not func:
func = contract.get_state_variable_from_name(ir.function_name)
func = contract.get_function_from_signature(sig)
if func:
# stop to explore if func is found (prevent dupplicate issue)
break
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

@ -222,6 +222,7 @@ class ContractSolc04(Contract):
def parse_state_variables(self):
for father in self.inheritance_reverse:
self._variables.update(father.variables_as_dict())
self._variables_ordered += father.state_variables_ordered
for varNotParsed in self._variablesNotParsed:
var = StateVariableSolc(varNotParsed)
@ -229,6 +230,7 @@ class ContractSolc04(Contract):
var.set_contract(self)
self._variables[var.name] = var
self._variables_ordered.append(var)
def _parse_modifier(self, modifier):

@ -256,7 +256,7 @@ class FunctionSolc(Function):
condition = ifStatement['condition']
# Note: check if the expression could be directly
# parsed here
condition_node = self._new_node(NodeType.IF, ifStatement['src'])
condition_node = self._new_node(NodeType.IF, condition['src'])
condition_node.add_unparsed_expression(condition)
link_nodes(node, condition_node)
trueStatement = self._parse_statement(ifStatement['trueBody'], condition_node)
@ -267,7 +267,7 @@ class FunctionSolc(Function):
condition = children[0]
# Note: check if the expression could be directly
# parsed here
condition_node = self._new_node(NodeType.IF, ifStatement['src'])
condition_node = self._new_node(NodeType.IF, condition['src'])
condition_node.add_unparsed_expression(condition)
link_nodes(node, condition_node)
trueStatement = self._parse_statement(children[1], condition_node)
@ -287,14 +287,15 @@ class FunctionSolc(Function):
# WhileStatement = 'while' '(' Expression ')' Statement
node_startWhile = self._new_node(NodeType.STARTLOOP, whileStatement['src'])
node_condition = self._new_node(NodeType.IFLOOP, whileStatement['src'])
if self.is_compact_ast:
node_condition = self._new_node(NodeType.IFLOOP, whileStatement['condition']['src'])
node_condition.add_unparsed_expression(whileStatement['condition'])
statement = self._parse_statement(whileStatement['body'], node_condition)
else:
children = whileStatement[self.get_children('children')]
expression = children[0]
node_condition = self._new_node(NodeType.IFLOOP, expression['src'])
node_condition.add_unparsed_expression(expression)
statement = self._parse_statement(children[1], node_condition)
@ -323,7 +324,7 @@ class FunctionSolc(Function):
link_nodes(node, node_startLoop)
if condition:
node_condition = self._new_node(NodeType.IFLOOP, statement['src'])
node_condition = self._new_node(NodeType.IFLOOP, condition['src'])
node_condition.add_unparsed_expression(condition)
link_nodes(node_startLoop, node_condition)
link_nodes(node_condition, node_endLoop)
@ -417,9 +418,9 @@ class FunctionSolc(Function):
if candidate[self.get_key()] not in ['VariableDefinitionStatement',
'VariableDeclarationStatement',
'ExpressionStatement']:
node_condition = self._new_node(NodeType.IFLOOP, statement['src'])
#expression = parse_expression(candidate, self)
expression = candidate
node_condition = self._new_node(NodeType.IFLOOP, expression['src'])
#expression = parse_expression(candidate, self)
node_condition.add_unparsed_expression(expression)
link_nodes(node_startLoop, node_condition)
link_nodes(node_condition, node_endLoop)
@ -448,15 +449,16 @@ class FunctionSolc(Function):
def _parse_dowhile(self, doWhilestatement, node):
node_startDoWhile = self._new_node(NodeType.STARTLOOP, doWhilestatement['src'])
node_condition = self._new_node(NodeType.IFLOOP, doWhilestatement['src'])
if self.is_compact_ast:
node_condition = self._new_node(NodeType.IFLOOP, doWhilestatement['condition']['src'])
node_condition.add_unparsed_expression(doWhilestatement['condition'])
statement = self._parse_statement(doWhilestatement['body'], node_condition)
else:
children = doWhilestatement[self.get_children('children')]
# same order in the AST as while
expression = children[0]
node_condition = self._new_node(NodeType.IFLOOP, expression['src'])
node_condition.add_unparsed_expression(expression)
statement = self._parse_statement(children[1], node_condition)
@ -964,7 +966,8 @@ class FunctionSolc(Function):
if has_cond.result():
st = SplitTernaryExpression(node.expression)
condition = st.condition
assert condition
if not condition:
raise ParsingError(f'Incorrect ternary conversion {node.expression} {node.source_mapping_str}')
true_expr = st.true_expression
false_expr = st.false_expression
self.split_ternary_node(node, condition, true_expr, false_expr)

@ -7,17 +7,15 @@ from slither.core.expressions.assignment_operation import AssignmentOperation
from slither.core.expressions.binary_operation import BinaryOperation
from slither.core.expressions.call_expression import CallExpression
from slither.core.expressions.conditional_expression import ConditionalExpression
from slither.core.expressions.elementary_type_name_expression import ElementaryTypeNameExpression
from slither.core.expressions.identifier import Identifier
from slither.core.expressions.index_access import IndexAccess
from slither.core.expressions.literal import Literal
from slither.core.expressions.member_access import MemberAccess
from slither.core.expressions.new_array import NewArray
from slither.core.expressions.new_contract import NewContract
from slither.core.expressions.new_elementary_type import NewElementaryType
from slither.core.expressions.tuple_expression import TupleExpression
from slither.core.expressions.type_conversion import TypeConversion
from slither.core.expressions.unary_operation import UnaryOperation
from slither.all_exceptions import SlitherException
def f_expressions(e, x):
e._expressions.append(x)
@ -35,8 +33,6 @@ class SplitTernaryExpression(object):
def __init__(self, expression):
# print(expression)
if isinstance(expression, ConditionalExpression):
self.true_expression = copy.copy(expression.then_expression)
self.false_expression = copy.copy(expression.else_expression)
@ -64,7 +60,7 @@ class SplitTernaryExpression(object):
return
if isinstance(expression, ConditionalExpression):
raise Exception('Nested ternary operator not handled')
raise SlitherException('Nested ternary operator not handled')
if isinstance(expression, (Literal, Identifier, IndexAccess, NewArray, NewContract)):
return None
@ -117,5 +113,5 @@ class SplitTernaryExpression(object):
false_expression.expression)
else:
raise Exception('Ternary operation not handled {}({})'.format(expression, type(expression)))
raise SlitherException('Ternary operation not handled {}({})'.format(expression, type(expression)))

@ -77,26 +77,23 @@
"check": "deprecated-standards",
"impact": "Informational",
"confidence": "High",
"description": "Deprecated standard detected @ tests/deprecated_calls.sol#7-10:\n\t- Usage of \"msg.gas\" should be replaced with \"gasleft()\"\n",
"description": "Deprecated standard detected @ tests/deprecated_calls.sol#7:\n\t- Usage of \"msg.gas\" should be replaced with \"gasleft()\"\n",
"elements": [
{
"type": "node",
"name": "msg.gas == msg.value",
"source_mapping": {
"start": 258,
"length": 107,
"start": 261,
"length": 20,
"filename_used": "/home/travis/build/crytic/slither/tests/deprecated_calls.sol",
"filename_relative": "tests/deprecated_calls.sol",
"filename_absolute": "/home/travis/build/crytic/slither/tests/deprecated_calls.sol",
"filename_short": "tests/deprecated_calls.sol",
"lines": [
7,
8,
9,
10
7
],
"starting_column": 9,
"ending_column": 10
"starting_column": 12,
"ending_column": 32
},
"type_specific_fields": {
"parent": {

@ -1,7 +1,7 @@
INFO:Detectors:
Deprecated standard detected @ tests/deprecated_calls.sol#2:
- Usage of "block.blockhash()" should be replaced with "blockhash()"
Deprecated standard detected @ tests/deprecated_calls.sol#7-10:
Deprecated standard detected @ tests/deprecated_calls.sol#7:
- Usage of "msg.gas" should be replaced with "gasleft()"
Deprecated standard detected @ tests/deprecated_calls.sol#9:
- Usage of "throw" should be replaced with "revert()"

@ -1259,19 +1259,17 @@
"type": "node",
"name": "balance == 10000000000000000000",
"source_mapping": {
"start": 1270,
"length": 80,
"start": 1274,
"length": 19,
"filename_used": "/home/travis/build/crytic/slither/tests/incorrect_equality.sol",
"filename_relative": "tests/incorrect_equality.sol",
"filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_equality.sol",
"filename_short": "tests/incorrect_equality.sol",
"lines": [
54,
55,
56
54
],
"starting_column": 9,
"ending_column": 10
"starting_column": 13,
"ending_column": 32
},
"type_specific_fields": {
"parent": {
@ -1506,19 +1504,17 @@
"type": "node",
"name": "10000000000000000000 == balance",
"source_mapping": {
"start": 1446,
"length": 80,
"start": 1450,
"length": 19,
"filename_used": "/home/travis/build/crytic/slither/tests/incorrect_equality.sol",
"filename_relative": "tests/incorrect_equality.sol",
"filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_equality.sol",
"filename_short": "tests/incorrect_equality.sol",
"lines": [
61,
62,
63
61
],
"starting_column": 9,
"ending_column": 10
"starting_column": 13,
"ending_column": 32
},
"type_specific_fields": {
"parent": {
@ -1753,19 +1749,17 @@
"type": "node",
"name": "balance == 10000000000000000000",
"source_mapping": {
"start": 1631,
"length": 80,
"start": 1635,
"length": 19,
"filename_used": "/home/travis/build/crytic/slither/tests/incorrect_equality.sol",
"filename_relative": "tests/incorrect_equality.sol",
"filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_equality.sol",
"filename_short": "tests/incorrect_equality.sol",
"lines": [
68,
69,
70
68
],
"starting_column": 9,
"ending_column": 10
"starting_column": 13,
"ending_column": 32
},
"type_specific_fields": {
"parent": {

@ -7,7 +7,7 @@
"check": "reentrancy-eth",
"impact": "High",
"confidence": "Medium",
"description": "Reentrancy in Reentrancy.withdrawBalance() (tests/reentrancy.sol#14-21):\n\tExternal calls:\n\t- ! (msg.sender.call.value(userBalance[msg.sender])()) (tests/reentrancy.sol#17-19)\n\tState variables written after the call(s):\n\t- userBalance (tests/reentrancy.sol#20)\n",
"description": "Reentrancy in Reentrancy.withdrawBalance() (tests/reentrancy.sol#14-21):\n\tExternal calls:\n\t- ! (msg.sender.call.value(userBalance[msg.sender])()) (tests/reentrancy.sol#17)\n\tState variables written after the call(s):\n\t- userBalance (tests/reentrancy.sol#20)\n",
"elements": [
{
"type": "function",
@ -126,19 +126,17 @@
"type": "node",
"name": "! (msg.sender.call.value(userBalance[msg.sender])())",
"source_mapping": {
"start": 478,
"length": 92,
"start": 482,
"length": 53,
"filename_used": "/home/travis/build/crytic/slither/tests/reentrancy.sol",
"filename_relative": "tests/reentrancy.sol",
"filename_absolute": "/home/travis/build/crytic/slither/tests/reentrancy.sol",
"filename_short": "tests/reentrancy.sol",
"lines": [
17,
18,
19
17
],
"starting_column": 9,
"ending_column": 10
"starting_column": 13,
"ending_column": 66
},
"type_specific_fields": {
"parent": {

@ -1,7 +1,7 @@
INFO:Detectors:
Reentrancy in Reentrancy.withdrawBalance() (tests/reentrancy.sol#14-21):
External calls:
- ! (msg.sender.call.value(userBalance[msg.sender])()) (tests/reentrancy.sol#17-19)
- ! (msg.sender.call.value(userBalance[msg.sender])()) (tests/reentrancy.sol#17)
State variables written after the call(s):
- userBalance (tests/reentrancy.sol#20)
Reentrancy in Reentrancy.withdrawBalance_nested() (tests/reentrancy.sol#64-70):

@ -96,25 +96,23 @@
"check": "tx-origin",
"impact": "Medium",
"confidence": "Medium",
"description": "TxOrigin.bug2() uses tx.origin for authorization: \"tx.origin != owner\" (tests/tx_origin-0.5.1.sol#14-16)\n",
"description": "TxOrigin.bug2() uses tx.origin for authorization: \"tx.origin != owner\" (tests/tx_origin-0.5.1.sol#14)\n",
"elements": [
{
"type": "node",
"name": "tx.origin != owner",
"source_mapping": {
"start": 231,
"length": 57,
"start": 235,
"length": 18,
"filename_used": "/home/travis/build/crytic/slither/tests/tx_origin-0.5.1.sol",
"filename_relative": "tests/tx_origin-0.5.1.sol",
"filename_absolute": "/home/travis/build/crytic/slither/tests/tx_origin-0.5.1.sol",
"filename_short": "tests/tx_origin-0.5.1.sol",
"lines": [
14,
15,
16
14
],
"starting_column": 9,
"ending_column": 10
"starting_column": 13,
"ending_column": 31
},
"type_specific_fields": {
"parent": {

@ -1,5 +1,5 @@
INFO:Detectors:
TxOrigin.bug0() uses tx.origin for authorization: "require(bool)(tx.origin == owner)" (tests/tx_origin-0.5.1.sol#10)
TxOrigin.bug2() uses tx.origin for authorization: "tx.origin != owner" (tests/tx_origin-0.5.1.sol#14-16)
TxOrigin.bug2() uses tx.origin for authorization: "tx.origin != owner" (tests/tx_origin-0.5.1.sol#14)
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#dangerous-usage-of-txorigin
INFO:Slither:tests/tx_origin-0.5.1.sol analyzed (1 contracts), 2 result(s) found

@ -96,25 +96,23 @@
"check": "tx-origin",
"impact": "Medium",
"confidence": "Medium",
"description": "TxOrigin.bug2() uses tx.origin for authorization: \"tx.origin != owner\" (tests/tx_origin.sol#14-16)\n",
"description": "TxOrigin.bug2() uses tx.origin for authorization: \"tx.origin != owner\" (tests/tx_origin.sol#14)\n",
"elements": [
{
"type": "node",
"name": "tx.origin != owner",
"source_mapping": {
"start": 208,
"length": 57,
"start": 212,
"length": 18,
"filename_used": "/home/travis/build/crytic/slither/tests/tx_origin.sol",
"filename_relative": "tests/tx_origin.sol",
"filename_absolute": "/home/travis/build/crytic/slither/tests/tx_origin.sol",
"filename_short": "tests/tx_origin.sol",
"lines": [
14,
15,
16
14
],
"starting_column": 9,
"ending_column": 10
"starting_column": 13,
"ending_column": 31
},
"type_specific_fields": {
"parent": {

@ -1,5 +1,5 @@
INFO:Detectors:
TxOrigin.bug0() uses tx.origin for authorization: "require(bool)(tx.origin == owner)" (tests/tx_origin.sol#10)
TxOrigin.bug2() uses tx.origin for authorization: "tx.origin != owner" (tests/tx_origin.sol#14-16)
TxOrigin.bug2() uses tx.origin for authorization: "tx.origin != owner" (tests/tx_origin.sol#14)
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#dangerous-usage-of-txorigin
INFO:Slither:tests/tx_origin.sol analyzed (1 contracts), 2 result(s) found

@ -0,0 +1,22 @@
contract A{
// ëëëëëëëëëëëëë
address unused;
address unused2;
//
address unused3;
address unused4;
//
address used;
}
Loading…
Cancel
Save