From 82fd8606ea736cb2d1191bce638ab5f7f261ed11 Mon Sep 17 00:00:00 2001 From: Natalie Chin Date: Fri, 4 Dec 2020 14:56:59 -0500 Subject: [PATCH] Added test and json artifact for boolean-constant-misuse --- .../statements/boolean_constant_misuse.py | 53 ++--- .../boolean-constant-misuse.sol | 46 ++++ ...isuse.sol.0.6.0.BooleanConstantMisuse.json | 208 ++++++++++++++++++ tests/test_detectors.py | 2 + 4 files changed, 280 insertions(+), 29 deletions(-) create mode 100644 tests/detectors/boolean-constant-misuse/boolean-constant-misuse.sol create mode 100644 tests/detectors/boolean-constant-misuse/boolean-constant-misuse.sol.0.6.0.BooleanConstantMisuse.json diff --git a/slither/detectors/statements/boolean_constant_misuse.py b/slither/detectors/statements/boolean_constant_misuse.py index 3b77ba02d..4ab6fe8ff 100644 --- a/slither/detectors/statements/boolean_constant_misuse.py +++ b/slither/detectors/statements/boolean_constant_misuse.py @@ -67,39 +67,35 @@ Other uses (in complex expressions, as conditionals) indicate either an error or results = [] # Loop for each function and modifier. - for function in contract.functions_declared: # pylint: disable=too-many-nested-blocks + for function in contract.functions_declared: f_results = set() # Loop for every node in this function, looking for boolean constants for node in function.nodes: # Do not report "while(true)" - if node.type == NodeType.IFLOOP: - if node.irs: - if len(node.irs) == 1: - ir = node.irs[0] - if isinstance(ir, Condition) and ir.value == Constant( - "True", ElementaryType("bool") - ): - continue + if node.type == NodeType.IFLOOP and node.irs and len(node.irs) == 1: + ir = node.irs[0] + if isinstance(ir, Condition) and ir.value == Constant( + "True", ElementaryType("bool") + ): + continue for ir in node.irs: if isinstance(ir, (Assignment, Call, Return, InitArray)): # It's ok to use a bare boolean constant in these contexts continue - if isinstance(ir, Binary): - if ir.type in [ + if isinstance(ir, Binary) and ir.type in [ BinaryType.ADDITION, BinaryType.EQUAL, BinaryType.NOT_EQUAL, ]: - # Comparing to a Boolean constant is dubious style, but harmless - # Equal is catch by another detector (informational severity) - continue + # Comparing to a Boolean constant is dubious style, but harmless + # Equal is catch by another detector (informational severity) + continue for r in ir.read: - if isinstance(r, Constant): - if isinstance(r.value, bool): - f_results.add(node) + if isinstance(r, Constant) and isinstance(r.value, bool): + f_results.add(node) results.append((function, f_results)) # Return the resulting set of nodes with improper uses of Boolean constants @@ -112,17 +108,16 @@ Other uses (in complex expressions, as conditionals) indicate either an error or results = [] for contract in self.contracts: boolean_constant_misuses = self._detect_boolean_constant_misuses(contract) - if boolean_constant_misuses: - for (func, nodes) in boolean_constant_misuses: - for node in nodes: - info = [ - func, - " uses a Boolean constant improperly:\n\t-", - node, - "\n", - ] - - res = self.generate_result(info) - results.append(res) + for (func, nodes) in boolean_constant_misuses: + for node in nodes: + info = [ + func, + " uses a Boolean constant improperly:\n\t-", + node, + "\n", + ] + + res = self.generate_result(info) + results.append(res) return results diff --git a/tests/detectors/boolean-constant-misuse/boolean-constant-misuse.sol b/tests/detectors/boolean-constant-misuse/boolean-constant-misuse.sol new file mode 100644 index 000000000..6d3df44f3 --- /dev/null +++ b/tests/detectors/boolean-constant-misuse/boolean-constant-misuse.sol @@ -0,0 +1,46 @@ +contract MyConc { + function bad0(bool foo) public pure returns (bool) { + if (foo) { + return true; + } + return false; + } + + function bad1(bool b) public pure returns (bool) { + return (b || true); + } + + function bad2(bool x, uint8 y) public pure returns (bool) { + while (x == (y > 0)) { + return true; + } + return false; + } + + function bad3(bool a) public pure returns (bool) { + uint256 b = 0; + while (a) { + b++; + } + return true; + } + + function bad4() public pure returns (bool) { + uint256 b = 0; + while (true) { + b++; + } + return true; + } + + function bad5() public pure returns (bool) { + while (true) { + return true; + } + return false; + } + + function good() public pure returns (bool) { + return true; + } +} \ No newline at end of file diff --git a/tests/detectors/boolean-constant-misuse/boolean-constant-misuse.sol.0.6.0.BooleanConstantMisuse.json b/tests/detectors/boolean-constant-misuse/boolean-constant-misuse.sol.0.6.0.BooleanConstantMisuse.json new file mode 100644 index 000000000..a4bf8607d --- /dev/null +++ b/tests/detectors/boolean-constant-misuse/boolean-constant-misuse.sol.0.6.0.BooleanConstantMisuse.json @@ -0,0 +1,208 @@ +[ + [ + { + "elements": [ + { + "type": "function", + "name": "bad1", + "source_mapping": { + "start": 162, + "length": 84, + "filename_used": "/GENERIC_PATH", + "filename_relative": "tests/detectors/boolean-constant-misuse/boolean-constant-misuse.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/boolean-constant-misuse/boolean-constant-misuse.sol", + "is_dependency": false, + "lines": [ + 9, + 10, + 11 + ], + "starting_column": 5, + "ending_column": 6 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "MyConc", + "source_mapping": { + "start": 0, + "length": 923, + "filename_used": "/GENERIC_PATH", + "filename_relative": "tests/detectors/boolean-constant-misuse/boolean-constant-misuse.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/boolean-constant-misuse/boolean-constant-misuse.sol", + "is_dependency": false, + "lines": [ + 1, + 2, + 3, + 4, + 5, + 6, + 7, + 8, + 9, + 10, + 11, + 12, + 13, + 14, + 15, + 16, + 17, + 18, + 19, + 20, + 21, + 22, + 23, + 24, + 25, + 26, + 27, + 28, + 29, + 30, + 31, + 32, + 33, + 34, + 35, + 36, + 37, + 38, + 39, + 40, + 41, + 42, + 43, + 44, + 45, + 46, + 47 + ], + "starting_column": 1, + "ending_column": 0 + } + }, + "signature": "bad1(bool)" + } + }, + { + "type": "node", + "name": "(b || true)", + "source_mapping": { + "start": 221, + "length": 18, + "filename_used": "/GENERIC_PATH", + "filename_relative": "tests/detectors/boolean-constant-misuse/boolean-constant-misuse.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/boolean-constant-misuse/boolean-constant-misuse.sol", + "is_dependency": false, + "lines": [ + 10 + ], + "starting_column": 9, + "ending_column": 27 + }, + "type_specific_fields": { + "parent": { + "type": "function", + "name": "bad1", + "source_mapping": { + "start": 162, + "length": 84, + "filename_used": "/GENERIC_PATH", + "filename_relative": "tests/detectors/boolean-constant-misuse/boolean-constant-misuse.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/boolean-constant-misuse/boolean-constant-misuse.sol", + "is_dependency": false, + "lines": [ + 9, + 10, + 11 + ], + "starting_column": 5, + "ending_column": 6 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "MyConc", + "source_mapping": { + "start": 0, + "length": 923, + "filename_used": "/GENERIC_PATH", + "filename_relative": "tests/detectors/boolean-constant-misuse/boolean-constant-misuse.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/boolean-constant-misuse/boolean-constant-misuse.sol", + "is_dependency": false, + "lines": [ + 1, + 2, + 3, + 4, + 5, + 6, + 7, + 8, + 9, + 10, + 11, + 12, + 13, + 14, + 15, + 16, + 17, + 18, + 19, + 20, + 21, + 22, + 23, + 24, + 25, + 26, + 27, + 28, + 29, + 30, + 31, + 32, + 33, + 34, + 35, + 36, + 37, + 38, + 39, + 40, + 41, + 42, + 43, + 44, + 45, + 46, + 47 + ], + "starting_column": 1, + "ending_column": 0 + } + }, + "signature": "bad1(bool)" + } + } + } + } + ], + "description": "MyConc.bad1(bool) (tests/detectors/boolean-constant-misuse/boolean-constant-misuse.sol#9-11) uses a Boolean constant improperly:\n\t-(b || true) (tests/detectors/boolean-constant-misuse/boolean-constant-misuse.sol#10)\n", + "markdown": "[MyConc.bad1(bool)](tests/detectors/boolean-constant-misuse/boolean-constant-misuse.sol#L9-L11) uses a Boolean constant improperly:\n\t-[(b || true)](tests/detectors/boolean-constant-misuse/boolean-constant-misuse.sol#L10)\n", + "id": "12517fed0ec8f0a2232b467a6add9fd94a6a84325017e02e8a48794fc9112c6b", + "check": "boolean-cst", + "impact": "Medium", + "confidence": "Medium" + } + ] +] \ No newline at end of file diff --git a/tests/test_detectors.py b/tests/test_detectors.py index 517394791..04a6bb405 100644 --- a/tests/test_detectors.py +++ b/tests/test_detectors.py @@ -37,6 +37,7 @@ from slither.detectors.shadowing.local import LocalShadowing from slither.detectors.shadowing.state import StateShadowing from slither.detectors.source.rtlo import RightToLeftOverride from slither.detectors.statements.assembly import Assembly +from slither.detectors.statements.boolean_constant_misuse import BooleanConstantMisuse from slither.detectors.statements.boolean_constant_equality import BooleanEquality from slither.detectors.statements.calls_in_loop import MultipleCallsInLoop from slither.detectors.statements.controlled_delegatecall import ControlledDelegateCall @@ -97,6 +98,7 @@ ALL_TESTS = [ "tests/detectors/boolean-constant-equality/boolean-constant-equality.sol", "0.4.25", ), + Test(BooleanConstantMisuse, "tests/detectors/boolean-constant-misuse/boolean-constant-misuse.sol", "0.6.0"), Test(UncheckedLowLevel, "tests/detectors/unchecked-lowlevel/unchecked_lowlevel.sol", "0.4.25"), Test( UncheckedLowLevel,