Open source 4 detectors:

- boolean-cst - tautology - boolean-equal - divide-before-multiply
5 years ago · 81b3032499
parent 1b15a401c9
commit 81b3032499
5 changed files with 516 additions and 0 deletions
--- 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
 #
 #
--- a/slither/detectors/statements/boolean_constant_equality.py
+++ 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
--- a/slither/detectors/statements/boolean_constant_misuse.py
+++ 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
--- a/slither/detectors/statements/divide_before_multiply.py
+++ 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
--- a/slither/detectors/statements/type_based_tautology.py
+++ 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