Merge branch 'dev' into dev-upgradibility

setup.py

@@ -5,7 +5,7 @@ setup(
     description='Slither is a Solidity static analysis framework written in Python 3.',
     url='https://github.com/trailofbits/slither',
     author='Trail of Bits',
-    version='0.5.0',
+    version='0.5.1',
     packages=find_packages(),
     python_requires='>=3.6',
     install_requires=['prettytable>=0.7.2', 'pysha3>=1.0.2'],

slither/core/cfg/node.py

@@ -525,7 +525,12 @@ class Node(SourceMapping, ChildFunction):
                 if var and self._is_valid_slithir_var(var):
                     self._slithir_vars.add(var)
 
-            self._vars_read += [v for v in ir.read if self._is_non_slithir_var(v)]
+            if not isinstance(ir, (Phi, Index, Member)):
+                self._vars_read += [v for v in ir.read if self._is_non_slithir_var(v)]
+                for var in ir.read:
+                    if isinstance(var, (ReferenceVariable)):
+                        self._vars_read.append(var.points_to_origin)
+
             if isinstance(ir, OperationWithLValue):
                 if isinstance(ir, (Index, Member, Length, Balance)):
                     continue  # Don't consider Member and Index operations -> ReferenceVariable
@@ -582,9 +587,13 @@ class Node(SourceMapping, ChildFunction):
         for ir in self.irs_ssa:
             if isinstance(ir, (PhiCallback)):
                 continue
-            self._ssa_vars_read += [v for v in ir.read if isinstance(v,
-                                                                     (StateIRVariable,
-                                                                      LocalIRVariable))]
+            if not isinstance(ir, (Phi, Index, Member)):
+                self._ssa_vars_read += [v for v in ir.read if isinstance(v,
+                                                                         (StateIRVariable,
+                                                                          LocalIRVariable))]
+                for var in ir.read:
+                    if isinstance(var, (ReferenceVariable)):
+                        self._vars_read.append(var.points_to_origin)
             if isinstance(ir, OperationWithLValue):
                 if isinstance(ir, (Index, Member, Length, Balance)):
                     continue  # Don't consider Member and Index operations -> ReferenceVariable
@@ -596,7 +605,6 @@ class Node(SourceMapping, ChildFunction):
                     if isinstance(ir, (PhiCallback)):
                         continue
                     self._ssa_vars_written.append(var)
-
         self._ssa_vars_read = list(set(self._ssa_vars_read))
         self._ssa_state_vars_read = [v for v in self._ssa_vars_read if isinstance(v, StateVariable)]
         self._ssa_local_vars_read = [v for v in self._ssa_vars_read if isinstance(v, LocalVariable)]

slither/core/declarations/contract.py

@@ -19,7 +19,12 @@ class Contract(ChildSlither, SourceMapping):
 
         self._name = None
         self._id = None
-        self._inheritance = []
+        self._inheritance = [] # all contract inherited, c3 linearization
+        self._immediate_inheritance = [] # immediate inheritance
+
+        # Constructors called on contract's definition
+        # contract B is A(1) { ..
+        self._explicit_base_constructor_calls = []
 
         self._enums = {}
         self._structures = {}
@@ -60,6 +65,13 @@ class Contract(ChildSlither, SourceMapping):
         '''
         return list(self._inheritance)
 
+    @property
+    def immediate_inheritance(self):
+        '''
+            list(Contract): List of contracts immediately inherited from (fathers). Order: order of declaration.
+        '''
+        return list(self._immediate_inheritance)
+
     @property
     def inheritance_reverse(self):
         '''
@@ -67,8 +79,10 @@ class Contract(ChildSlither, SourceMapping):
         '''
         return reversed(self._inheritance)
 
-    def setInheritance(self, inheritance):
+    def setInheritance(self, inheritance, immediate_inheritance, called_base_constructor_contracts):
         self._inheritance = inheritance
+        self._immediate_inheritance = immediate_inheritance
+        self._explicit_base_constructor_calls = called_base_constructor_contracts
 
     @property
     def derived_contracts(self):
@@ -101,7 +115,32 @@ class Contract(ChildSlither, SourceMapping):
 
     @property
     def constructor(self):
-        return next((func for func in self.functions if func.is_constructor), None)
+        '''
+            Return the contract's immediate constructor.
+            If there is no immediate constructor, returns the first constructor
+            executed, following the c3 linearization
+            Return None if there is no constructor.
+        '''
+        cst = self.constructor_not_inherited
+        if cst:
+            return cst
+        for inherited_contract in self.inheritance:
+            cst = inherited_contract.constructor_not_inherited
+            if cst:
+                return cst
+        return None
+
+    @property
+    def constructor_not_inherited(self):
+        return next((func for func in self.functions if func.is_constructor and func.contract == self), None)
+
+    @property
+    def constructors(self):
+        '''
+            Return the list of constructors (including inherited)
+        '''
+        return [func for func in self.functions if func.is_constructor]
+
 
     @property
     def functions(self):
@@ -131,6 +170,19 @@ class Contract(ChildSlither, SourceMapping):
         '''
         return [f for f in self.functions if f.visibility in ['public', 'external']]
 
+    @property
+    def explicit_base_constructor_calls(self):
+        """
+            list(Function): List of the base constructors called explicitly by this contract definition.
+
+                            Base constructors called by any constructor definition will not be included.
+                            Base constructors implicitly called by the contract definition (without
+                            parenthesis) will not be included.
+
+                            On "contract B is A(){..}" it returns the constructor of A
+        """
+        return [c.constructor for c in self._explicit_base_constructor_calls if c.constructor]
+
     @property
     def modifiers(self):
         '''
@@ -210,6 +262,7 @@ class Contract(ChildSlither, SourceMapping):
         all_state_variables_written = [f.all_state_variables_written() for f in self.functions + self.modifiers]
         all_state_variables_written = [item for sublist in all_state_variables_written for item in sublist]
         return list(set(all_state_variables_written))
+
     @property
     def all_state_variables_read(self):
         '''

slither/core/declarations/function.py

@@ -56,9 +56,25 @@ class Function(ChildContract, SourceMapping):
         self._expression_calls = []
         self._expression_modifiers = []
         self._modifiers = []
+        self._explicit_base_constructor_calls = []
         self._payable = False
         self._contains_assembly = False
 
+        self._expressions = None
+        self._slithir_operations = None
+
+        self._all_expressions = None
+        self._all_slithir_operations = None
+        self._all_internals_calls = None
+        self._all_high_level_calls = None
+        self._all_low_level_calls = None
+        self._all_state_variables_read = None
+        self._all_solidity_variables_read = None
+        self._all_state_variables_written = None
+        self._all_conditional_state_variables_read = None
+        self._all_conditional_solidity_variables_read = None
+        self._all_solidity_variables_used_as_args = None
+
     @property
     def contains_assembly(self):
         return self._contains_assembly
@@ -198,6 +214,17 @@ class Function(ChildContract, SourceMapping):
         """
         return list(self._modifiers)
 
+    @property
+    def explicit_base_constructor_calls(self):
+        """
+            list(Function): List of the base constructors called explicitly by this presumed constructor definition.
+
+                            Base constructors implicitly or explicitly called by the contract definition will not be
+                            included.
+        """
+        # This is a list of contracts internally, so we convert it to a list of constructor functions.
+        return [c.constructor_not_inherited for c in self._explicit_base_constructor_calls if c.constructor_not_inherited]
+
     def __str__(self):
         return self._name
 
@@ -328,9 +355,22 @@ class Function(ChildContract, SourceMapping):
         """
             list(Expression): List of the expressions
         """
-        expressions = [n.expression for n in self.nodes]
-        expressions = [e for e in expressions if e]
-        return expressions
+        if self._expressions is None:
+            expressions = [n.expression for n in self.nodes]
+            expressions = [e for e in expressions if e]
+            self._expressions = expressions
+        return self._expressions
+
+    @property
+    def slithir_operations(self):
+        """
+            list(Operation): List of the slithir operations
+        """
+        if self._slithir_operations is None:
+            operations = [n.irs for n in self.nodes]
+            operations = [item for sublist in operations for item in sublist if item]
+            self._slithir_operations = operations
+        return self._slithir_operations
 
     @property
     def signature(self):
@@ -506,27 +546,66 @@ class Function(ChildContract, SourceMapping):
     def all_state_variables_read(self):
         """ recursive version of variables_read
         """
-        return self._explore_functions(lambda x: x.state_variables_read)
+        if self._all_state_variables_read is None:
+            self._all_state_variables_read = self._explore_functions(
+                lambda x: x.state_variables_read)
+        return self._all_state_variables_read
 
     def all_solidity_variables_read(self):
         """ recursive version of solidity_read
         """
-        return self._explore_functions(lambda x: x.solidity_variables_read)
+        if self._all_solidity_variables_read is None:
+            self._all_solidity_variables_read = self._explore_functions(
+                lambda x: x.solidity_variables_read)
+        return self._all_solidity_variables_read
 
     def all_expressions(self):
         """ recursive version of variables_read
         """
-        return self._explore_functions(lambda x: x.expressions)
+        if self._all_expressions is None:
+            self._all_expressions = self._explore_functions(lambda x: x.expressions)
+        return self._all_expressions
+
+    def all_slithir_operations(self):
+        """
+        """
+        if self._all_slithir_operations is None:
+            self._all_slithir_operations = self._explore_functions(lambda x: x.slithir_operations)
+        return self._all_slithir_operations
 
     def all_state_variables_written(self):
         """ recursive version of variables_written
         """
-        return self._explore_functions(lambda x: x.state_variables_written)
+        if self._all_state_variables_written is None:
+            self._all_state_variables_written = self._explore_functions(
+                lambda x: x.state_variables_written)
+        return self._all_state_variables_written
 
     def all_internal_calls(self):
         """ recursive version of internal_calls
         """
-        return self._explore_functions(lambda x: x.internal_calls)
+        if self._all_internals_calls is None:
+            self._all_internals_calls = self._explore_functions(lambda x: x.internal_calls)
+        return self._all_internals_calls
+
+    def all_low_level_calls(self):
+        """ recursive version of low_level calls
+        """
+        if self._all_low_level_calls is None:
+            self._all_low_level_calls = self._explore_functions(lambda x: x.low_level_calls)
+        return self._all_low_level_calls
+
+    def all_high_level_calls(self):
+        """ recursive version of high_level calls
+        """
+        if self._all_high_level_calls is None:
+            self._all_high_level_calls = self._explore_functions(lambda x: x.high_level_calls)
+        return self._all_high_level_calls
+
+    @staticmethod
+    def _explore_func_cond_read(func, include_loop):
+        ret = [n.state_variables_read for n in func.nodes if n.is_conditional(include_loop)]
+        return [item for sublist in ret for item in sublist]
 
     def all_conditional_state_variables_read(self, include_loop=True):
         """
@@ -535,10 +614,25 @@ class Function(ChildContract, SourceMapping):
             Over approximate and also return index access
             It won't work if the variable is assigned to a temp variable
         """
-        def _explore_func(func):
-            ret = [n.state_variables_read for n in func.nodes if n.is_conditional(include_loop)]
-            return [item for sublist in ret for item in sublist]
-        return self._explore_functions(lambda x: _explore_func(x))
+        if self._all_conditional_state_variables_read is None:
+            self._all_conditional_state_variables_read = self._explore_functions(
+                lambda x: self._explore_func_cond_read(x,
+                                                       include_loop))
+        return self._all_conditional_state_variables_read
+
+    @staticmethod
+    def _solidity_variable_in_binary(node):
+        from slither.slithir.operations.binary import Binary
+        ret = []
+        for ir in node.irs:
+            if isinstance(ir, Binary):
+                ret += ir.read
+        return [var for var in ret if isinstance(var, SolidityVariable)]
+
+    @staticmethod
+    def _explore_func_conditional(func, f, include_loop):
+        ret = [f(n) for n in func.nodes if n.is_conditional(include_loop)]
+        return [item for sublist in ret for item in sublist]
 
     def all_conditional_solidity_variables_read(self, include_loop=True):
         """
@@ -548,17 +642,26 @@ class Function(ChildContract, SourceMapping):
             Assumption: the solidity vars are used directly in the conditional node
             It won't work if the variable is assigned to a temp variable
         """
-        from slither.slithir.operations.binary import Binary
-        def _solidity_variable_in_node(node):
-            ret = []
-            for ir in node.irs:
-                if isinstance(ir, Binary):
-                    ret += ir.read
-            return [var for var in ret if isinstance(var, SolidityVariable)]
-        def _explore_func(func, f):
-            ret = [f(n) for n in func.nodes if n.is_conditional(include_loop)]
-            return [item for sublist in ret for item in sublist]
-        return self._explore_functions(lambda x: _explore_func(x, _solidity_variable_in_node))
+        if self._all_conditional_solidity_variables_read is None:
+            self._all_conditional_solidity_variables_read = self._explore_functions(
+                lambda x: self._explore_func_conditional(x,
+                                                         self._solidity_variable_in_binary,
+                                                         include_loop))
+        return self._all_conditional_solidity_variables_read
+
+    @staticmethod
+    def _solidity_variable_in_internal_calls(node):
+        from slither.slithir.operations.internal_call import InternalCall
+        ret = []
+        for ir in node.irs:
+            if isinstance(ir, InternalCall):
+                ret += ir.read
+        return [var for var in ret if isinstance(var, SolidityVariable)]
+
+    @staticmethod
+    def _explore_func_nodes(func, f):
+        ret = [f(n) for n in func.nodes]
+        return [item for sublist in ret for item in sublist]
 
     def all_solidity_variables_used_as_args(self):
         """
@@ -567,17 +670,10 @@ class Function(ChildContract, SourceMapping):
             Use of the IR to filter index access
             Used to catch check(msg.sender)
         """
-        from slither.slithir.operations.internal_call import InternalCall
-        def _solidity_variable_in_node(node):
-            ret = []
-            for ir in node.irs:
-                if isinstance(ir, InternalCall):
-                    ret += ir.read
-            return [var for var in ret if isinstance(var, SolidityVariable)]
-        def _explore_func(func, f):
-            ret = [f(n) for n in func.nodes]
-            return [item for sublist in ret for item in sublist]
-        return self._explore_functions(lambda x: _explore_func(x, _solidity_variable_in_node))
+        if self._all_solidity_variables_used_as_args is None:
+            self._all_solidity_variables_used_as_args = self._explore_functions(
+                lambda x: self._explore_func_nodes(x, self._solidity_variable_in_internal_calls))
+        return self._all_solidity_variables_used_as_args
 
     def is_reading(self, variable):
         """

slither/core/declarations/solidity_variables.py

@@ -55,7 +55,9 @@ SOLIDITY_FUNCTIONS = {"gasleft()":['uint256'],
                       "abi.encode()":['bytes'],
                       "abi.encodePacked()":['bytes'],
                       "abi.encodeWithSelector()":["bytes"],
-                      "abi.encodeWithSignature()":["bytes"]}
+                      "abi.encodeWithSignature()":["bytes"],
+                      # abi.decode returns an a list arbitrary types
+                      "abi.decode()":[]}
 
 def solidity_function_signature(name):
     """

slither/core/variables/event_variable.py

@@ -1,5 +1,16 @@
 from .variable import Variable
 from slither.core.children.child_event import ChildEvent
 
-class EventVariable(ChildEvent, Variable): pass
+class EventVariable(ChildEvent, Variable):
+    def __init__(self):
+        super(EventVariable, self).__init__()
+        self._indexed = False
+
+    @property
+    def indexed(self):
+        """
+        Indicates whether the event variable is indexed in the bloom filter.
+        :return: Returns True if the variable is indexed in bloom filter, False otherwise.
+        """
+        return self._indexed
 

slither/detectors/reentrancy/reentrancy.py

@@ -0,0 +1,189 @@
+""""
+    Re-entrancy detection
+
+    Based on heuristics, it may lead to FP and FN
+    Iterate over all the nodes of the graph until reaching a fixpoint
+"""
+
+from slither.core.cfg.node import NodeType
+from slither.core.declarations import Function, SolidityFunction
+from slither.core.expressions import UnaryOperation, UnaryOperationType
+from slither.detectors.abstract_detector import (AbstractDetector,
+                                                 DetectorClassification)
+from slither.slithir.operations import (HighLevelCall, LowLevelCall,
+                                        LibraryCall,
+                                        Send, Transfer)
+
+def union_dict(d1, d2):
+    d3 = {k: d1.get(k, []) + d2.get(k, []) for k in set(list(d1.keys()) + list(d2.keys()))}
+    return d3
+
+def dict_are_equal(d1, d2):
+    if set(list(d1.keys())) != set(list(d2.keys())):
+        return False
+    return all(set(d1[k]) == set(d2[k]) for k in d1.keys())
+
+class Reentrancy(AbstractDetector):
+# This detector is not meant to be registered
+# It is inherited by reentrancy variantsœ
+#    ARGUMENT = 'reentrancy'
+#    HELP = 'Reentrancy vulnerabilities'
+#    IMPACT = DetectorClassification.HIGH
+#    CONFIDENCE = DetectorClassification.HIGH
+
+    KEY = 'REENTRANCY'
+
+    @staticmethod
+    def _can_callback(irs):
+        """
+            Detect if the node contains a call that can
+            be used to re-entrance
+
+            Consider as valid target:
+            - low level call
+            - high level call
+
+            Do not consider Send/Transfer as there is not enough gas
+        """
+        for ir in irs:
+            if isinstance(ir, LowLevelCall):
+                return True
+            if isinstance(ir, HighLevelCall) and not isinstance(ir, LibraryCall):
+                return True
+        return False
+
+    @staticmethod
+    def _can_send_eth(irs):
+        """
+            Detect if the node can send eth
+        """
+        for ir in irs:
+            if isinstance(ir, (HighLevelCall, LowLevelCall, Transfer, Send)):
+                if ir.call_value:
+                    return True
+        return False
+
+    def _filter_if(self, node):
+        """
+            Check if the node is a condtional node where
+            there is an external call checked
+            Heuristic:
+                - The call is a IF node
+                - It contains a, external call
+                - The condition is the negation (!)
+
+            This will work only on naive implementation
+        """
+        return isinstance(node.expression, UnaryOperation)\
+            and node.expression.type == UnaryOperationType.BANG
+
+    def _explore(self, node, visited, skip_father=None):
+        """
+            Explore the CFG and look for re-entrancy
+            Heuristic: There is a re-entrancy if a state variable is written
+                        after an external call
+
+            node.context will contains the external calls executed
+            It contains the calls executed in father nodes
+
+            if node.context is not empty, and variables are written, a re-entrancy is possible
+        """
+        if node in visited:
+            return
+
+        visited = visited + [node]
+
+        # First we add the external calls executed in previous nodes
+        # send_eth returns the list of calls sending value
+        # calls returns the list of calls that can callback
+        # read returns the variable read
+        # read_prior_calls returns the variable read prior a call
+        fathers_context = {'send_eth':[], 'calls':[], 'read':[], 'read_prior_calls':{}}
+
+        for father in node.fathers:
+            if self.KEY in father.context:
+                fathers_context['send_eth'] += [s for s in father.context[self.KEY]['send_eth'] if s!=skip_father]
+                fathers_context['calls'] += [c for c in father.context[self.KEY]['calls'] if c!=skip_father]
+                fathers_context['read'] += father.context[self.KEY]['read']
+                fathers_context['read_prior_calls'] = union_dict(fathers_context['read_prior_calls'], father.context[self.KEY]['read_prior_calls'])
+
+        # Exclude path that dont bring further information
+        if node in self.visited_all_paths:
+            if all(call in self.visited_all_paths[node]['calls'] for call in fathers_context['calls']):
+                if all(send in self.visited_all_paths[node]['send_eth'] for send in fathers_context['send_eth']):
+                    if all(read in self.visited_all_paths[node]['read'] for read in fathers_context['read']):
+                        if dict_are_equal(self.visited_all_paths[node]['read_prior_calls'], fathers_context['read_prior_calls']):
+                            return
+        else:
+            self.visited_all_paths[node] = {'send_eth':[], 'calls':[], 'read':[], 'read_prior_calls':{}}
+
+        self.visited_all_paths[node]['send_eth'] = list(set(self.visited_all_paths[node]['send_eth'] + fathers_context['send_eth']))
+        self.visited_all_paths[node]['calls'] = list(set(self.visited_all_paths[node]['calls'] + fathers_context['calls']))
+        self.visited_all_paths[node]['read'] = list(set(self.visited_all_paths[node]['read'] + fathers_context['read']))
+        self.visited_all_paths[node]['read_prior_calls'] = union_dict(self.visited_all_paths[node]['read_prior_calls'], fathers_context['read_prior_calls'])
+
+        node.context[self.KEY] = fathers_context
+
+        state_vars_read = node.state_variables_read
+
+        # All the state variables written
+        state_vars_written = node.state_variables_written
+        slithir_operations = []
+        # Add the state variables written in internal calls
+        for internal_call in node.internal_calls:
+            # Filter to Function, as internal_call can be a solidity call
+            if isinstance(internal_call, Function):
+                state_vars_written += internal_call.all_state_variables_written()
+                state_vars_read += internal_call.all_state_variables_read()
+                slithir_operations += internal_call.all_slithir_operations()
+
+        contains_call = False
+        node.context[self.KEY]['written'] = state_vars_written
+        if self._can_callback(node.irs + slithir_operations):
+            node.context[self.KEY]['calls'] = list(set(node.context[self.KEY]['calls'] + [node]))
+            node.context[self.KEY]['read_prior_calls'][node] = list(set(node.context[self.KEY]['read_prior_calls'].get(node, []) + node.context[self.KEY]['read']+ state_vars_read))
+            contains_call = True
+        if self._can_send_eth(node.irs + slithir_operations):
+            node.context[self.KEY]['send_eth'] = list(set(node.context[self.KEY]['send_eth'] + [node]))
+
+        node.context[self.KEY]['read'] = list(set(node.context[self.KEY]['read'] + state_vars_read))
+
+        sons = node.sons
+        if contains_call and node.type in [NodeType.IF, NodeType.IFLOOP]:
+            if self._filter_if(node):
+                son = sons[0]
+                self._explore(son, visited, node)
+                sons = sons[1:]
+            else:
+                son = sons[1]
+                self._explore(son, visited, node)
+                sons = [sons[0]]
+
+
+        for son in sons:
+            self._explore(son, visited)
+
+    def detect_reentrancy(self, contract):
+        """
+        """
+        for function in contract.functions_and_modifiers_not_inherited:
+            if function.is_implemented:
+                if self.KEY in function.context:
+                    continue
+                self._explore(function.entry_point, [])
+                function.context[self.KEY] = True
+
+    def detect(self):
+        """
+        """
+        # if a node was already visited by another path
+        # we will only explore it if the traversal brings
+        # new variables written
+        # This speedup the exploration through a light fixpoint
+        # Its particular useful on 'complex' functions with several loops and conditions
+        self.visited_all_paths = {}
+
+        for c in self.contracts:
+            self.detect_reentrancy(c)
+
+        return []

slither/detectors/reentrancy/reentrancy_benign.py

@@ -8,14 +8,15 @@
 from slither.core.cfg.node import NodeType
 from slither.core.declarations import Function, SolidityFunction
 from slither.core.expressions import UnaryOperation, UnaryOperationType
-from slither.detectors.abstract_detector import (AbstractDetector,
-                                                 DetectorClassification)
+from slither.detectors.abstract_detector import DetectorClassification
 from slither.visitors.expression.export_values import ExportValues
 from slither.slithir.operations import (HighLevelCall, LowLevelCall,
                                         LibraryCall,
                                         Send, Transfer)
 
-class ReentrancyBenign(AbstractDetector):
+from .reentrancy import Reentrancy
+
+class ReentrancyBenign(Reentrancy):
     ARGUMENT = 'reentrancy-benign'
     HELP = 'Benign reentrancy vulnerabilities'
     IMPACT = DetectorClassification.LOW
@@ -23,170 +24,36 @@ class ReentrancyBenign(AbstractDetector):
 
     WIKI = 'https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#reentrancy-vulnerabilities-2'
 
-    key = 'REENTRANCY-BENIGN'
-
-    @staticmethod
-    def _can_callback(node):
-        """
-            Detect if the node contains a call that can
-            be used to re-entrance
-
-            Consider as valid target:
-            - low level call
-            - high level call
-
-            Do not consider Send/Transfer as there is not enough gas
-        """
-        for ir in node.irs:
-            if isinstance(ir, LowLevelCall):
-                return True
-            if isinstance(ir, HighLevelCall) and not isinstance(ir, LibraryCall):
-                return True
-        return False
-
-    @staticmethod
-    def _can_send_eth(node):
-        """
-            Detect if the node can send eth
-        """
-        for ir in node.irs:
-            if isinstance(ir, (HighLevelCall, LowLevelCall, Transfer, Send)):
-                if ir.call_value:
-                    return True
-        return False
-
-    def _filter_if(self, node):
-        """
-            Check if the node is a condtional node where
-            there is an external call checked
-            Heuristic:
-                - The call is a IF node
-                - It contains a, external call
-                - The condition is the negation (!)
-
-            This will work only on naive implementation
-        """
-        return isinstance(node.expression, UnaryOperation)\
-            and node.expression.type == UnaryOperationType.BANG
-
-    def _explore(self, node, visited, skip_father=None):
-        """
-            Explore the CFG and look for re-entrancy
-            Heuristic: There is a re-entrancy if a state variable is written
-                        after an external call
-
-            node.context will contains the external calls executed
-            It contains the calls executed in father nodes
-
-            if node.context is not empty, and variables are written, a re-entrancy is possible
-        """
-        if node in visited:
-            return
-
-        visited = visited + [node]
-
-        # First we add the external calls executed in previous nodes
-        # send_eth returns the list of calls sending value
-        # calls returns the list of calls that can callback
-        # read returns the variable read
-        fathers_context = {'send_eth':[], 'calls':[], 'read':[]}
-
-        for father in node.fathers:
-            if self.key in father.context:
-                fathers_context['send_eth'] += [s for s in father.context[self.key]['send_eth'] if s!=skip_father]
-                fathers_context['calls'] += [c for c in father.context[self.key]['calls'] if c!=skip_father]
-                fathers_context['read'] += father.context[self.key]['read']
-
-        # Exclude path that dont bring further information
-        if node in self.visited_all_paths:
-            if all(call in self.visited_all_paths[node]['calls'] for call in fathers_context['calls']):
-                if all(send in self.visited_all_paths[node]['send_eth'] for send in fathers_context['send_eth']):
-                    if all(read in self.visited_all_paths[node]['read'] for read in fathers_context['read']):
-                        return
-        else:
-            self.visited_all_paths[node] = {'send_eth':[], 'calls':[], 'read':[]}
-
-        self.visited_all_paths[node]['send_eth'] = list(set(self.visited_all_paths[node]['send_eth'] + fathers_context['send_eth']))
-        self.visited_all_paths[node]['calls'] = list(set(self.visited_all_paths[node]['calls'] + fathers_context['calls']))
-        self.visited_all_paths[node]['read'] = list(set(self.visited_all_paths[node]['read'] + fathers_context['read']))
-
-        node.context[self.key] = fathers_context
-
-        state_vars_read = node.state_variables_read
-
-        # All the state variables written
-        state_vars_written = node.state_variables_written
-        # Add the state variables written in internal calls
-        for internal_call in node.internal_calls:
-            # Filter to Function, as internal_call can be a solidity call
-            if isinstance(internal_call, Function):
-                state_vars_written += internal_call.all_state_variables_written()
-                state_vars_read += internal_call.all_state_variables_read()
-
-        contains_call = False
-        if self._can_callback(node):
-            node.context[self.key]['calls'] = list(set(node.context[self.key]['calls'] + [node]))
-            contains_call = True
-        if self._can_send_eth(node):
-            node.context[self.key]['send_eth'] = list(set(node.context[self.key]['send_eth'] + [node]))
-
-        not_read_then_written = [(v, node) for v in state_vars_written if v not in node.context[self.key]['read']]
-        node.context[self.key]['read'] = list(set(node.context[self.key]['read'] + state_vars_read))
-
-        # If a state variables was read and is then written, there is a dangerous call and
-        # ether were sent
-        # We found a potential re-entrancy bug
-        if (not_read_then_written and
-                node.context[self.key]['calls']):
-            # calls are ordered
-            finding_key = (node.function,
-                           tuple(set(node.context[self.key]['calls'])),
-                           tuple(set(node.context[self.key]['send_eth'])))
-            finding_vars = not_read_then_written
-            if finding_key not in self.result:
-                self.result[finding_key] = []
-            self.result[finding_key] = list(set(self.result[finding_key] + finding_vars))
-
-        sons = node.sons
-        if contains_call and node.type in [NodeType.IF, NodeType.IFLOOP]:
-            if self._filter_if(node):
-                son = sons[0]
-                self._explore(son, visited, node)
-                sons = sons[1:]
-            else:
-                son = sons[1]
-                self._explore(son, visited, node)
-                sons = [sons[0]]
-
-
-        for son in sons:
-            self._explore(son, visited)
-
-    def detect_reentrancy(self, contract):
-        """
-        """
-        for function in contract.functions:
-            if function.is_implemented:
-                self._explore(function.entry_point, [])
+    def find_reentrancies(self):
+        result = {}
+        for contract in self.contracts:
+            for f in contract.functions_and_modifiers_not_inherited:
+                for node in f.nodes:
+                    if node.context[self.KEY]['calls']:
+                        not_read_then_written = [(v, node) for v in node.context[self.KEY]['written']
+                                                 if v not in node.context[self.KEY]['read']]
+                        if not_read_then_written:
+
+                            # calls are ordered
+                            finding_key = (node.function,
+                                           tuple(set(node.context[self.KEY]['calls'])),
+                                           tuple(set(node.context[self.KEY]['send_eth'])))
+                            finding_vars = not_read_then_written
+                            if finding_key not in result:
+                                result[finding_key] = []
+                            result[finding_key] = list(set(result[finding_key] + finding_vars))
+        return result
 
     def detect(self):
         """
         """
-        self.result = {}
 
-        # if a node was already visited by another path
-        # we will only explore it if the traversal brings
-        # new variables written
-        # This speedup the exploration through a light fixpoint
-        # Its particular useful on 'complex' functions with several loops and conditions
-        self.visited_all_paths = {}
-
-        for c in self.contracts:
-            self.detect_reentrancy(c)
+        super().detect()
+        reentrancies = self.find_reentrancies()
 
         results = []
 
-        result_sorted = sorted(list(self.result.items()), key=lambda x:x[0][0].name)
+        result_sorted = sorted(list(reentrancies.items()), key=lambda x:x[0][0].name)
         for (func, calls, send_eth), varsWritten in result_sorted:
             calls = list(set(calls))
             send_eth = list(set(send_eth))
@@ -195,7 +62,7 @@ class ReentrancyBenign(AbstractDetector):
             info += '\tExternal calls:\n'
             for call_info in calls:
                 info += '\t- {} ({})\n'.format(call_info.expression, call_info.source_mapping_str)
-            if calls != send_eth:
+            if calls != send_eth and send_eth:
                 info += '\tExternal calls sending eth:\n'
                 for call_info in send_eth:
                     info += '\t- {} ({})\n'.format(call_info.expression, call_info.source_mapping_str)

slither/detectors/reentrancy/reentrancy_eth.py

@@ -8,13 +8,14 @@
 from slither.core.cfg.node import NodeType
 from slither.core.declarations import Function, SolidityFunction
 from slither.core.expressions import UnaryOperation, UnaryOperationType
-from slither.detectors.abstract_detector import (AbstractDetector,
-                                                 DetectorClassification)
+from slither.detectors.abstract_detector import DetectorClassification
 from slither.slithir.operations import (HighLevelCall, LowLevelCall,
                                         LibraryCall,
                                         Send, Transfer)
 
-class ReentrancyEth(AbstractDetector):
+
+from .reentrancy import Reentrancy
+class ReentrancyEth(Reentrancy):
     ARGUMENT = 'reentrancy-eth'
     HELP = 'Reentrancy vulnerabilities (theft of ethers)'
     IMPACT = DetectorClassification.HIGH
@@ -24,175 +25,38 @@ class ReentrancyEth(AbstractDetector):
 
     key = 'REENTRANCY-ETHERS'
 
-    @staticmethod
-    def _can_callback(node):
-        """
-            Detect if the node contains a call that can
-            be used to re-entrance
-
-            Consider as valid target:
-            - low level call
-            - high level call
-
-            Do not consider Send/Transfer as there is not enough gas
-        """
-        for ir in node.irs:
-            if isinstance(ir, LowLevelCall):
-                return True
-            if isinstance(ir, HighLevelCall) and not isinstance(ir, LibraryCall):
-                return True
-        return False
-
-    @staticmethod
-    def _can_send_eth(node):
-        """
-            Detect if the node can send eth
-        """
-        for ir in node.irs:
-            if isinstance(ir, (HighLevelCall, LowLevelCall, Transfer, Send)):
-                if ir.call_value:
-                    return True
-        return False
-
-    def _filter_if(self, node):
-        """
-            Check if the node is a condtional node where
-            there is an external call checked
-            Heuristic:
-                - The call is a IF node
-                - It contains a, external call
-                - The condition is the negation (!)
-
-            This will work only on naive implementation
-        """
-        return isinstance(node.expression, UnaryOperation)\
-            and node.expression.type == UnaryOperationType.BANG
-
-    def _explore(self, node, visited, skip_father=None):
-        """
-            Explore the CFG and look for re-entrancy
-            Heuristic: There is a re-entrancy if a state variable is written
-                        after an external call
-
-            node.context will contains the external calls executed
-            It contains the calls executed in father nodes
-
-            if node.context is not empty, and variables are written, a re-entrancy is possible
-        """
-        if node in visited:
-            return
-
-        visited = visited + [node]
-
-        # First we add the external calls executed in previous nodes
-        # send_eth returns the list of calls sending value
-        # calls returns the list of calls that can callback
-        # read returns the variable read
-        fathers_context = {'send_eth':[], 'calls':[], 'read':[], 'read_prior_calls':[]}
-
-        for father in node.fathers:
-            if self.key in father.context:
-                fathers_context['send_eth'] += [s for s in father.context[self.key]['send_eth'] if s!=skip_father]
-                fathers_context['calls'] += [c for c in father.context[self.key]['calls'] if c!=skip_father]
-                fathers_context['read'] += father.context[self.key]['read']
-                fathers_context['read_prior_calls'] += father.context[self.key]['read_prior_calls']
-
-        # Exclude path that dont bring further information
-        if node in self.visited_all_paths:
-            if all(call in self.visited_all_paths[node]['calls'] for call in fathers_context['calls']):
-                if all(send in self.visited_all_paths[node]['send_eth'] for send in fathers_context['send_eth']):
-                    if all(read in self.visited_all_paths[node]['read'] for read in fathers_context['read']):
-                        if all(read in self.visited_all_paths[node]['read_prior_calls'] for read in fathers_context['read_prior_calls']):
-                            return
-        else:
-            self.visited_all_paths[node] = {'send_eth':[], 'calls':[], 'read':[], 'read_prior_calls':[]}
-
-        self.visited_all_paths[node]['send_eth'] = list(set(self.visited_all_paths[node]['send_eth'] + fathers_context['send_eth']))
-        self.visited_all_paths[node]['calls'] = list(set(self.visited_all_paths[node]['calls'] + fathers_context['calls']))
-        self.visited_all_paths[node]['read'] = list(set(self.visited_all_paths[node]['read'] + fathers_context['read']))
-        self.visited_all_paths[node]['read_prior_calls'] = list(set(self.visited_all_paths[node]['read_prior_calls'] + fathers_context['read_prior_calls']))
-
-        node.context[self.key] = fathers_context
-
-        state_vars_read = node.state_variables_read
-
-        # All the state variables written
-        state_vars_written = node.state_variables_written
-        # Add the state variables written in internal calls
-        for internal_call in node.internal_calls:
-            # Filter to Function, as internal_call can be a solidity call
-            if isinstance(internal_call, Function):
-                state_vars_written += internal_call.all_state_variables_written()
-                state_vars_read += internal_call.all_state_variables_read()
-
-        contains_call = False
-        if self._can_callback(node):
-            node.context[self.key]['calls'] = list(set(node.context[self.key]['calls'] + [node]))
-            node.context[self.key]['read_prior_calls'] = list(set(node.context[self.key]['read_prior_calls'] + node.context[self.key]['read']+ state_vars_read))
-            node.context[self.key]['read'] = []
-            contains_call = True
-        if self._can_send_eth(node):
-            node.context[self.key]['send_eth'] = list(set(node.context[self.key]['send_eth'] + [node]))
-
-
-        read_then_written = [(v, node) for v in state_vars_written if v in node.context[self.key]['read_prior_calls']]
-
-        node.context[self.key]['read'] = list(set(node.context[self.key]['read'] + state_vars_read))
-        # If a state variables was read and is then written, there is a dangerous call and
-        # ether were sent
-        # We found a potential re-entrancy bug
-        if (read_then_written and
-                node.context[self.key]['calls'] and
-                node.context[self.key]['send_eth']):
-            # calls are ordered
-            finding_key = (node.function,
-                           tuple(set(node.context[self.key]['calls'])),
-                           tuple(set(node.context[self.key]['send_eth'])))
-            finding_vars = read_then_written
-            if finding_key not in self.result:
-                self.result[finding_key] = []
-            self.result[finding_key] = list(set(self.result[finding_key] + finding_vars))
-
-        sons = node.sons
-        if contains_call and node.type in [NodeType.IF, NodeType.IFLOOP]:
-            if self._filter_if(node):
-                son = sons[0]
-                self._explore(son, visited, node)
-                sons = sons[1:]
-            else:
-                son = sons[1]
-                self._explore(son, visited, node)
-                sons = [sons[0]]
-
-
-        for son in sons:
-            self._explore(son, visited)
-
-    def detect_reentrancy(self, contract):
-        """
-        """
-        for function in contract.functions:
-            if function.is_implemented:
-                self._explore(function.entry_point, [])
+    def find_reentrancies(self):
+        result = {}
+        for contract in self.contracts:
+            for f in contract.functions_and_modifiers_not_inherited:
+                for node in f.nodes:
+                    if node.context[self.KEY]['calls'] and node.context[self.KEY]['send_eth']:
+                        read_then_written = []
+                        for c in node.context[self.KEY]['calls']:
+                            read_then_written += [(v, node) for v in node.context[self.KEY]['written']
+                                                 if v in node.context[self.KEY]['read_prior_calls'][c]]
+
+                        if read_then_written:
+                            # calls are ordered
+                            finding_key = (node.function,
+                                           tuple(set(node.context[self.KEY]['calls'])),
+                                           tuple(set(node.context[self.KEY]['send_eth'])))
+                            finding_vars = read_then_written
+                            if finding_key not in result:
+                                result[finding_key] = []
+                            result[finding_key] = list(set(result[finding_key] + finding_vars))
+        return result
 
     def detect(self):
         """
         """
-        self.result = {}
-
-        # if a node was already visited by another path
-        # we will only explore it if the traversal brings
-        # new variables written
-        # This speedup the exploration through a light fixpoint
-        # Its particular useful on 'complex' functions with several loops and conditions
-        self.visited_all_paths = {}
+        super().detect()
 
-        for c in self.contracts:
-            self.detect_reentrancy(c)
+        reentrancies = self.find_reentrancies()
 
         results = []
 
-        result_sorted = sorted(list(self.result.items()), key=lambda x:x[0][0].name)
+        result_sorted = sorted(list(reentrancies.items()), key=lambda x:x[0][0].name)
         for (func, calls, send_eth), varsWritten in result_sorted:
             calls = list(set(calls))
             send_eth = list(set(send_eth))

slither/detectors/reentrancy/reentrancy_read_before_write.py

@@ -8,14 +8,16 @@
 from slither.core.cfg.node import NodeType
 from slither.core.declarations import Function, SolidityFunction
 from slither.core.expressions import UnaryOperation, UnaryOperationType
-from slither.detectors.abstract_detector import (AbstractDetector,
-                                                 DetectorClassification)
+from slither.detectors.abstract_detector import DetectorClassification
 from slither.visitors.expression.export_values import ExportValues
 from slither.slithir.operations import (HighLevelCall, LowLevelCall,
                                         LibraryCall,
                                         Send, Transfer)
 
-class ReentrancyReadBeforeWritten(AbstractDetector):
+
+from .reentrancy import Reentrancy
+
+class ReentrancyReadBeforeWritten(Reentrancy):
     ARGUMENT = 'reentrancy-no-eth'
     HELP = 'Reentrancy vulnerabilities (no theft of ethers)'
     IMPACT = DetectorClassification.MEDIUM
@@ -23,174 +25,38 @@ class ReentrancyReadBeforeWritten(AbstractDetector):
 
     WIKI = 'https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#reentrancy-vulnerabilities-1'
 
-    key = 'REENTRANCY-NO-ETHER'
-
-    @staticmethod
-    def _can_callback(node):
-        """
-            Detect if the node contains a call that can
-            be used to re-entrance
-
-            Consider as valid target:
-            - low level call
-            - high level call
-
-            Do not consider Send/Transfer as there is not enough gas
-        """
-        for ir in node.irs:
-            if isinstance(ir, LowLevelCall):
-                return True
-            if isinstance(ir, HighLevelCall) and not isinstance(ir, LibraryCall):
-                return True
-        return False
-
-    @staticmethod
-    def _can_send_eth(node):
-        """
-            Detect if the node can send eth
-        """
-        for ir in node.irs:
-            if isinstance(ir, (HighLevelCall, LowLevelCall, Transfer, Send)):
-                if ir.call_value:
-                    return True
-        return False
-
-    def _filter_if(self, node):
-        """
-            Check if the node is a condtional node where
-            there is an external call checked
-            Heuristic:
-                - The call is a IF node
-                - It contains a, external call
-                - The condition is the negation (!)
-
-            This will work only on naive implementation
-        """
-        return isinstance(node.expression, UnaryOperation)\
-            and node.expression.type == UnaryOperationType.BANG
-
-    def _explore(self, node, visited, skip_father=None):
-        """
-            Explore the CFG and look for re-entrancy
-            Heuristic: There is a re-entrancy if a state variable is written
-                        after an external call
-
-            node.context will contains the external calls executed
-            It contains the calls executed in father nodes
-
-            if node.context is not empty, and variables are written, a re-entrancy is possible
-        """
-        if node in visited:
-            return
-
-        visited = visited + [node]
-
-        # First we add the external calls executed in previous nodes
-        # send_eth returns the list of calls sending value
-        # calls returns the list of calls that can callback
-        # read returns the variable read
-        fathers_context = {'send_eth':[], 'calls':[], 'read':[], 'read_prior_calls':[]}
-
-        for father in node.fathers:
-            if self.key in father.context:
-                fathers_context['send_eth'] += [s for s in father.context[self.key]['send_eth'] if s!=skip_father]
-                fathers_context['calls'] += [c for c in father.context[self.key]['calls'] if c!=skip_father]
-                fathers_context['read'] += father.context[self.key]['read']
-                fathers_context['read_prior_calls'] += father.context[self.key]['read_prior_calls']
-
-        # Exclude path that dont bring further information
-        if node in self.visited_all_paths:
-            if all(call in self.visited_all_paths[node]['calls'] for call in fathers_context['calls']):
-                if all(send in self.visited_all_paths[node]['send_eth'] for send in fathers_context['send_eth']):
-                    if all(read in self.visited_all_paths[node]['read'] for read in fathers_context['read']):
-                        if all(read in self.visited_all_paths[node]['read_prior_calls'] for read in fathers_context['read_prior_calls']):
-                            return
-        else:
-            self.visited_all_paths[node] = {'send_eth':[], 'calls':[], 'read':[], 'read_prior_calls':[]}
-
-        self.visited_all_paths[node]['send_eth'] = list(set(self.visited_all_paths[node]['send_eth'] + fathers_context['send_eth']))
-        self.visited_all_paths[node]['calls'] = list(set(self.visited_all_paths[node]['calls'] + fathers_context['calls']))
-        self.visited_all_paths[node]['read'] = list(set(self.visited_all_paths[node]['read'] + fathers_context['read']))
-        self.visited_all_paths[node]['read_prior_calls'] = list(set(self.visited_all_paths[node]['read_prior_calls'] + fathers_context['read_prior_calls']))
-
-        node.context[self.key] = fathers_context
-
-        state_vars_read = node.state_variables_read
-        # All the state variables written
-        state_vars_written = node.state_variables_written
-        # Add the state variables written in internal calls
-        for internal_call in node.internal_calls:
-            # Filter to Function, as internal_call can be a solidity call
-            if isinstance(internal_call, Function):
-                state_vars_written += internal_call.all_state_variables_written()
-                state_vars_read += internal_call.all_state_variables_read()
-
-        contains_call = False
-        if self._can_callback(node):
-            node.context[self.key]['calls'] = list(set(node.context[self.key]['calls'] + [node]))
-            node.context[self.key]['read_prior_calls'] = list(set(node.context[self.key]['read_prior_calls'] + node.context[self.key]['read'] + state_vars_read))
-            node.context[self.key]['read'] = []
-            contains_call = True
-        if self._can_send_eth(node):
-            node.context[self.key]['send_eth'] = list(set(node.context[self.key]['send_eth'] + [node]))
-
-        read_then_written = [(v, node) for v in state_vars_written if v in node.context[self.key]['read_prior_calls']]
-
-        node.context[self.key]['read'] = list(set(node.context[self.key]['read'] + state_vars_read))
-        # If a state variables was read and is then written, there is a dangerous call and
-        # ether were sent
-        # We found a potential re-entrancy bug
-        if (read_then_written and
-                node.context[self.key]['calls'] and
-                not node.context[self.key]['send_eth']):
-            # calls are ordered
-            finding_key = (node.function,
-                           tuple(set(node.context[self.key]['calls'])))
-            finding_vars = read_then_written
-            if finding_key not in self.result:
-                self.result[finding_key] = []
-            self.result[finding_key] = list(set(self.result[finding_key] + finding_vars))
-
-        sons = node.sons
-        if contains_call and node.type in [NodeType.IF, NodeType.IFLOOP]:
-            if self._filter_if(node):
-                son = sons[0]
-                self._explore(son, visited, node)
-                sons = sons[1:]
-            else:
-                son = sons[1]
-                self._explore(son, visited, node)
-                sons = [sons[0]]
-
-
-        for son in sons:
-            self._explore(son, visited)
-
-    def detect_reentrancy(self, contract):
-        """
-        """
-        for function in contract.functions:
-            if function.is_implemented:
-                self._explore(function.entry_point, [])
+    def find_reentrancies(self):
+        result = {}
+        for contract in self.contracts:
+            for f in contract.functions_and_modifiers_not_inherited:
+                for node in f.nodes:
+                    if node.context[self.KEY]['calls'] and not node.context[self.KEY]['send_eth']:
+                        read_then_written = []
+                        for c in node.context[self.KEY]['calls']:
+                            read_then_written += [(v, node) for v in node.context[self.KEY]['written']
+                                                  if v in node.context[self.KEY]['read_prior_calls'][c]]
+
+                        # We found a potential re-entrancy bug
+                        if read_then_written:
+                            # calls are ordered
+                            finding_key = (node.function,
+                                           tuple(set(node.context[self.KEY]['calls'])))
+                            finding_vars = read_then_written
+                            if finding_key not in self.result:
+                                result[finding_key] = []
+                            result[finding_key] = list(set(result[finding_key] + finding_vars))
+        return result
 
     def detect(self):
         """
         """
-        self.result = {}
-
-        # if a node was already visited by another path
-        # we will only explore it if the traversal brings
-        # new variables written
-        # This speedup the exploration through a light fixpoint
-        # Its particular useful on 'complex' functions with several loops and conditions
-        self.visited_all_paths = {}
 
-        for c in self.contracts:
-            self.detect_reentrancy(c)
+        super().detect()
+        reentrancies = self.find_reentrancies()
 
         results = []
 
-        result_sorted = sorted(list(self.result.items()), key=lambda x:x[0][0].name)
+        result_sorted = sorted(list(reentrancies.items()), key=lambda x:x[0][0].name)
         for (func, calls), varsWritten in result_sorted:
             calls = list(set(calls))
             info = 'Reentrancy in {}.{} ({}):\n'

slither/slithir/convert.py

@@ -199,7 +199,8 @@ def convert_to_low_level(ir):
     elif ir.destination.name ==  'abi' and ir.function_name in ['encode',
                                                                 'encodePacked',
                                                                 'encodeWithSelector',
-                                                                'encodeWithSignature']:
+                                                                'encodeWithSignature',
+                                                                'decode']:
 
         call = SolidityFunction('abi.{}()'.format(ir.function_name))
         new_ir = SolidityCall(call, ir.nbr_arguments, ir.lvalue, ir.type_call)

slither/solc_parsing/declarations/contract.py

@@ -91,12 +91,62 @@ class ContractSolc04(Contract):
         self.linearizedBaseContracts = attributes['linearizedBaseContracts']
         self.fullyImplemented = attributes['fullyImplemented']
 
+        # Parse base contract information
+        self._parse_base_contract_info()
+
         # trufle does some re-mapping of id
         if 'baseContracts' in self._data:
             for elem in self._data['baseContracts']:
                 if elem['nodeType'] == 'InheritanceSpecifier':
                     self._remapping[elem['baseName']['referencedDeclaration']] = elem['baseName']['name']
 
+    def _parse_base_contract_info(self):
+        # Parse base contracts (immediate, non-linearized)
+        self.baseContracts = []
+        self.baseConstructorContractsCalled = []
+        if self.is_compact_ast:
+            # Parse base contracts + constructors in compact-ast
+            if 'baseContracts' in self._data:
+                for base_contract in self._data['baseContracts']:
+                    if base_contract['nodeType'] != 'InheritanceSpecifier':
+                        continue
+                    if 'baseName' not in base_contract or 'referencedDeclaration' not in base_contract['baseName']:
+                        continue
+
+                    # Obtain our contract reference and add it to our base contract list
+                    referencedDeclaration = base_contract['baseName']['referencedDeclaration']
+                    self.baseContracts.append(referencedDeclaration)
+
+                    # If we have defined arguments in our arguments object, this is a constructor invocation.
+                    # (note: 'arguments' can be [], which is not the same as None. [] implies a constructor was
+                    #  called with no arguments, while None implies no constructor was called).
+                    if 'arguments' in base_contract and base_contract['arguments'] is not None:
+                        self.baseConstructorContractsCalled.append(referencedDeclaration)
+        else:
+            # Parse base contracts + constructors in legacy-ast
+            if 'children' in self._data:
+                for base_contract in self._data['children']:
+                    if base_contract['name'] != 'InheritanceSpecifier':
+                        continue
+                    if 'children' not in base_contract or len(base_contract['children']) == 0:
+                        continue
+                    # Obtain all items for this base contract specification (base contract, followed by arguments)
+                    base_contract_items = base_contract['children']
+                    if 'name' not in base_contract_items[0] or base_contract_items[0]['name'] != 'UserDefinedTypeName':
+                        continue
+                    if 'attributes' not in base_contract_items[0] or 'referencedDeclaration' not in \
+                            base_contract_items[0]['attributes']:
+                        continue
+
+                    # Obtain our contract reference and add it to our base contract list
+                    referencedDeclaration = base_contract_items[0]['attributes']['referencedDeclaration']
+                    self.baseContracts.append(referencedDeclaration)
+
+                    # If we have an 'attributes'->'arguments' which is None, this is not a constructor call.
+                    if 'attributes' not in base_contract or 'arguments' not in base_contract['attributes'] or \
+                            base_contract['attributes']['arguments'] is not None:
+                        self.baseConstructorContractsCalled.append(referencedDeclaration)
+
     def _parse_contract_items(self):
         if not self.get_children() in self._data: # empty contract
             return

slither/solc_parsing/declarations/function.py

@@ -26,6 +26,7 @@ from slither.solc_parsing.variables.variable_declaration import \
 from slither.utils.expression_manipulations import SplitTernaryExpression
 from slither.visitors.expression.export_values import ExportValues
 from slither.visitors.expression.has_conditional import HasConditional
+from slither.core.declarations.contract import Contract
 
 logger = logging.getLogger("FunctionSolc")
 
@@ -772,7 +773,11 @@ class FunctionSolc(Function):
     def _parse_modifier(self, modifier):
         m = parse_expression(modifier, self)
         self._expression_modifiers.append(m)
-        self._modifiers += [m for m in ExportValues(m).result() if isinstance(m, Function)]
+        for m in ExportValues(m).result():
+            if isinstance(m, Function):
+                self._modifiers.append(m)
+            elif isinstance(m, Contract):
+                self._explicit_base_constructor_calls.append(m)
 
 
     def analyze_params(self):

slither/solc_parsing/slitherSolc.py

@@ -159,20 +159,38 @@ class SlitherSolc(Slither):
         # Update of the inheritance 
         for contract in self._contractsNotParsed:
             # remove the first elem in linearizedBaseContracts as it is the contract itself
+            ancestors = []
             fathers = []
+            father_constructors = []
             try:
+                # Resolve linearized base contracts.
                 for i in contract.linearizedBaseContracts[1:]:
+                    if i in contract.remapping:
+                        ancestors.append(self.get_contract_from_name(contract.remapping[i]))
+                    else:
+                        ancestors.append(self._contracts_by_id[i])
+
+                # Resolve immediate base contracts
+                for i in contract.baseContracts:
                     if i in contract.remapping:
                         fathers.append(self.get_contract_from_name(contract.remapping[i]))
                     else:
                         fathers.append(self._contracts_by_id[i])
+
+                # Resolve immediate base constructor calls
+                for i in contract.baseConstructorContractsCalled:
+                    if i in contract.remapping:
+                        father_constructors.append(self.get_contract_from_name(contract.remapping[i]))
+                    else:
+                        father_constructors.append(self._contracts_by_id[i])
+
             except KeyError:
                 logger.error(red('A contract was not found, it is likely that your codebase contains muliple contracts with the same name'))
                 logger.error(red('Truffle does not handle this case during compilation'))
                 logger.error(red('Please read https://github.com/trailofbits/slither/wiki#keyerror-or-nonetype-error'))
                 logger.error(red('And update your code to remove the duplicate'))
                 exit(-1)
-            contract.setInheritance(fathers)
+            contract.setInheritance(ancestors, fathers, father_constructors)
 
         contracts_to_be_analyzed = self.contracts
 

slither/solc_parsing/variables/event_variable.py

@@ -2,4 +2,18 @@
 from .variable_declaration import VariableDeclarationSolc
 from slither.core.variables.event_variable import EventVariable
 
-class EventVariableSolc(VariableDeclarationSolc, EventVariable): pass
+class EventVariableSolc(VariableDeclarationSolc, EventVariable):
+
+    def _analyze_variable_attributes(self, attributes):
+        """
+        Analyze event variable attributes
+        :param attributes: The event variable attributes to parse.
+        :return: None
+        """
+
+        # Check for the indexed attribute
+        if 'indexed' in attributes:
+            self._indexed = attributes['indexed']
+
+        super(EventVariableSolc, self)._analyze_variable_attributes(attributes)
+

tests/expected_json/uninitialized_storage_pointer.uninitialized-storage.json

@@ -1 +1 @@
-[{"check": "uninitialized-storage", "impact": "High", "confidence": "High", "description": "st_bug in Uninitialized.func (tests/uninitialized_storage_pointer.sol#10) is a storage variable never initialiazed\n", "elements": [{"type": "variable", "name": "st_bug", "source_mapping": {"start": 171, "length": 9, "filename": "tests/uninitialized_storage_pointer.sol", "lines": [10]}}, {"type": "function", "name": "func", "source_mapping": {"start": 67, "length": 138, "filename": "tests/uninitialized_storage_pointer.sol", "lines": [7, 8, 9, 10, 11, 12]}, "contract": {"type": "contract", "name": "Uninitialized", "source_mapping": {"start": 0, "length": 212, "filename": "tests/uninitialized_storage_pointer.sol", "lines": [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14]}}}]}]
+[{"check": "uninitialized-storage", "impact": "High", "confidence": "High", "description": "st_bug in Uninitialized.func (tests/uninitialized_storage_pointer.sol#10) is a storage variable never initialiazed\n", "elements": [{"type": "variable", "name": "st_bug", "source_mapping": {"start": 171, "length": 9, "filename": "tests/uninitialized_storage_pointer.sol", "lines": [10]}}, {"type": "function", "name": "func", "source_mapping": {"start": 67, "length": 143, "filename": "tests/uninitialized_storage_pointer.sol", "lines": [7, 8, 9, 10, 11, 12]}, "contract": {"type": "contract", "name": "Uninitialized", "source_mapping": {"start": 0, "length": 217, "filename": "tests/uninitialized_storage_pointer.sol", "lines": [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14]}}}]}]

tests/uninitialized_storage_pointer.sol

@@ -8,7 +8,7 @@ contract Uninitialized{
         St st; // non init, but never read so its fine
         St memory st2;
         St st_bug;
-        st_bug.a;
+        st_bug.a += 1;
     }    
 
 }