From a97903836ca9adf1984ae441e54b55d57beb77e6 Mon Sep 17 00:00:00 2001 From: Josselin Date: Thu, 16 May 2019 19:49:40 +0100 Subject: [PATCH 1/6] Fix incorrect source mapping on if/while/for condition (close #245) --- scripts/tests_generate_expected_json_5.sh | 32 ++++++++--------- slither/core/cfg/node.py | 6 ++-- slither/solc_parsing/declarations/function.py | 16 +++++---- ...deprecated_calls.deprecated-standards.json | 15 ++++---- .../deprecated_calls.deprecated-standards.txt | 2 +- ...incorrect_equality.incorrect-equality.json | 36 ++++++++----------- .../reentrancy.reentrancy-eth.json | 14 ++++---- .../reentrancy.reentrancy-eth.txt | 2 +- .../tx_origin-0.5.1.tx-origin.json | 14 ++++---- .../tx_origin-0.5.1.tx-origin.txt | 2 +- tests/expected_json/tx_origin.tx-origin.json | 14 ++++---- tests/expected_json/tx_origin.tx-origin.txt | 2 +- 12 files changed, 71 insertions(+), 84 deletions(-) diff --git a/scripts/tests_generate_expected_json_5.sh b/scripts/tests_generate_expected_json_5.sh index 4a43fade4..fb9552437 100755 --- a/scripts/tests_generate_expected_json_5.sh +++ b/scripts/tests_generate_expected_json_5.sh @@ -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" diff --git a/slither/core/cfg/node.py b/slither/core/cfg/node.py index 6d2da977b..c74096c57 100644 --- a/slither/core/cfg/node.py +++ b/slither/core/cfg/node.py @@ -44,9 +44,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 diff --git a/slither/solc_parsing/declarations/function.py b/slither/solc_parsing/declarations/function.py index f66b3efd7..e86cbaac2 100644 --- a/slither/solc_parsing/declarations/function.py +++ b/slither/solc_parsing/declarations/function.py @@ -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) diff --git a/tests/expected_json/deprecated_calls.deprecated-standards.json b/tests/expected_json/deprecated_calls.deprecated-standards.json index 49736378d..e18879fa2 100644 --- a/tests/expected_json/deprecated_calls.deprecated-standards.json +++ b/tests/expected_json/deprecated_calls.deprecated-standards.json @@ -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": { diff --git a/tests/expected_json/deprecated_calls.deprecated-standards.txt b/tests/expected_json/deprecated_calls.deprecated-standards.txt index e0733a66e..7056ea4ce 100644 --- a/tests/expected_json/deprecated_calls.deprecated-standards.txt +++ b/tests/expected_json/deprecated_calls.deprecated-standards.txt @@ -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()" diff --git a/tests/expected_json/incorrect_equality.incorrect-equality.json b/tests/expected_json/incorrect_equality.incorrect-equality.json index 1337fbed5..eb890ff8a 100644 --- a/tests/expected_json/incorrect_equality.incorrect-equality.json +++ b/tests/expected_json/incorrect_equality.incorrect-equality.json @@ -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": { diff --git a/tests/expected_json/reentrancy.reentrancy-eth.json b/tests/expected_json/reentrancy.reentrancy-eth.json index 01a58ed4f..92c77b522 100644 --- a/tests/expected_json/reentrancy.reentrancy-eth.json +++ b/tests/expected_json/reentrancy.reentrancy-eth.json @@ -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": { diff --git a/tests/expected_json/reentrancy.reentrancy-eth.txt b/tests/expected_json/reentrancy.reentrancy-eth.txt index 243541a7c..e1160ca4c 100644 --- a/tests/expected_json/reentrancy.reentrancy-eth.txt +++ b/tests/expected_json/reentrancy.reentrancy-eth.txt @@ -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): diff --git a/tests/expected_json/tx_origin-0.5.1.tx-origin.json b/tests/expected_json/tx_origin-0.5.1.tx-origin.json index b68f18578..a0c117cb6 100644 --- a/tests/expected_json/tx_origin-0.5.1.tx-origin.json +++ b/tests/expected_json/tx_origin-0.5.1.tx-origin.json @@ -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": { diff --git a/tests/expected_json/tx_origin-0.5.1.tx-origin.txt b/tests/expected_json/tx_origin-0.5.1.tx-origin.txt index 5a12a42f8..24811f6e4 100644 --- a/tests/expected_json/tx_origin-0.5.1.tx-origin.txt +++ b/tests/expected_json/tx_origin-0.5.1.tx-origin.txt @@ -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 diff --git a/tests/expected_json/tx_origin.tx-origin.json b/tests/expected_json/tx_origin.tx-origin.json index 192d82792..0dc5b7d20 100644 --- a/tests/expected_json/tx_origin.tx-origin.json +++ b/tests/expected_json/tx_origin.tx-origin.json @@ -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": { diff --git a/tests/expected_json/tx_origin.tx-origin.txt b/tests/expected_json/tx_origin.tx-origin.txt index 62a9c9f85..ad8f9b78a 100644 --- a/tests/expected_json/tx_origin.tx-origin.txt +++ b/tests/expected_json/tx_origin.tx-origin.txt @@ -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 From 79a4a10304f41ae2804f60f49d0145bd92ddb40a Mon Sep 17 00:00:00 2001 From: David Pokora Date: Mon, 20 May 2019 04:53:38 -0400 Subject: [PATCH 2/6] Fixed a source mapping issue with utf8 characters where length != byte length --- slither/core/source_mapping/source_mapping.py | 1 + 1 file changed, 1 insertion(+) diff --git a/slither/core/source_mapping/source_mapping.py b/slither/core/source_mapping/source_mapping.py index 22dabca31..041f954ee 100644 --- a/slither/core/source_mapping/source_mapping.py +++ b/slither/core/source_mapping/source_mapping.py @@ -21,6 +21,7 @@ class SourceMapping(Context): Not done in an efficient way """ total_length = len(source_code) + source_code = source_code.encode('utf-8') source_code = source_code.splitlines(True) counter = 0 i = 0 From c1fc109e775243dcdd5c12185b4dffd65e97dfcf Mon Sep 17 00:00:00 2001 From: David Pokora Date: Mon, 20 May 2019 05:47:03 -0400 Subject: [PATCH 3/6] Added test for source mapping. Fixed total source length calculation --- slither/core/source_mapping/source_mapping.py | 11 +++++----- tests/source_mapping.sol | 22 +++++++++++++++++++ 2 files changed, 28 insertions(+), 5 deletions(-) create mode 100644 tests/source_mapping.sol diff --git a/slither/core/source_mapping/source_mapping.py b/slither/core/source_mapping/source_mapping.py index 041f954ee..422287f41 100644 --- a/slither/core/source_mapping/source_mapping.py +++ b/slither/core/source_mapping/source_mapping.py @@ -20,8 +20,8 @@ class SourceMapping(Context): Not done in an efficient way """ - total_length = len(source_code) source_code = source_code.encode('utf-8') + total_length = len(source_code) source_code = source_code.splitlines(True) counter = 0 i = 0 @@ -30,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) diff --git a/tests/source_mapping.sol b/tests/source_mapping.sol new file mode 100644 index 000000000..771f2daed --- /dev/null +++ b/tests/source_mapping.sol @@ -0,0 +1,22 @@ +contract A{ + + // ëëëëëëëëëëëëë这这这这这这这这这这 + + address unused; + + + address unused2; + + + // ই এই এই এইই এই এই এইই এই এই এই + + address unused3; + + + address unused4; + + // 这这这这这这这这这这这这这这这这这这这这这这这这这这这这这这这这这这这这这这这这这这这这 + address used; + + +} From 29e8a3a905d69afe3d3fdc299d3e250018f0686a Mon Sep 17 00:00:00 2001 From: Josselin Date: Tue, 21 May 2019 11:21:40 +0100 Subject: [PATCH 4/6] API add: contract.state_variables_ordered: keep variables declaration order + shadowed variables Update variable order printer to use state_variables_ordered Fix #250 --- slither/core/declarations/contract.py | 8 ++++++++ slither/printers/summary/variable_order.py | 4 ++-- slither/solc_parsing/declarations/contract.py | 2 ++ 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/slither/core/declarations/contract.py b/slither/core/declarations/contract.py index 73aeb61ee..545c8749e 100644 --- a/slither/core/declarations/contract.py +++ b/slither/core/declarations/contract.py @@ -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): ''' diff --git a/slither/printers/summary/variable_order.py b/slither/printers/summary/variable_order.py index dac13301a..48d96356b 100644 --- a/slither/printers/summary/variable_order.py +++ b/slither/printers/summary/variable_order.py @@ -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) diff --git a/slither/solc_parsing/declarations/contract.py b/slither/solc_parsing/declarations/contract.py index 0bedaf9c9..d3a01c51f 100644 --- a/slither/solc_parsing/declarations/contract.py +++ b/slither/solc_parsing/declarations/contract.py @@ -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): From 00ee4637b89107d1bdd1f19bd68e5ff6e4106cdf Mon Sep 17 00:00:00 2001 From: Josselin Date: Tue, 21 May 2019 12:23:00 +0100 Subject: [PATCH 5/6] Fix incorrect highlevel call lookup --- slither/slithir/convert.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/slither/slithir/convert.py b/slither/slithir/convert.py index c9014edf2..63284db77 100644 --- a/slither/slithir/convert.py +++ b/slither/slithir/convert.py @@ -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 From 6d86220a53603476f9567c3358524ea4db07fb25 Mon Sep 17 00:00:00 2001 From: Josselin Date: Tue, 21 May 2019 16:48:34 +0100 Subject: [PATCH 6/6] Improve exceptions catching --- slither/core/cfg/node.py | 6 +++++- slither/solc_parsing/declarations/function.py | 3 ++- slither/utils/expression_manipulations.py | 10 +++------- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/slither/core/cfg/node.py b/slither/core/cfg/node.py index c74096c57..0ff442a2b 100644 --- a/slither/core/cfg/node.py +++ b/slither/core/cfg/node.py @@ -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") @@ -715,7 +716,10 @@ class Node(SourceMapping, ChildFunction): elif ir.destination == SolidityVariable('this'): self._high_level_calls.append((self.function.contract, ir.function)) else: - self._high_level_calls.append((ir.destination.type.type, ir.function)) + 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)) diff --git a/slither/solc_parsing/declarations/function.py b/slither/solc_parsing/declarations/function.py index e86cbaac2..721f49941 100644 --- a/slither/solc_parsing/declarations/function.py +++ b/slither/solc_parsing/declarations/function.py @@ -966,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) diff --git a/slither/utils/expression_manipulations.py b/slither/utils/expression_manipulations.py index c5f70f015..871b0e8bd 100644 --- a/slither/utils/expression_manipulations.py +++ b/slither/utils/expression_manipulations.py @@ -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)))