From 20e57903375731351ca3e7d16b3cbb66200f531c Mon Sep 17 00:00:00 2001 From: Bernhard Mueller Date: Tue, 1 Jan 2019 10:46:57 +0700 Subject: [PATCH] Improve report descriptions in deprecated ops module --- mythril/analysis/modules/deprecated_ops.py | 10 ++++++---- .../outputs_expected/kinds_of_calls.sol.o.json | 2 +- .../outputs_expected/kinds_of_calls.sol.o.jsonv2 | 2 +- .../outputs_expected/kinds_of_calls.sol.o.markdown | 2 +- .../outputs_expected/kinds_of_calls.sol.o.text | 2 +- tests/testdata/outputs_expected/origin.sol.o.json | 2 +- tests/testdata/outputs_expected/origin.sol.o.jsonv2 | 2 +- tests/testdata/outputs_expected/origin.sol.o.markdown | 4 ++-- tests/testdata/outputs_expected/origin.sol.o.text | 4 ++-- 9 files changed, 16 insertions(+), 14 deletions(-) diff --git a/mythril/analysis/modules/deprecated_ops.py b/mythril/analysis/modules/deprecated_ops.py index 05c77bc0..114a050b 100644 --- a/mythril/analysis/modules/deprecated_ops.py +++ b/mythril/analysis/modules/deprecated_ops.py @@ -17,10 +17,11 @@ def _analyze_state(state): if instruction["opcode"] == "ORIGIN": log.debug("ORIGIN in function " + node.function_name) - title = "Use of tx.origin is deprecated." + title = "Use of tx.origin" description_head = "Use of tx.origin is deprecated." description_tail = ( - "The function `{}` retrieves the transaction origin (tx.origin) using the ORIGIN opcode. " + "The smart contract retrieves the transaction origin (tx.origin) using msg.origin. " + "Use of msg.origin is deprecated and the instruction may be removed in the future. " "Use msg.sender instead.\nSee also: " "https://solidity.readthedocs.io/en/develop/security-considerations.html#tx-origin".format( node.function_name @@ -33,8 +34,9 @@ def _analyze_state(state): title = "Use of Callcode" description_head = "Use of callcode is deprecated." description_tail = ( - "The function `{}` uses the callcode function. Callcode does not persist sender or value over the call. " - "Use delegatecall instead.".format(node.function_name) + "The callcode method executes code of another contract in the context of the caller account. " + "Due to a bug in the implementation it does not persist sender and value over the call. It was " + "therefore deprecated and may be removed in the future. Use the delegatecall method instead." ) swc_id = DEPRICATED_FUNCTIONS_USAGE diff --git a/tests/testdata/outputs_expected/kinds_of_calls.sol.o.json b/tests/testdata/outputs_expected/kinds_of_calls.sol.o.json index 9e38db02..73280f56 100644 --- a/tests/testdata/outputs_expected/kinds_of_calls.sol.o.json +++ b/tests/testdata/outputs_expected/kinds_of_calls.sol.o.json @@ -1 +1 @@ -{"error": null, "issues": [{"SourceMap": null, "address": 618, "contract": "Unknown", "debug": "", "description": "The return value of a message call is not checked.\nExternal calls return a boolean value. If the callee contract halts with an exception, 'false' is returned and execution continues in the caller. It is usually recommended to wrap external calls into a require statement to prevent unexpected states.", "function": "_function_0x141f32ff", "max_gas_used": 35865, "min_gas_used": 1113, "severity": "Low", "swc-id": "104", "title": "Unchecked Call Return Value"}, {"SourceMap": null, "address": 618, "contract": "Unknown", "debug": "", "description": "Use of callcode is deprecated.\nThe function `_function_0x141f32ff` uses the callcode function. Callcode does not persist sender or value over the call. Use delegatecall instead.", "function": "_function_0x141f32ff", "max_gas_used": 1141, "min_gas_used": 389, "severity": "Medium", "swc-id": "111", "title": "Use of Callcode"}, {"SourceMap": null, "address": 849, "contract": "Unknown", "debug": "", "description": "The return value of a message call is not checked.\nExternal calls return a boolean value. If the callee contract halts with an exception, 'false' is returned and execution continues in the caller. It is usually recommended to wrap external calls into a require statement to prevent unexpected states.", "function": "_function_0x9b58bc26", "max_gas_used": 35922, "min_gas_used": 1170, "severity": "Low", "swc-id": "104", "title": "Unchecked Call Return Value"}, {"SourceMap": null, "address": 1038, "contract": "Unknown", "debug": "", "description": "A call to a user-supplied address is executed.\nThe callee address of an external message call can be set by the caller. Note that the callee can contain arbitrary code and may re-enter any function in this contract. Review the business logic carefully to prevent averse effects on thecontract state.", "function": "_function_0xeea4c864", "max_gas_used": 1223, "min_gas_used": 471, "severity": "Medium", "swc-id": "107", "title": "External Call To User-Supplied Address"}, {"SourceMap": null, "address": 1038, "contract": "Unknown", "debug": "", "description": "The return value of a message call is not checked.\nExternal calls return a boolean value. If the callee contract halts with an exception, 'false' is returned and execution continues in the caller. It is usually recommended to wrap external calls into a require statement to prevent unexpected states.", "function": "_function_0xeea4c864", "max_gas_used": 35947, "min_gas_used": 1195, "severity": "Low", "swc-id": "104", "title": "Unchecked Call Return Value"}], "success": true} \ No newline at end of file +{"error": null, "issues": [{"SourceMap": null, "address": 618, "contract": "Unknown", "debug": "", "description": "The return value of a message call is not checked.\nExternal calls return a boolean value. If the callee contract halts with an exception, 'false' is returned and execution continues in the caller. It is usually recommended to wrap external calls into a require statement to prevent unexpected states.", "function": "_function_0x141f32ff", "max_gas_used": 35865, "min_gas_used": 1113, "severity": "Low", "swc-id": "104", "title": "Unchecked Call Return Value"}, {"SourceMap": null, "address": 618, "contract": "Unknown", "debug": "", "description": "Use of callcode is deprecated.\nThe callcode method executes code of another contract in the context of the caller account. Due to a bug in the implementation it does not persist sender and value over the call. It was therefore deprecated and may be removed in the future. Use the delegatecall method instead.", "function": "_function_0x141f32ff", "max_gas_used": 1141, "min_gas_used": 389, "severity": "Medium", "swc-id": "111", "title": "Use of Callcode"}, {"SourceMap": null, "address": 849, "contract": "Unknown", "debug": "", "description": "The return value of a message call is not checked.\nExternal calls return a boolean value. If the callee contract halts with an exception, 'false' is returned and execution continues in the caller. It is usually recommended to wrap external calls into a require statement to prevent unexpected states.", "function": "_function_0x9b58bc26", "max_gas_used": 35922, "min_gas_used": 1170, "severity": "Low", "swc-id": "104", "title": "Unchecked Call Return Value"}, {"SourceMap": null, "address": 1038, "contract": "Unknown", "debug": "", "description": "A call to a user-supplied address is executed.\nThe callee address of an external message call can be set by the caller. Note that the callee can contain arbitrary code and may re-enter any function in this contract. Review the business logic carefully to prevent averse effects on thecontract state.", "function": "_function_0xeea4c864", "max_gas_used": 1223, "min_gas_used": 471, "severity": "Medium", "swc-id": "107", "title": "External Call To User-Supplied Address"}, {"SourceMap": null, "address": 1038, "contract": "Unknown", "debug": "", "description": "The return value of a message call is not checked.\nExternal calls return a boolean value. If the callee contract halts with an exception, 'false' is returned and execution continues in the caller. It is usually recommended to wrap external calls into a require statement to prevent unexpected states.", "function": "_function_0xeea4c864", "max_gas_used": 35947, "min_gas_used": 1195, "severity": "Low", "swc-id": "104", "title": "Unchecked Call Return Value"}], "success": true} \ No newline at end of file diff --git a/tests/testdata/outputs_expected/kinds_of_calls.sol.o.jsonv2 b/tests/testdata/outputs_expected/kinds_of_calls.sol.o.jsonv2 index 6ebacfcc..5e8133d1 100644 --- a/tests/testdata/outputs_expected/kinds_of_calls.sol.o.jsonv2 +++ b/tests/testdata/outputs_expected/kinds_of_calls.sol.o.jsonv2 @@ -1 +1 @@ -{"issues": [{"description": {"head": ["Use of callcode is deprecated."], "tail": ["The function `_function_0x141f32ff` uses the callcode function. Callcode does not persist sender or value over the call. Use delegatecall instead."]}, "extra": {}, "locations": [{"sourceMap": "618:1:0"}], "severity": "Medium", "swcID": "111", "swcTitle": "Use of Deprecated Solidity Functions"}, {"description": {"head": ["A call to a user-supplied address is executed."], "tail": ["The callee address of an external message call can be set by the caller. Note that the callee can contain arbitrary code and may re-enter any function in this contract. Review the business logic carefully to prevent averse effects on thecontract state."]}, "extra": {}, "locations": [{"sourceMap": "1038:1:0"}], "severity": "Medium", "swcID": "107", "swcTitle": "Reentrancy"}, {"description": {"head": ["The return value of a message call is not checked."], "tail": ["External calls return a boolean value. If the callee contract halts with an exception, 'false' is returned and execution continues in the caller. It is usually recommended to wrap external calls into a require statement to prevent unexpected states."]}, "extra": {}, "locations": [{"sourceMap": "618:1:0"}], "severity": "Low", "swcID": "104", "swcTitle": "Unchecked Call Return Value"}, {"description": {"head": ["The return value of a message call is not checked."], "tail": ["External calls return a boolean value. If the callee contract halts with an exception, 'false' is returned and execution continues in the caller. It is usually recommended to wrap external calls into a require statement to prevent unexpected states."]}, "extra": {}, "locations": [{"sourceMap": "849:1:0"}], "severity": "Low", "swcID": "104", "swcTitle": "Unchecked Call Return Value"}, {"description": {"head": ["The return value of a message call is not checked."], "tail": ["External calls return a boolean value. If the callee contract halts with an exception, 'false' is returned and execution continues in the caller. It is usually recommended to wrap external calls into a require statement to prevent unexpected states."]}, "extra": {}, "locations": [{"sourceMap": "1038:1:0"}], "severity": "Low", "swcID": "104", "swcTitle": "Unchecked Call Return Value"}], "meta": {}, "sourceFormat": "evm-byzantium-bytecode", "sourceList": ["0x6daec61d05d8f1210661e7e7d1ed6d72bd6ade639398fac1e867aff50abfc1c1"], "sourceType": "raw-bytecode"} \ No newline at end of file +{"issues": [{"description": {"head": ["Use of callcode is deprecated."], "tail": ["The callcode method executes code of another contract in the context of the caller account. Due to a bug in the implementation it does not persist sender and value over the call. It was therefore deprecated and may be removed in the future. Use the delegatecall method instead."]}, "extra": {}, "locations": [{"sourceMap": "618:1:0"}], "severity": "Medium", "swcID": "111", "swcTitle": "Use of Deprecated Solidity Functions"}, {"description": {"head": ["A call to a user-supplied address is executed."], "tail": ["The callee address of an external message call can be set by the caller. Note that the callee can contain arbitrary code and may re-enter any function in this contract. Review the business logic carefully to prevent averse effects on thecontract state."]}, "extra": {}, "locations": [{"sourceMap": "1038:1:0"}], "severity": "Medium", "swcID": "107", "swcTitle": "Reentrancy"}, {"description": {"head": ["The return value of a message call is not checked."], "tail": ["External calls return a boolean value. If the callee contract halts with an exception, 'false' is returned and execution continues in the caller. It is usually recommended to wrap external calls into a require statement to prevent unexpected states."]}, "extra": {}, "locations": [{"sourceMap": "618:1:0"}], "severity": "Low", "swcID": "104", "swcTitle": "Unchecked Call Return Value"}, {"description": {"head": ["The return value of a message call is not checked."], "tail": ["External calls return a boolean value. If the callee contract halts with an exception, 'false' is returned and execution continues in the caller. It is usually recommended to wrap external calls into a require statement to prevent unexpected states."]}, "extra": {}, "locations": [{"sourceMap": "849:1:0"}], "severity": "Low", "swcID": "104", "swcTitle": "Unchecked Call Return Value"}, {"description": {"head": ["The return value of a message call is not checked."], "tail": ["External calls return a boolean value. If the callee contract halts with an exception, 'false' is returned and execution continues in the caller. It is usually recommended to wrap external calls into a require statement to prevent unexpected states."]}, "extra": {}, "locations": [{"sourceMap": "1038:1:0"}], "severity": "Low", "swcID": "104", "swcTitle": "Unchecked Call Return Value"}], "meta": {}, "sourceFormat": "evm-byzantium-bytecode", "sourceList": ["0x6daec61d05d8f1210661e7e7d1ed6d72bd6ade639398fac1e867aff50abfc1c1"], "sourceType": "raw-bytecode"} \ No newline at end of file diff --git a/tests/testdata/outputs_expected/kinds_of_calls.sol.o.markdown b/tests/testdata/outputs_expected/kinds_of_calls.sol.o.markdown index bf30348e..5ea1cb47 100644 --- a/tests/testdata/outputs_expected/kinds_of_calls.sol.o.markdown +++ b/tests/testdata/outputs_expected/kinds_of_calls.sol.o.markdown @@ -24,7 +24,7 @@ External calls return a boolean value. If the callee contract halts with an exce ### Description Use of callcode is deprecated. -The function `_function_0x141f32ff` uses the callcode function. Callcode does not persist sender or value over the call. Use delegatecall instead. +The callcode method executes code of another contract in the context of the caller account. Due to a bug in the implementation it does not persist sender and value over the call. It was therefore deprecated and may be removed in the future. Use the delegatecall method instead. ## Unchecked Call Return Value - SWC ID: 104 diff --git a/tests/testdata/outputs_expected/kinds_of_calls.sol.o.text b/tests/testdata/outputs_expected/kinds_of_calls.sol.o.text index 19e00cf9..05a399f3 100644 --- a/tests/testdata/outputs_expected/kinds_of_calls.sol.o.text +++ b/tests/testdata/outputs_expected/kinds_of_calls.sol.o.text @@ -17,7 +17,7 @@ Function name: _function_0x141f32ff PC address: 618 Estimated Gas Usage: 389 - 1141 Use of callcode is deprecated. -The function `_function_0x141f32ff` uses the callcode function. Callcode does not persist sender or value over the call. Use delegatecall instead. +The callcode method executes code of another contract in the context of the caller account. Due to a bug in the implementation it does not persist sender and value over the call. It was therefore deprecated and may be removed in the future. Use the delegatecall method instead. -------------------- ==== Unchecked Call Return Value ==== diff --git a/tests/testdata/outputs_expected/origin.sol.o.json b/tests/testdata/outputs_expected/origin.sol.o.json index f8fa7dce..93603018 100644 --- a/tests/testdata/outputs_expected/origin.sol.o.json +++ b/tests/testdata/outputs_expected/origin.sol.o.json @@ -1 +1 @@ -{"error": null, "issues": [{"SourceMap": null, "address": 317, "contract": "Unknown", "debug": "", "description": "Use of tx.origin is deprecated.\nThe function `transferOwnership(address)` retrieves the transaction origin (tx.origin) using the ORIGIN opcode. Use msg.sender instead.\nSee also: https://solidity.readthedocs.io/en/develop/security-considerations.html#tx-origin", "function": "transferOwnership(address)", "max_gas_used": 1051, "min_gas_used": 626, "severity": "Medium", "swc-id": "111", "title": "Use of tx.origin is deprecated."}], "success": true} \ No newline at end of file +{"error": null, "issues": [{"SourceMap": null, "address": 317, "contract": "Unknown", "debug": "", "description": "Use of tx.origin is deprecated.\nThe smart contract retrieves the transaction origin (tx.origin) using msg.origin. Use of msg.origin is deprecated and the instruction may be removed in the future. Use msg.sender instead.\nSee also: https://solidity.readthedocs.io/en/develop/security-considerations.html#tx-origin", "function": "transferOwnership(address)", "max_gas_used": 1051, "min_gas_used": 626, "severity": "Medium", "swc-id": "111", "title": "Use of tx.origin"}], "success": true} \ No newline at end of file diff --git a/tests/testdata/outputs_expected/origin.sol.o.jsonv2 b/tests/testdata/outputs_expected/origin.sol.o.jsonv2 index 8d023d47..8591ceee 100644 --- a/tests/testdata/outputs_expected/origin.sol.o.jsonv2 +++ b/tests/testdata/outputs_expected/origin.sol.o.jsonv2 @@ -1 +1 @@ -{"issues": [{"description": {"head": ["Use of tx.origin is deprecated."], "tail": ["The function `transferOwnership(address)` retrieves the transaction origin (tx.origin) using the ORIGIN opcode. Use msg.sender instead.\nSee also: https://solidity.readthedocs.io/en/develop/security-considerations.html#tx-origin"]}, "extra": {}, "locations": [{"sourceMap": "317:1:0"}], "severity": "Medium", "swcID": "111", "swcTitle": "Use of Deprecated Solidity Functions"}], "meta": {}, "sourceFormat": "evm-byzantium-bytecode", "sourceList": ["0x25b20ef097dfc0aa56a932c4e09f06ee02a69c005767df86877f48c6c2412f03"], "sourceType": "raw-bytecode"} \ No newline at end of file +{"issues": [{"description": {"head": ["Use of tx.origin is deprecated."], "tail": ["The smart contract retrieves the transaction origin (tx.origin) using msg.origin. Use of msg.origin is deprecated and the instruction may be removed in the future. Use msg.sender instead.\nSee also: https://solidity.readthedocs.io/en/develop/security-considerations.html#tx-origin"]}, "extra": {}, "locations": [{"sourceMap": "317:1:0"}], "severity": "Medium", "swcID": "111", "swcTitle": "Use of Deprecated Solidity Functions"}], "meta": {}, "sourceFormat": "evm-byzantium-bytecode", "sourceList": ["0x25b20ef097dfc0aa56a932c4e09f06ee02a69c005767df86877f48c6c2412f03"], "sourceType": "raw-bytecode"} \ No newline at end of file diff --git a/tests/testdata/outputs_expected/origin.sol.o.markdown b/tests/testdata/outputs_expected/origin.sol.o.markdown index e9603d8c..cbacdae3 100644 --- a/tests/testdata/outputs_expected/origin.sol.o.markdown +++ b/tests/testdata/outputs_expected/origin.sol.o.markdown @@ -1,6 +1,6 @@ # Analysis results for test-filename.sol -## Use of tx.origin is deprecated. +## Use of tx.origin - SWC ID: 111 - Severity: Medium - Contract: Unknown @@ -11,5 +11,5 @@ ### Description Use of tx.origin is deprecated. -The function `transferOwnership(address)` retrieves the transaction origin (tx.origin) using the ORIGIN opcode. Use msg.sender instead. +The smart contract retrieves the transaction origin (tx.origin) using msg.origin. Use of msg.origin is deprecated and the instruction may be removed in the future. Use msg.sender instead. See also: https://solidity.readthedocs.io/en/develop/security-considerations.html#tx-origin diff --git a/tests/testdata/outputs_expected/origin.sol.o.text b/tests/testdata/outputs_expected/origin.sol.o.text index 2b96e50f..5b62a208 100644 --- a/tests/testdata/outputs_expected/origin.sol.o.text +++ b/tests/testdata/outputs_expected/origin.sol.o.text @@ -1,4 +1,4 @@ -==== Use of tx.origin is deprecated. ==== +==== Use of tx.origin ==== SWC ID: 111 Severity: Medium Contract: Unknown @@ -6,7 +6,7 @@ Function name: transferOwnership(address) PC address: 317 Estimated Gas Usage: 626 - 1051 Use of tx.origin is deprecated. -The function `transferOwnership(address)` retrieves the transaction origin (tx.origin) using the ORIGIN opcode. Use msg.sender instead. +The smart contract retrieves the transaction origin (tx.origin) using msg.origin. Use of msg.origin is deprecated and the instruction may be removed in the future. Use msg.sender instead. See also: https://solidity.readthedocs.io/en/develop/security-considerations.html#tx-origin --------------------