From 81b303249950375366317f41e3999cb0c7abdcba Mon Sep 17 00:00:00 2001 From: Josselin Date: Wed, 29 Jan 2020 21:36:22 +0100 Subject: [PATCH] Open source 4 detectors: - boolean-cst - tautology - boolean-equal - divide-before-multiply --- slither/detectors/all_detectors.py | 4 + .../statements/boolean_constant_equality.py | 79 ++++++++ .../statements/boolean_constant_misuse.py | 96 ++++++++++ .../statements/divide_before_multiply.py | 179 ++++++++++++++++++ .../statements/type_based_tautology.py | 158 ++++++++++++++++ 5 files changed, 516 insertions(+) create mode 100644 slither/detectors/statements/boolean_constant_equality.py create mode 100644 slither/detectors/statements/boolean_constant_misuse.py create mode 100644 slither/detectors/statements/divide_before_multiply.py create mode 100644 slither/detectors/statements/type_based_tautology.py diff --git a/slither/detectors/all_detectors.py b/slither/detectors/all_detectors.py index 8e5412773..0a4c0acd1 100644 --- a/slither/detectors/all_detectors.py +++ b/slither/detectors/all_detectors.py @@ -40,5 +40,9 @@ from .statements.too_many_digits import TooManyDigits from .operations.unchecked_low_level_return_values import UncheckedLowLevel from .operations.unchecked_send_return_value import UncheckedSend from .operations.void_constructor import VoidConstructor +from .statements.type_based_tautology import TypeBasedTautology +from .statements.boolean_constant_equality import BooleanEquality +from .statements.boolean_constant_misuse import BooleanConstantMisuse +from .statements.divide_before_multiply import DivideBeforeMultiply # # diff --git a/slither/detectors/statements/boolean_constant_equality.py b/slither/detectors/statements/boolean_constant_equality.py new file mode 100644 index 000000000..d43e75b7c --- /dev/null +++ b/slither/detectors/statements/boolean_constant_equality.py @@ -0,0 +1,79 @@ +""" +Module detecting misuse of Boolean constants +""" + +from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification +from slither.slithir.operations import Assignment, Call, Return, InitArray, Binary, BinaryType +from slither.slithir.variables import Constant + + +class BooleanEquality(AbstractDetector): + """ + Boolean constant misuse + """ + + ARGUMENT = 'boolean-equal' + HELP = 'Comparison to boolean constant' + IMPACT = DetectorClassification.INFORMATIONAL + CONFIDENCE = DetectorClassification.HIGH + + WIKI = 'https://github.com/trailofbits/slither-private/wiki/Vulnerabilities-Description#boolean-equality' + + WIKI_TITLE = 'Boolean Equality' + WIKI_DESCRIPTION = '''Detects the comparison to boolean constant.''' + WIKI_EXPLOIT_SCENARIO = ''' +```solidity +contract A { + function f(bool x) public { + // ... + if (x == true) { // bad! + // ... + } + // ... + } +} +``` +Boolean can be used directly and do not need to be compare to `true` or `false`.''' + + WIKI_RECOMMENDATION = '''Remove the equality to the boolean constant.''' + + @staticmethod + def _detect_boolean_equality(contract): + + # Create our result set. + results = [] + + # Loop for each function and modifier. + for function in contract.functions_and_modifiers_declared: + f_results = set() + + # Loop for every node in this function, looking for boolean constants + for node in function.nodes: + for ir in node.irs: + if isinstance(ir, Binary): + if ir.type in [BinaryType.EQUAL, BinaryType.NOT_EQUAL]: + for r in ir.read: + if isinstance(r, Constant): + if type(r.value) is bool: + f_results.add(node) + results.append((function, f_results)) + + # Return the resulting set of nodes with improper uses of Boolean constants + return results + + def _detect(self): + """ + Detect Boolean constant misuses + """ + results = [] + for contract in self.contracts: + boolean_constant_misuses = self._detect_boolean_equality(contract) + if boolean_constant_misuses: + for (func, nodes) in boolean_constant_misuses: + for node in nodes: + info = [func, " compares to a boolean constant:\n\t-", node, "\n"] + + res = self.generate_result(info) + results.append(res) + + return results diff --git a/slither/detectors/statements/boolean_constant_misuse.py b/slither/detectors/statements/boolean_constant_misuse.py new file mode 100644 index 000000000..c68fef9cb --- /dev/null +++ b/slither/detectors/statements/boolean_constant_misuse.py @@ -0,0 +1,96 @@ +""" +Module detecting misuse of Boolean constants +""" + +from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification +from slither.slithir.operations import Assignment, Call, Return, InitArray, Binary, BinaryType +from slither.slithir.variables import Constant + + +class BooleanConstantMisuse(AbstractDetector): + """ + Boolean constant misuse + """ + + ARGUMENT = 'boolean-cst' + HELP = 'Misuse of Boolean constant' + IMPACT = DetectorClassification.MEDIUM + CONFIDENCE = DetectorClassification.MEDIUM + + WIKI = 'https://github.com/trailofbits/slither-private/wiki/Vulnerabilities-Description#misuse-of-a-boolean-constant' + + WIKI_TITLE = 'Misuse of a Boolean constant' + WIKI_DESCRIPTION = '''Detects the misuse of a Boolean constant.''' + WIKI_EXPLOIT_SCENARIO = ''' +```solidity +contract A { + function f(uint x) public { + // ... + if (false) { // bad! + // ... + } + // ... + } + + function g(bool b) public returns (bool) { + // ... + return (b || true); // bad! + // ... + } +} +``` +Boolean constants in code have only a few legitimate uses. Other uses (in complex expressions, as conditionals) indicate either an error or (most likely) the persistence of debugging/development code that is likely faulty.''' + + WIKI_RECOMMENDATION = '''Verify and simplify the condition.''' + + @staticmethod + def _detect_boolean_constant_misuses(contract): + """ + Detects and returns all nodes which misuse a Boolean constant. + :param contract: Contract to detect assignment within. + :return: A list of misusing nodes. + """ + + # Create our result set. + results = [] + + # Loop for each function and modifier. + 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: + 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 [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 + for r in ir.read: + if isinstance(r, Constant): + if type(r.value) is bool: + f_results.add(node) + results.append((function, f_results)) + + # Return the resulting set of nodes with improper uses of Boolean constants + return results + + def _detect(self): + """ + Detect Boolean constant misuses + """ + 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) + + return results diff --git a/slither/detectors/statements/divide_before_multiply.py b/slither/detectors/statements/divide_before_multiply.py new file mode 100644 index 000000000..95b0c988b --- /dev/null +++ b/slither/detectors/statements/divide_before_multiply.py @@ -0,0 +1,179 @@ +""" +Module detecting possible loss of precision due to divide before multiple +""" +from collections import defaultdict +from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification +from slither.slithir.operations import Binary, Assignment, BinaryType, LibraryCall +from slither.slithir.variables import Constant + + +def is_division(ir): + if isinstance(ir, Binary): + if ir.type == BinaryType.DIVISION: + return True + + if isinstance(ir, LibraryCall): + if ir.function.name.lower() in ['div', 'safediv', ]: + if len(ir.arguments) == 2: + if ir.lvalue: + return True + return False + + +def is_multiplication(ir): + if isinstance(ir, Binary): + if ir.type == BinaryType.MULTIPLICATION: + return True + + if isinstance(ir, LibraryCall): + if ir.function.name.lower() in ['mul', 'safemul', ]: + if len(ir.arguments) == 2: + if ir.lvalue: + return True + return False + + +def is_assert(node): + if node.contains_require_or_assert(): + return True + # Old Solidity code where using an internal 'assert(bool)' function + # While we dont check that this function is correct, we assume it is + # To avoid too many FP + if "assert(bool)" in [c.full_name for c in node.internal_calls]: + return True + return False + + +class DivideBeforeMultiply(AbstractDetector): + """ + Divide before multiply + """ + + ARGUMENT = 'divide-before-multiply' + HELP = 'Imprecise arithmetic operations order' + IMPACT = DetectorClassification.MEDIUM + CONFIDENCE = DetectorClassification.MEDIUM + + WIKI = 'https://github.com/trailofbits/slither-private/wiki/Vulnerabilities-Description#divide-before-multiply' + + WIKI_TITLE = 'Divide before multiply' + WIKI_DESCRIPTION = '''Solidity only supports integers, so division will often truncate; performing a multiply before a divison can sometimes avoid loss of precision.''' + WIKI_DESCRIPTION = '''Solidity integeer division will might truncate. As a result, performing a multiply before a divison might lead to loss of precision.''' + WIKI_EXPLOIT_SCENARIO = ''' +```solidity +contract A { + function f(uint n) public { + coins = (oldSupply / n) * interest; + } +} +``` +If `n` is greater than `oldSupply`, `coins` will be zero. For example, with `oldSupply = 5; n = 10, interest = 2`, coins will be zero. +If `(oldSupply * interest / n)` was used, `coins` would have been `1`. +In general, it's usually a good idea to re-arrange arithmetic to perform multiply before divide, unless the limit of a smaller type makes this dangerous.''' + + WIKI_RECOMMENDATION = '''Consider ordering multiplication prior division.''' + + def _explore(self, node, explored, f_results, divisions): + if node in explored: + return + explored.add(node) + + equality_found = False + # List of nodes related to one bug instance + node_results = [] + + for ir in node.irs: + # check for Constant, has its not hashable (TODO: make Constant hashable) + if isinstance(ir, Assignment) and not isinstance(ir.rvalue, Constant): + if ir.rvalue in divisions: + # Avoid dupplicate. We dont use set so we keep the order of the nodes + if node not in divisions[ir.rvalue]: + divisions[ir.lvalue] = divisions[ir.rvalue] + [node] + else: + divisions[ir.lvalue] = divisions[ir.rvalue] + + if is_division(ir): + divisions[ir.lvalue] = [node] + + if is_multiplication(ir): + mul_arguments = ir.read if isinstance(ir, Binary) else ir.arguments + nodes = [] + for r in mul_arguments: + if not isinstance(r, Constant) and (r in divisions): + # Dont add node already present to avoid dupplicate + # We dont use set to keep the order of the nodes + if node in divisions[r]: + nodes += [n for n in divisions[r] if n not in nodes] + else: + nodes += [n for n in divisions[r] + [node] if n not in nodes] + if nodes: + node_results = nodes + + if isinstance(ir, Binary) and ir.type == BinaryType.EQUAL: + equality_found = True + + if node_results: + # We do not track the case where the multiplication is done in a require() or assert() + # Which also contains a ==, to prevent FP due to the form + # assert(a == b * c + a % b) + if is_assert(node) and equality_found: + pass + else: + f_results.append(node_results) + + for son in node.sons: + self._explore(son, explored, f_results, divisions) + + def detect_divide_before_multiply(self, contract): + """ + Detects and returns all nodes with multiplications of division results. + :param contract: Contract to detect assignment within. + :return: A list of nodes with multiplications of divisions. + """ + + # Create our result set. + # List of tuple (function -> list(list(nodes))) + # Each list(nodes) of the list is one bug instances + # Each node in the list(nodes) is involved in the bug + results = [] + + # Loop for each function and modifier. + for function in contract.functions_declared: + if not function.entry_point: + continue + + # List of list(nodes) + # Each list(nodes) is one bug instances + f_results = [] + + # lvalue -> node + # track all the division results (and the assignment of the division results) + divisions = defaultdict(list) + + self._explore(function.entry_point, set(), f_results, divisions) + + for f_result in f_results: + results.append((function, f_result)) + + # Return the resulting set of nodes with divisions before multiplications + return results + + def _detect(self): + """ + Detect divisions before multiplications + """ + results = [] + for contract in self.contracts: + divisions_before_multiplications = self.detect_divide_before_multiply(contract) + if divisions_before_multiplications: + for (func, nodes) in divisions_before_multiplications: + + info = [func, " performs a multiplication on the result of a division:\n"] + + for node in nodes: + info += ["\t-", node, "\n"] + + res = self.generate_result(info) + results.append(res) + + return results diff --git a/slither/detectors/statements/type_based_tautology.py b/slither/detectors/statements/type_based_tautology.py new file mode 100644 index 000000000..d4307ace3 --- /dev/null +++ b/slither/detectors/statements/type_based_tautology.py @@ -0,0 +1,158 @@ +""" +Module detecting tautologies and contradictions based on types in comparison operations over integers +""" + +from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification +from slither.slithir.operations import Binary, BinaryType +from slither.slithir.variables import Constant +from slither.core.solidity_types.elementary_type import Int, Uint + +class TypeBasedTautology(AbstractDetector): + """ + Type-based tautology or contradiction + """ + + ARGUMENT = 'tautology' + HELP = 'Tautology or contradiction' + IMPACT = DetectorClassification.MEDIUM + CONFIDENCE = DetectorClassification.HIGH + + WIKI = 'https://github.com/trailofbits/slither-private/wiki/Vulnerabilities-Description#tautology-or-contradiction' + + WIKI_TITLE = 'Tautology or contradiction' + WIKI_DESCRIPTION = '''Detects expressions that are tautologies or contradictions.''' + WIKI_EXPLOIT_SCENARIO = ''' +```solidity +contract A { + function f(uint x) public { + // ... + if (x >= 0) { // bad -- always true + // ... + } + // ... + } + + function g(uint8 y) public returns (bool) { + // ... + return (y < 512); // bad! + // ... + } +} +``` +`x` is an `uint256`, as a result `x >= 0` will be always true. +`y` is an `uint8`, as a result `y <512` will be always true. +''' + + WIKI_RECOMMENDATION = '''Fix the incorrect comparison by chaning the value type or the comparison.''' + + def typeRange(self, t): + bits = int(t.split("int")[1]) + if t in Uint: + return (0, (2**bits)-1) + if t in Int: + v = (2**(bits-1))-1 + return (-v, v) + + + flip_table = { + BinaryType.GREATER: BinaryType.LESS, + BinaryType.GREATER_EQUAL: BinaryType.LESS_EQUAL, + BinaryType.LESS: BinaryType.GREATER, + BinaryType.LESS_EQUAL: BinaryType.GREATER_EQUAL, + } + + def _detect_tautology_or_contradiction(self, low, high, cval, op): + ''' + Return true if "[low high] op cval " is always true or always false + :param low: + :param high: + :param cval: + :param op: + :return: + ''' + if op == BinaryType.LESS: + # a < cval + # its a tautology if + # high(a) < cval + # its a contradiction if + # low(a) >= cval + return high < cval or low >= cval + elif op == BinaryType.GREATER: + # a > cval + # its a tautology if + # low(a) > cval + # its a contradiction if + # high(a) <= cval + return low > cval or high <= cval + elif op == BinaryType.LESS_EQUAL: + # a <= cval + # its a tautology if + # high(a) <= cval + # its a contradiction if + # low(a) > cval + return (high <= cval) or (low > cval) + elif op == BinaryType.GREATER_EQUAL: + # a >= cval + # its a tautology if + # low(a) >= cval + # its a contradiction if + # high(a) < cval + return (low >= cval) or (high < cval) + return False + + def detect_type_based_tautologies(self, contract): + """ + Detects and returns all nodes with tautology/contradiction comparisons (based on type alone). + :param contract: Contract to detect assignment within. + :return: A list of nodes with tautolgies/contradictions. + """ + + # Create our result set. + results = [] + allInts = Int + Uint + + # Loop for each function and modifier. + for function in contract.functions_declared: + f_results = set() + + for node in function.nodes: + for ir in node.irs: + if isinstance(ir, Binary) and ir.type in self.flip_table: + # If neither side is a constant, we can't do much + if isinstance(ir.variable_left, Constant): + cval = ir.variable_left.value + rtype = str(ir.variable_right.type) + if rtype in allInts: + (low, high) = self.typeRange(rtype) + if self._detect_tautology_or_contradiction(low, high, cval, self.flip_table[ir.type]): + f_results.add(node) + + if isinstance(ir.variable_right, Constant): + cval = ir.variable_right.value + ltype = str(ir.variable_left.type) + if ltype in allInts: + (low, high) = self.typeRange(ltype) + if self._detect_tautology_or_contradiction(low, high, cval, ir.type): + f_results.add(node) + results.append((function, f_results)) + + # Return the resulting set of nodes with tautologies and contradictions + return results + + def _detect(self): + """ + Detect tautological (or contradictory) comparisons + """ + results = [] + for contract in self.contracts: + tautologies = self.detect_type_based_tautologies(contract) + if tautologies: + for (func, nodes) in tautologies: + for node in nodes: + info = [func, " contains a tautology or contradiction:\n"] + info += ["\t- ", node, "\n"] + + res = self.generate_result(info) + results.append(res) + + return results