mirror of https://github.com/crytic/slither
Josselin
6 years ago
parent
11eedcceec
commit
f53151e695
@ -0,0 +1,79 @@ |
||||
""" |
||||
Detect incorrect erc20 interface. |
||||
Some contracts do not return a bool on transfer/transferFrom/approve, which may lead to prevent |
||||
the contract to be used with contracts compiled with recent solc (>0.4.22) |
||||
""" |
||||
from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification |
||||
|
||||
|
||||
class IncorrectERC20InterfaceDetection(AbstractDetector): |
||||
""" |
||||
Incorrect ERC20 Interface |
||||
""" |
||||
|
||||
ARGUMENT = 'erc20-interface' |
||||
HELP = 'Incorrect ERC20 interfaces' |
||||
IMPACT = DetectorClassification.MEDIUM |
||||
CONFIDENCE = DetectorClassification.HIGH |
||||
|
||||
WIKI = 'https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#incorrect-erc20-interface' |
||||
|
||||
WIKI_TITLE = 'Incorrect erc20 interface' |
||||
WIKI_DESCRIPTION = 'Lack of return value for the ERC20 `approve`/`transfer`/`transferFrom` functions. A contract compiled with solidity > 0.4.22 interacting with these functions will fail to execute them, as the return value is missing.' |
||||
WIKI_EXPLOIT_SCENARIO = ''' |
||||
```solidity |
||||
contract Token{ |
||||
function transfer(address to, uint value) external; |
||||
//... |
||||
} |
||||
``` |
||||
`Token.transfer` does not return a boolean. Bob deploys the token. Alice creates a contract that interacts with it but assumes a correct ERC20 interface implementation. Alice's contract is unable to interact with Bob's contract.''' |
||||
|
||||
WIKI_RECOMMENDATION = 'Return a boolean for the `approve`/`transfer`/`transferFrom` functions.' |
||||
|
||||
@staticmethod |
||||
def incorrect_erc20_interface(signature): |
||||
(name, parameters, returnVars) = signature |
||||
|
||||
if name == 'transfer' and parameters == ['address', 'uint256'] and returnVars != ['bool']: |
||||
return True |
||||
|
||||
if name == 'transferFrom' and parameters == ['address', 'address', 'uint256'] and returnVars != ['bool']: |
||||
return True |
||||
|
||||
if name == 'approve' and parameters == ['address', 'uint256'] and returnVars != ['bool']: |
||||
return True |
||||
|
||||
return False |
||||
|
||||
@staticmethod |
||||
def detect_incorrect_erc20_interface(contract): |
||||
""" Detect incorrect ERC20 interface |
||||
|
||||
Returns: |
||||
list(str) : list of incorrect function signatures |
||||
""" |
||||
functions = [f for f in contract.functions if f.contract == contract and \ |
||||
IncorrectERC20InterfaceDetection.incorrect_erc20_interface(f.signature)] |
||||
return functions |
||||
|
||||
def _detect(self): |
||||
""" Detect incorrect erc20 interface |
||||
|
||||
Returns: |
||||
dict: [contrat name] = set(str) events |
||||
""" |
||||
results = [] |
||||
for c in self.contracts: |
||||
functions = IncorrectERC20InterfaceDetection.detect_incorrect_erc20_interface(c) |
||||
if functions: |
||||
info = "{} ({}) has incorrect ERC20 function interface(s):\n" |
||||
info = info.format(c.name, |
||||
c.source_mapping_str) |
||||
for function in functions: |
||||
info += "\t-{} ({})\n".format(function.name, function.source_mapping_str) |
||||
json = self.generate_json_result(info) |
||||
self.add_functions_to_json(functions, json) |
||||
results.append(json) |
||||
|
||||
return results |
||||
@ -0,0 +1,85 @@ |
||||
""" |
||||
Detect mistakenly un-indexed ERC20 event parameters |
||||
""" |
||||
from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification |
||||
|
||||
|
||||
class UnindexedERC20EventParameters(AbstractDetector): |
||||
""" |
||||
Un-indexed ERC20 event parameters |
||||
""" |
||||
|
||||
ARGUMENT = 'erc20-indexed' |
||||
HELP = 'Un-indexed ERC20 event parameters' |
||||
IMPACT = DetectorClassification.INFORMATIONAL |
||||
CONFIDENCE = DetectorClassification.HIGH |
||||
|
||||
WIKI = 'https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#unindexed-erc20-event-parameters' |
||||
|
||||
WIKI_TITLE = 'Unindexed ERC20 Event Parameters' |
||||
WIKI_DESCRIPTION = 'Detects that events defined by the ERC20 specification which are meant to have some parameters as `indexed`, are not missing the `indexed` keyword.' |
||||
WIKI_EXPLOIT_SCENARIO = ''' |
||||
```solidity |
||||
contract ERC20Bad { |
||||
// ... |
||||
event Transfer(address from, address to, uint value); |
||||
event Approval(address owner, address spender, uint value); |
||||
|
||||
// ... |
||||
} |
||||
``` |
||||
In this case, Transfer and Approval events should have the 'indexed' keyword on their two first parameters, as defined by the ERC20 specification. Failure to include these keywords will not include the parameter data in the transaction/block's bloom filter. This may cause external tooling searching for these parameters to overlook them, and fail to index logs from this token contract.''' |
||||
|
||||
WIKI_RECOMMENDATION = 'Add the `indexed` keyword to event parameters which should include it, according to the ERC20 specification.' |
||||
|
||||
@staticmethod |
||||
def detect_erc20_unindexed_event_params(contract): |
||||
""" |
||||
Detect un-indexed ERC20 event parameters in a given contract. |
||||
:param contract: The contract to check ERC20 events for un-indexed parameters in. |
||||
:return: A list of tuple(event, parameter) of parameters which should be indexed. |
||||
""" |
||||
# Create our result array |
||||
results = [] |
||||
|
||||
# If this contract isn't an ERC20 token, we return our empty results. |
||||
if not contract.is_erc20(): |
||||
return results |
||||
|
||||
# Loop through all events to look for poor form. |
||||
for event in contract.events: |
||||
|
||||
# Only handle events which are declared in this contract. |
||||
if event.contract != contract: |
||||
continue |
||||
|
||||
# If this is transfer/approval events, expect the first two parameters to be indexed. |
||||
if event.full_name in ["Transfer(address,address,uint256)", |
||||
"Approval(address,address,uint256)"]: |
||||
if not event.elems[0].indexed: |
||||
results.append((event, event.elems[0])) |
||||
if not event.elems[1].indexed: |
||||
results.append((event, event.elems[1])) |
||||
|
||||
# Return the results. |
||||
return results |
||||
|
||||
def _detect(self): |
||||
""" |
||||
Detect un-indexed ERC20 event parameters in all contracts. |
||||
""" |
||||
results = [] |
||||
for c in self.contracts: |
||||
unindexed_params = self.detect_erc20_unindexed_event_params(c) |
||||
if unindexed_params: |
||||
info = "{} ({}) does not mark important ERC20 parameters as 'indexed':\n" |
||||
info = info.format(c.name, c.source_mapping_str) |
||||
for (event, parameter) in unindexed_params: |
||||
info += "\t-{} ({}) does not index parameter '{}'\n".format(event.name, event.source_mapping_str, parameter.name) |
||||
|
||||
# Add the events to the JSON (note: we do not add the params/vars as they have no source mapping). |
||||
json = self.generate_json_result(info) |
||||
self.add_functions_to_json([event for event, _ in unindexed_params], json) |
||||
results.append(json) |
||||
|
||||
return results |
||||
@ -0,0 +1,177 @@ |
||||
""" |
||||
Module detecting deprecated standards. |
||||
""" |
||||
|
||||
from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification |
||||
from slither.visitors.expression.export_values import ExportValues |
||||
from slither.core.declarations.solidity_variables import SolidityVariableComposed, SolidityFunction |
||||
from slither.core.cfg.node import NodeType |
||||
from slither.slithir.operations import LowLevelCall |
||||
from slither.solc_parsing.variables.state_variable import StateVariableSolc, StateVariable |
||||
|
||||
# Reference: https://smartcontractsecurity.github.io/SWC-registry/docs/SWC-111 |
||||
class DeprecatedStandards(AbstractDetector): |
||||
""" |
||||
Use of Deprecated Standards |
||||
""" |
||||
|
||||
ARGUMENT = 'deprecated-standards' |
||||
HELP = 'Deprecated Solidity Standards' |
||||
IMPACT = DetectorClassification.INFORMATIONAL |
||||
CONFIDENCE = DetectorClassification.HIGH |
||||
|
||||
WIKI = 'https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#deprecated-standards' |
||||
|
||||
WIKI_TITLE = 'Deprecated Standards' |
||||
WIKI_DESCRIPTION = 'Detect the usage of deprecated standards (as defined by SWC-111), excluding only `constant` keyword detection on functions.' |
||||
WIKI_EXPLOIT_SCENARIO = ''' |
||||
```solidity |
||||
contract ContractWithDeprecatedReferences { |
||||
// Deprecated: Change block.blockhash() -> blockhash() |
||||
bytes32 globalBlockHash = block.blockhash(0); |
||||
|
||||
// Deprecated: Change constant -> view |
||||
function functionWithDeprecatedThrow() public constant { |
||||
// Deprecated: Change msg.gas -> gasleft() |
||||
if(msg.gas == msg.value) { |
||||
// Deprecated: Change throw -> revert() |
||||
throw; |
||||
} |
||||
} |
||||
|
||||
// Deprecated: Change constant -> view |
||||
function functionWithDeprecatedReferences() public constant { |
||||
// Deprecated: Change sha3() -> keccak256() |
||||
bytes32 sha3Result = sha3("test deprecated sha3 usage"); |
||||
|
||||
// Deprecated: Change callcode() -> delegatecall() |
||||
address(this).callcode(); |
||||
|
||||
// Deprecated: Change suicide() -> selfdestruct() |
||||
suicide(address(0)); |
||||
} |
||||
} |
||||
```''' |
||||
|
||||
WIKI_RECOMMENDATION = 'Replace all uses of deprecated symbols.' |
||||
|
||||
# The format for the following deprecated lists is [(detecting_signature, original_text, recommended_text)] |
||||
DEPRECATED_SOLIDITY_VARIABLE = [("block.blockhash", "block.blockhash()", "blockhash()"), |
||||
("msg.gas", "msg.gas", "gasleft()")] |
||||
DEPRECATED_SOLIDITY_FUNCTIONS = [("suicide(address)", "suicide()", "selfdestruct()"), |
||||
("sha3()", "sha3()", "keccak256()")] |
||||
DEPRECATED_NODE_TYPES = [(NodeType.THROW, "throw", "revert()")] |
||||
DEPRECATED_LOW_LEVEL_CALLS = [("callcode", "callcode", "delegatecall")] |
||||
|
||||
def detect_deprecation_in_expression(self, expression): |
||||
""" Detects if an expression makes use of any deprecated standards. |
||||
|
||||
Returns: |
||||
list of tuple: (detecting_signature, original_text, recommended_text)""" |
||||
# Perform analysis on this expression |
||||
export = ExportValues(expression) |
||||
export_values = export.result() |
||||
|
||||
# Define our results list |
||||
results = [] |
||||
|
||||
# Check if there is usage of any deprecated solidity variables or functions |
||||
for dep_var in self.DEPRECATED_SOLIDITY_VARIABLE: |
||||
if SolidityVariableComposed(dep_var[0]) in export_values: |
||||
results.append(dep_var) |
||||
for dep_func in self.DEPRECATED_SOLIDITY_FUNCTIONS: |
||||
if SolidityFunction(dep_func[0]) in export_values: |
||||
results.append(dep_func) |
||||
|
||||
return results |
||||
|
||||
def detect_deprecated_references_in_node(self, node): |
||||
""" Detects if a node makes use of any deprecated standards. |
||||
|
||||
Returns: |
||||
list of tuple: (detecting_signature, original_text, recommended_text)""" |
||||
# Define our results list |
||||
results = [] |
||||
|
||||
# If this node has an expression, we check the underlying expression. |
||||
if node.expression: |
||||
results += self.detect_deprecation_in_expression(node.expression) |
||||
|
||||
# Check if there is usage of any deprecated solidity variables or functions |
||||
for dep_node in self.DEPRECATED_NODE_TYPES: |
||||
if node.type == dep_node[0]: |
||||
results.append(dep_node) |
||||
|
||||
return results |
||||
|
||||
def detect_deprecated_references_in_contract(self, contract): |
||||
""" Detects the usage of any deprecated built-in symbols. |
||||
|
||||
Returns: |
||||
list of tuple: (state_variable | node, (detecting_signature, original_text, recommended_text))""" |
||||
results = [] |
||||
|
||||
for state_variable in contract.variables: |
||||
if state_variable.contract != contract: |
||||
continue |
||||
if state_variable.expression: |
||||
deprecated_results = self.detect_deprecation_in_expression(state_variable.expression) |
||||
if deprecated_results: |
||||
results.append((state_variable, deprecated_results)) |
||||
|
||||
# Loop through all functions + modifiers in this contract. |
||||
for function in contract.functions + contract.modifiers: |
||||
# We should only look for functions declared directly in this contract (not in a base contract). |
||||
if function.contract != contract: |
||||
continue |
||||
|
||||
# Loop through each node in this function. |
||||
for node in function.nodes: |
||||
# Detect deprecated references in the node. |
||||
deprecated_results = self.detect_deprecated_references_in_node(node) |
||||
|
||||
# Detect additional deprecated low-level-calls. |
||||
for ir in node.irs: |
||||
if isinstance(ir, LowLevelCall): |
||||
for dep_llc in self.DEPRECATED_LOW_LEVEL_CALLS: |
||||
if ir.function_name == dep_llc[0]: |
||||
deprecated_results.append(dep_llc) |
||||
|
||||
# If we have any results from this iteration, add them to our results list. |
||||
if deprecated_results: |
||||
results.append((node, deprecated_results)) |
||||
|
||||
return results |
||||
|
||||
def _detect(self): |
||||
""" Detect shadowing local variables |
||||
|
||||
Recursively visit the calls |
||||
Returns: |
||||
list: {'vuln', 'filename,'contract','func', 'deprecated_references'} |
||||
|
||||
""" |
||||
results = [] |
||||
for contract in self.contracts: |
||||
deprecated_references = self.detect_deprecated_references_in_contract(contract) |
||||
if deprecated_references: |
||||
for deprecated_reference in deprecated_references: |
||||
source_object = deprecated_reference[0] |
||||
deprecated_entries = deprecated_reference[1] |
||||
info = 'Deprecated standard detected @ {}:\n'.format(source_object.source_mapping_str) |
||||
|
||||
for (dep_id, original_desc, recommended_disc) in deprecated_entries: |
||||
info += "\t- Usage of \"{}\" should be replaced with \"{}\"\n".format(original_desc, |
||||
recommended_disc) |
||||
|
||||
|
||||
# Generate relevant JSON data for this shadowing definition. |
||||
json = self.generate_json_result(info) |
||||
if isinstance(source_object, StateVariableSolc) or isinstance(source_object, StateVariable): |
||||
self.add_variable_to_json(source_object, json) |
||||
else: |
||||
self.add_nodes_to_json([source_object], json) |
||||
|
||||
results.append(json) |
||||
|
||||
return results |
||||
@ -0,0 +1,27 @@ |
||||
contract ContractWithDeprecatedReferences { |
||||
bytes32 globalBlockHash = block.blockhash(0); |
||||
|
||||
// Deprecated: Change constant -> view |
||||
function functionWithDeprecatedThrow() public constant { |
||||
// Deprecated: Change msg.gas -> gasleft() |
||||
if(msg.gas == msg.value) { |
||||
// Deprecated: Change throw -> revert() |
||||
throw; |
||||
} |
||||
} |
||||
|
||||
// Deprecated: Change constant -> view |
||||
function functionWithDeprecatedReferences() public constant { |
||||
// Deprecated: Change sha3() -> keccak256() |
||||
bytes32 sha3Result = sha3("test deprecated sha3 usage"); |
||||
|
||||
// Deprecated: Change block.blockhash() -> blockhash() |
||||
bytes32 blockHashResult = block.blockhash(0); |
||||
|
||||
// Deprecated: Change callcode() -> delegatecall() |
||||
address(this).callcode(); |
||||
|
||||
// Deprecated: Change suicide() -> selfdestruct() |
||||
suicide(address(0)); |
||||
} |
||||
} |
||||
@ -0,0 +1,25 @@ |
||||
contract IERC20Good { |
||||
function transfer(address to, uint value) external returns (bool); |
||||
function approve(address spender, uint value) external returns (bool); |
||||
function transferFrom(address from, address to, uint value) external returns (bool); |
||||
function totalSupply() external view returns (uint); |
||||
function balanceOf(address who) external view returns (uint); |
||||
function allowance(address owner, address spender) external view returns (uint); |
||||
event Transfer(address indexed from, address indexed to, uint value); |
||||
event Approval(address indexed owner, address indexed spender, uint value); |
||||
} |
||||
|
||||
contract IERC20Bad { |
||||
function transfer(address to, uint value) external returns (bool); |
||||
function approve(address spender, uint value) external returns (bool); |
||||
function transferFrom(address from, address to, uint value) external returns (bool); |
||||
function totalSupply() external view returns (uint); |
||||
function balanceOf(address who) external view returns (uint); |
||||
function allowance(address owner, address spender) external view returns (uint); |
||||
event Transfer(address from, address to, uint value); |
||||
event Approval(address owner, address spender, uint value); |
||||
} |
||||
|
||||
contract ERC20BadDerived is IERC20Bad { |
||||
|
||||
} |
||||
@ -0,0 +1 @@ |
||||
[{"check": "deprecated-standards", "impact": "Informational", "confidence": "High", "description": "Deprecated standard detected @ tests/deprecated_calls.sol#2:\n\t- Usage of \"block.blockhash()\" should be replaced with \"blockhash()\"\n", "elements": [{"type": "variable", "name": "globalBlockHash", "source_mapping": {"start": 48, "length": 44, "filename": "tests/deprecated_calls.sol", "lines": [2]}}]}, {"check": "deprecated-standards", "impact": "Informational", "confidence": "High", "description": "Deprecated standard detected @ tests/deprecated_calls.sol#7-10:\n\t- Usage of \"msg.gas\" should be replaced with \"gasleft()\"\n", "elements": [{"type": "expression", "expression": "msg.gas == msg.value", "source_mapping": {"start": 258, "length": 107, "filename": "tests/deprecated_calls.sol", "lines": [7, 8, 9, 10]}}]}, {"check": "deprecated-standards", "impact": "Informational", "confidence": "High", "description": "Deprecated standard detected @ tests/deprecated_calls.sol#9:\n\t- Usage of \"throw\" should be replaced with \"revert()\"\n", "elements": [{"type": "expression", "expression": "None", "source_mapping": {"start": 349, "length": 5, "filename": "tests/deprecated_calls.sol", "lines": [9]}}]}, {"check": "deprecated-standards", "impact": "Informational", "confidence": "High", "description": "Deprecated standard detected @ tests/deprecated_calls.sol#16:\n\t- Usage of \"sha3()\" should be replaced with \"keccak256()\"\n", "elements": [{"type": "expression", "expression": "sha3Result = sha3()(test deprecated sha3 usage)", "source_mapping": {"start": 542, "length": 55, "filename": "tests/deprecated_calls.sol", "lines": [16]}}]}, {"check": "deprecated-standards", "impact": "Informational", "confidence": "High", "description": "Deprecated standard detected @ tests/deprecated_calls.sol#19:\n\t- Usage of \"block.blockhash()\" should be replaced with \"blockhash()\"\n", "elements": [{"type": "expression", "expression": "blockHashResult = block.blockhash(0)", "source_mapping": {"start": 671, "length": 44, "filename": "tests/deprecated_calls.sol", "lines": [19]}}]}, {"check": "deprecated-standards", "impact": "Informational", "confidence": "High", "description": "Deprecated standard detected @ tests/deprecated_calls.sol#22:\n\t- Usage of \"callcode\" should be replaced with \"delegatecall\"\n", "elements": [{"type": "expression", "expression": "address(this).callcode()", "source_mapping": {"start": 785, "length": 24, "filename": "tests/deprecated_calls.sol", "lines": [22]}}]}, {"check": "deprecated-standards", "impact": "Informational", "confidence": "High", "description": "Deprecated standard detected @ tests/deprecated_calls.sol#25:\n\t- Usage of \"suicide()\" should be replaced with \"selfdestruct()\"\n", "elements": [{"type": "expression", "expression": "suicide(address)(address(0))", "source_mapping": {"start": 878, "length": 19, "filename": "tests/deprecated_calls.sol", "lines": [25]}}]}] |
||||
@ -0,0 +1 @@ |
||||
[{"check": "erc20-indexed", "impact": "Informational", "confidence": "High", "description": "IERC20Bad (tests/erc20_indexed.sol#12-21) does not mark important ERC20 parameters as 'indexed':\n\t-Transfer (tests/erc20_indexed.sol#19) does not index parameter 'from'\n\t-Transfer (tests/erc20_indexed.sol#19) does not index parameter 'to'\n\t-Approval (tests/erc20_indexed.sol#20) does not index parameter 'owner'\n\t-Approval (tests/erc20_indexed.sol#20) does not index parameter 'spender'\n", "elements": [{"type": "function", "name": "Approval", "source_mapping": {"start": 1148, "length": 59, "filename": "tests/erc20_indexed.sol", "lines": [20]}, "contract": {"type": "contract", "name": "IERC20Bad", "source_mapping": {"start": 622, "length": 587, "filename": "tests/erc20_indexed.sol", "lines": [12, 13, 14, 15, 16, 17, 18, 19, 20, 21]}}}, {"type": "function", "name": "Approval", "source_mapping": {"start": 1148, "length": 59, "filename": "tests/erc20_indexed.sol", "lines": [20]}, "contract": {"type": "contract", "name": "IERC20Bad", "source_mapping": {"start": 622, "length": 587, "filename": "tests/erc20_indexed.sol", "lines": [12, 13, 14, 15, 16, 17, 18, 19, 20, 21]}}}, {"type": "function", "name": "Transfer", "source_mapping": {"start": 1090, "length": 53, "filename": "tests/erc20_indexed.sol", "lines": [19]}, "contract": {"type": "contract", "name": "IERC20Bad", "source_mapping": {"start": 622, "length": 587, "filename": "tests/erc20_indexed.sol", "lines": [12, 13, 14, 15, 16, 17, 18, 19, 20, 21]}}}, {"type": "function", "name": "Transfer", "source_mapping": {"start": 1090, "length": 53, "filename": "tests/erc20_indexed.sol", "lines": [19]}, "contract": {"type": "contract", "name": "IERC20Bad", "source_mapping": {"start": 622, "length": 587, "filename": "tests/erc20_indexed.sol", "lines": [12, 13, 14, 15, 16, 17, 18, 19, 20, 21]}}}]}] |
||||
@ -0,0 +1 @@ |
||||
[{"check": "erc20-interface", "impact": "Medium", "confidence": "High", "description": "Token (tests/incorrect_erc20_interface.sol#3-7) has incorrect ERC20 function interface(s):\n\t-transfer (tests/incorrect_erc20_interface.sol#5)\n", "elements": [{"type": "function", "name": "transfer", "source_mapping": {"start": 47, "length": 51, "filename": "tests/incorrect_erc20_interface.sol", "lines": [5]}, "contract": {"type": "contract", "name": "Token", "source_mapping": {"start": 26, "length": 75, "filename": "tests/incorrect_erc20_interface.sol", "lines": [3, 4, 5, 6, 7]}}}]}] |
||||
@ -0,0 +1,7 @@ |
||||
pragma solidity ^0.4.24; |
||||
|
||||
contract Token{ |
||||
|
||||
function transfer(address to, uint value) external; |
||||
|
||||
} |
||||
Loading…
Reference in new issue