Merge branch 'dev' into dev-source-mapping-refactor

slither/detectors/statements/unprotected_upgradeable.py

@@ -21,6 +21,14 @@ def _can_be_destroyed(contract) -> List[Function]:
     return targets
 
 
+def _has_initializer_modifier(functions: List[Function]) -> bool:
+    for f in functions:
+        for m in f.modifiers:
+            if m.name == "initializer":
+                return True
+    return False
+
+
 class UnprotectedUpgradeable(AbstractDetector):
 
     ARGUMENT = "unprotected-upgrade"
@@ -61,34 +69,38 @@ class UnprotectedUpgradeable(AbstractDetector):
 
         for contract in self.compilation_unit.contracts_derived:
             if contract.is_upgradeable:
-                functions_that_can_destroy = _can_be_destroyed(contract)
+                if not _has_initializer_modifier(contract.constructors):
-                if functions_that_can_destroy:
+                    functions_that_can_destroy = _can_be_destroyed(contract)
-                    initiliaze_functions = [f for f in contract.functions if f.name == "initialize"]
+                    if functions_that_can_destroy:
-                    vars_init_ = [
+                        initiliaze_functions = [
-                        init.all_state_variables_written() for init in initiliaze_functions
+                            f for f in contract.functions if f.name == "initialize"
-                    ]
+                        ]
-                    vars_init = [item for sublist in vars_init_ for item in sublist]
+
-
+                        vars_init_ = [
-                    vars_init_in_constructors_ = [
+                            init.all_state_variables_written() for init in initiliaze_functions
-                        f.all_state_variables_written() for f in contract.constructors
+                        ]
-                    ]
+                        vars_init = [item for sublist in vars_init_ for item in sublist]
-                    vars_init_in_constructors = [
+
-                        item for sublist in vars_init_in_constructors_ for item in sublist
+                        vars_init_in_constructors_ = [
-                    ]
+                            f.all_state_variables_written() for f in contract.constructors
-                    if vars_init and (set(vars_init) - set(vars_init_in_constructors)):
+                        ]
-                        info = (
+                        vars_init_in_constructors = [
-                            [
+                            item for sublist in vars_init_in_constructors_ for item in sublist
-                                contract,
+                        ]
-                                " is an upgradeable contract that does not protect its initiliaze functions: ",
+                        if vars_init and (set(vars_init) - set(vars_init_in_constructors)):
-                            ]
+                            info = (
-                            + initiliaze_functions
+                                [
-                            + [
+                                    contract,
-                                ". Anyone can delete the contract with: ",
+                                    " is an upgradeable contract that does not protect its initiliaze functions: ",
-                            ]
+                                ]
-                            + functions_that_can_destroy
+                                + initiliaze_functions
-                        )
+                                + [
-
+                                    ". Anyone can delete the contract with: ",
-                        res = self.generate_result(info)
+                                ]
-                        results.append(res)
+                                + functions_that_can_destroy
+                            )
+
+                            res = self.generate_result(info)
+                            results.append(res)
 
         return results

tests/detectors/unprotected-upgrade/0.4.25/Fixed.sol

@@ -21,7 +21,6 @@ contract Fixed is Initializable{
     }
 }
 
-
 contract Not_Upgradeable{
 }
 
@@ -36,4 +35,20 @@ contract UpgradeableNoDestruct is Initializable{
         require(owner == address(0));
         owner = msg.sender;
     }
+}
+
+contract Fixed2 is Initializable {
+    address owner;
+
+    constructor() public initializer {}
+
+    function initialize() external initializer {
+        require(owner == address(0));
+        owner = msg.sender;
+    }
+
+    function kill() external {
+        require(msg.sender == owner);
+        selfdestruct(owner);
+    }
 }

tests/detectors/unprotected-upgrade/0.5.16/Fixed.sol

@@ -21,7 +21,6 @@ contract Fixed is Initializable{
     }
 }
 
-
 contract Not_Upgradeable{
 }
 
@@ -36,4 +35,20 @@ contract UpgradeableNoDestruct is Initializable{
         require(owner == address(0));
         owner = msg.sender;
     }
+}
+
+contract Fixed2 is Initializable {
+    address payable owner;
+
+    constructor() public initializer {}
+
+    function initialize() external initializer {
+        require(owner == address(0));
+        owner = msg.sender;
+    }
+
+    function kill() external {
+        require(msg.sender == owner);
+        selfdestruct(owner);
+    }
 }

tests/detectors/unprotected-upgrade/0.6.11/Fixed.sol

@@ -36,4 +36,20 @@ contract UpgradeableNoDestruct is Initializable{
         require(owner == address(0));
         owner = msg.sender;
     }
+}
+
+contract Fixed2 is Initializable {
+    address payable owner;
+
+    constructor() public initializer {}
+
+    function initialize() external initializer {
+        require(owner == address(0));
+        owner = msg.sender;
+    }
+
+    function kill() external {
+        require(msg.sender == owner);
+        selfdestruct(owner);
+    }
 }

tests/detectors/unprotected-upgrade/0.7.6/Fixed.sol

@@ -3,7 +3,7 @@ import "./Initializable.sol";
 contract Fixed is Initializable{
     address payable owner;
 
-    constructor() public{
+    constructor() {
         owner = msg.sender;
     }
 
@@ -21,14 +21,13 @@ contract Fixed is Initializable{
     }
 }
 
-
 contract Not_Upgradeable{
 }
 
 contract UpgradeableNoDestruct is Initializable{
     address payable owner;
 
-    constructor() public{
+    constructor() {
         owner = msg.sender;
     }
 
@@ -36,4 +35,20 @@ contract UpgradeableNoDestruct is Initializable{
         require(owner == address(0));
         owner = msg.sender;
     }
+}
+
+contract Fixed2 is Initializable {
+    address payable owner;
+
+    constructor() initializer {}
+
+    function initialize() external initializer {
+        require(owner == address(0));
+        owner = msg.sender;
+    }
+
+    function kill() external {
+        require(msg.sender == owner);
+        selfdestruct(owner);
+    }
 }