mirror of https://github.com/crytic/slither
Merge pull request #725 from crytic/dev-unprotected-upgrade
Open source unprotected upgradepull/729/head
Feist Josselin
4 years ago
committed by
GitHub
commit
9bc6763c2a
@ -0,0 +1,89 @@ |
||||
from typing import List |
||||
|
||||
from slither.core.declarations import SolidityFunction, Function |
||||
from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification |
||||
from slither.slithir.operations import LowLevelCall, SolidityCall |
||||
|
||||
|
||||
def _can_be_destroyed(contract) -> List[Function]: |
||||
targets = [] |
||||
for f in contract.functions_entry_points: |
||||
for ir in f.all_slithir_operations(): |
||||
if ( |
||||
isinstance(ir, LowLevelCall) and ir.function_name in ["delegatecall", "codecall"] |
||||
) or ( |
||||
isinstance(ir, SolidityCall) |
||||
and ir.function |
||||
in [SolidityFunction("suicide(address)"), SolidityFunction("selfdestruct(address)")] |
||||
): |
||||
targets.append(f) |
||||
break |
||||
return targets |
||||
|
||||
|
||||
class UnprotectedUpgradeable(AbstractDetector): |
||||
|
||||
ARGUMENT = "unprotected-upgrade" |
||||
HELP = "Unprotected upgradeable contract" |
||||
IMPACT = DetectorClassification.HIGH |
||||
CONFIDENCE = DetectorClassification.HIGH |
||||
|
||||
WIKI = "https://github.com/crytic/slither/wiki/Detector-Documentation#unprotected-upgradeable-contract" |
||||
|
||||
WIKI_TITLE = "Unprotected upgradeable contract" |
||||
WIKI_DESCRIPTION = """Detects logic contract that can be destructed.""" |
||||
WIKI_EXPLOIT_SCENARIO = """ |
||||
```solidity |
||||
contract Buggy is Initializable{ |
||||
address payable owner; |
||||
|
||||
function initialize() external initializer{ |
||||
require(owner == address(0)); |
||||
owner = msg.sender; |
||||
} |
||||
function kill() external{ |
||||
require(msg.sender == owner); |
||||
selfdestruct(owner); |
||||
} |
||||
} |
||||
``` |
||||
Buggy is an upgradeable contract. Anyone can call initialize on the logic contract, and destruct the contract.""" |
||||
|
||||
WIKI_RECOMMENDATION = ( |
||||
"""Add a constructor to ensure `initialize` cannot be called on the logic contract.""" |
||||
) |
||||
|
||||
def _detect(self): |
||||
results = [] |
||||
|
||||
for contract in self.slither.contracts_derived: |
||||
if contract.is_upgradeable: |
||||
functions_that_can_destroy = _can_be_destroyed(contract) |
||||
if functions_that_can_destroy: |
||||
initiliaze_functions = [f for f in contract.functions if f.name == "initialize"] |
||||
vars_init_ = [ |
||||
init.all_state_variables_written() for init in initiliaze_functions |
||||
] |
||||
vars_init = [item for sublist in vars_init_ for item in sublist] |
||||
|
||||
vars_init_in_constructors_ = [ |
||||
f.all_state_variables_written() for f in contract.constructors |
||||
] |
||||
vars_init_in_constructors = [ |
||||
item for sublist in vars_init_in_constructors_ for item in sublist |
||||
] |
||||
if vars_init and (set(vars_init) - set(vars_init_in_constructors)): |
||||
info = ( |
||||
[ |
||||
contract, |
||||
" is an upgradeable contract that does not protect its initiliaze functions: ", |
||||
] |
||||
+ initiliaze_functions |
||||
+ [". Anyone can delete the contract with: ",] |
||||
+ functions_that_can_destroy |
||||
) |
||||
|
||||
res = self.generate_result(info) |
||||
results.append(res) |
||||
|
||||
return results |
||||
@ -0,0 +1,14 @@ |
||||
import "./Initializable.sol"; |
||||
|
||||
contract Buggy is Initializable{ |
||||
address payable owner; |
||||
|
||||
function initialize() external initializer{ |
||||
require(owner == address(0)); |
||||
owner = msg.sender; |
||||
} |
||||
function kill() external{ |
||||
require(msg.sender == owner); |
||||
selfdestruct(owner); |
||||
} |
||||
} |
||||
@ -0,0 +1,152 @@ |
||||
[ |
||||
[ |
||||
{ |
||||
"elements": [ |
||||
{ |
||||
"type": "contract", |
||||
"name": "Buggy", |
||||
"source_mapping": { |
||||
"start": 31, |
||||
"length": 285, |
||||
"filename_used": "/GENERIC_PATH", |
||||
"filename_relative": "tests/detectors/unprotected-upgrade/Buggy.sol", |
||||
"filename_absolute": "/GENERIC_PATH", |
||||
"filename_short": "tests/detectors/unprotected-upgrade/Buggy.sol", |
||||
"is_dependency": false, |
||||
"lines": [ |
||||
3, |
||||
4, |
||||
5, |
||||
6, |
||||
7, |
||||
8, |
||||
9, |
||||
10, |
||||
11, |
||||
12, |
||||
13, |
||||
14, |
||||
15 |
||||
], |
||||
"starting_column": 1, |
||||
"ending_column": 0 |
||||
} |
||||
}, |
||||
{ |
||||
"type": "function", |
||||
"name": "initialize", |
||||
"source_mapping": { |
||||
"start": 96, |
||||
"length": 115, |
||||
"filename_used": "/GENERIC_PATH", |
||||
"filename_relative": "tests/detectors/unprotected-upgrade/Buggy.sol", |
||||
"filename_absolute": "/GENERIC_PATH", |
||||
"filename_short": "tests/detectors/unprotected-upgrade/Buggy.sol", |
||||
"is_dependency": false, |
||||
"lines": [ |
||||
6, |
||||
7, |
||||
8, |
||||
9 |
||||
], |
||||
"starting_column": 5, |
||||
"ending_column": 6 |
||||
}, |
||||
"type_specific_fields": { |
||||
"parent": { |
||||
"type": "contract", |
||||
"name": "Buggy", |
||||
"source_mapping": { |
||||
"start": 31, |
||||
"length": 285, |
||||
"filename_used": "/GENERIC_PATH", |
||||
"filename_relative": "tests/detectors/unprotected-upgrade/Buggy.sol", |
||||
"filename_absolute": "/GENERIC_PATH", |
||||
"filename_short": "tests/detectors/unprotected-upgrade/Buggy.sol", |
||||
"is_dependency": false, |
||||
"lines": [ |
||||
3, |
||||
4, |
||||
5, |
||||
6, |
||||
7, |
||||
8, |
||||
9, |
||||
10, |
||||
11, |
||||
12, |
||||
13, |
||||
14, |
||||
15 |
||||
], |
||||
"starting_column": 1, |
||||
"ending_column": 0 |
||||
} |
||||
}, |
||||
"signature": "initialize()" |
||||
} |
||||
}, |
||||
{ |
||||
"type": "function", |
||||
"name": "kill", |
||||
"source_mapping": { |
||||
"start": 216, |
||||
"length": 98, |
||||
"filename_used": "/GENERIC_PATH", |
||||
"filename_relative": "tests/detectors/unprotected-upgrade/Buggy.sol", |
||||
"filename_absolute": "/GENERIC_PATH", |
||||
"filename_short": "tests/detectors/unprotected-upgrade/Buggy.sol", |
||||
"is_dependency": false, |
||||
"lines": [ |
||||
10, |
||||
11, |
||||
12, |
||||
13 |
||||
], |
||||
"starting_column": 5, |
||||
"ending_column": 6 |
||||
}, |
||||
"type_specific_fields": { |
||||
"parent": { |
||||
"type": "contract", |
||||
"name": "Buggy", |
||||
"source_mapping": { |
||||
"start": 31, |
||||
"length": 285, |
||||
"filename_used": "/GENERIC_PATH", |
||||
"filename_relative": "tests/detectors/unprotected-upgrade/Buggy.sol", |
||||
"filename_absolute": "/GENERIC_PATH", |
||||
"filename_short": "tests/detectors/unprotected-upgrade/Buggy.sol", |
||||
"is_dependency": false, |
||||
"lines": [ |
||||
3, |
||||
4, |
||||
5, |
||||
6, |
||||
7, |
||||
8, |
||||
9, |
||||
10, |
||||
11, |
||||
12, |
||||
13, |
||||
14, |
||||
15 |
||||
], |
||||
"starting_column": 1, |
||||
"ending_column": 0 |
||||
} |
||||
}, |
||||
"signature": "kill()" |
||||
} |
||||
} |
||||
], |
||||
"description": "Buggy (tests/detectors/unprotected-upgrade/Buggy.sol#3-15) is an upgradeable contract that does not protect its initiliaze functions: Buggy.initialize() (tests/detectors/unprotected-upgrade/Buggy.sol#6-9). Anyone can delete the contract with: Buggy.kill() (tests/detectors/unprotected-upgrade/Buggy.sol#10-13)", |
||||
"markdown": "[Buggy](tests/detectors/unprotected-upgrade/Buggy.sol#L3-L15) is an upgradeable contract that does not protect its initiliaze functions: [Buggy.initialize()](tests/detectors/unprotected-upgrade/Buggy.sol#L6-L9). Anyone can delete the contract with: [Buggy.kill()](tests/detectors/unprotected-upgrade/Buggy.sol#L10-L13)", |
||||
"id": "aceca400ce0b482809a70df612af22e24d154c5c89c24d630ec0ee5a366d09fe", |
||||
"check": "unprotected-upgrade", |
||||
"impact": "High", |
||||
"confidence": "High" |
||||
} |
||||
] |
||||
] |
||||
@ -0,0 +1,39 @@ |
||||
import "./Initializable.sol"; |
||||
|
||||
contract Fixed is Initializable{ |
||||
address payable owner; |
||||
|
||||
constructor() public{ |
||||
owner = msg.sender; |
||||
} |
||||
|
||||
function initialize() external initializer{ |
||||
require(owner == address(0)); |
||||
owner = msg.sender; |
||||
} |
||||
function kill() external{ |
||||
require(msg.sender == owner); |
||||
selfdestruct(owner); |
||||
} |
||||
|
||||
function other_function() external{ |
||||
|
||||
} |
||||
} |
||||
|
||||
|
||||
contract Not_Upgradeable{ |
||||
} |
||||
|
||||
contract UpgradeableNoDestruct is Initializable{ |
||||
address payable owner; |
||||
|
||||
constructor() public{ |
||||
owner = msg.sender; |
||||
} |
||||
|
||||
function initialize() external initializer{ |
||||
require(owner == address(0)); |
||||
owner = msg.sender; |
||||
} |
||||
} |
||||
@ -0,0 +1,3 @@ |
||||
[ |
||||
[] |
||||
] |
||||
@ -0,0 +1,5 @@ |
||||
contract Initializable{ |
||||
modifier initializer() { |
||||
_; |
||||
} |
||||
} |
||||
Loading…
Reference in new issue