Open source shadowing state variable detectors

pull/108/head
Josselin 6 years ago
parent fed11e9cf6
commit d5c439c5d2
  1. 41
      README.md
  2. 3
      scripts/tests_generate_expected_json_4.sh
  3. 2
      scripts/travis_test_4.sh
  4. 8
      slither/__main__.py
  5. 65
      slither/detectors/shadowing/abstract.py
  6. 64
      slither/detectors/shadowing/state.py
  7. 1
      tests/expected_json/shadowing_abstract.shadowing-abstract.json
  8. 1
      tests/expected_json/shadowing_state_variable.shadowing-state.json

@ -53,26 +53,27 @@ By default, all the detectors are run.
Num | Detector | What it Detects | Impact | Confidence
--- | --- | --- | --- | ---
1 | `suicidal` | [Functions allowing anyone to destruct the contract](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#suicidal) | High | High
2 | `uninitialized-state` | [Uninitialized state variables](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#uninitialized-state-variables) | High | High
3 | `uninitialized-storage` | [Uninitialized storage variables](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#uninitialized-storage-variables) | High | High
4 | `arbitrary-send` | [Functions that send ether to arbitrary destinations](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#functions-that-send-ether-to-arbitrary-destinations) | High | Medium
5 | `controlled-delegatecall` | [Controlled delegatecall destination](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#controlled-delegatecall) | High | Medium
6 | `reentrancy` | [Reentrancy vulnerabilities](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#reentrancy-vulnerabilities) | High | Medium
7 | `locked-ether` | [Contracts that lock ether](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#contracts-that-lock-ether) | Medium | High
8 | `constant-function` | [Constant functions changing the state](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#constant-functions-changing-the-state) | Medium | Medium
9 | `tx-origin` | [Dangerous usage of `tx.origin`](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#dangerous-usage-of-txorigin) | Medium | Medium
10 | `uninitialized-local` | [Uninitialized local variables](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#uninitialized-local-variables) | Medium | Medium
11 | `unused-return` | [Unused return values](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#unused-return) | Medium | Medium
12 | `assembly` | [Assembly usage](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#assembly-usage) | Informational | High
13 | `constable-states` | [State variables that could be declared constant](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#state-variables-that-could-be-declared-constant) | Informational | High
14 | `external-function` | [Public function that could be declared as external](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#public-function-that-could-be-declared-as-external) | Informational | High
15 | `low-level-calls` | [Low level calls](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#low-level-calls) | Informational | High
16 | `naming-convention` | [Conformance to Solidity naming conventions](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#conformance-to-solidity-naming-conventions) | Informational | High
17 | `pragma` | [If different pragma directives are used](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#state-variables-that-could-be-declared-constant) | Informational | High
18 | `solc-version` | [Old versions of Solidity (< 0.4.23)](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#old-versions-of-solidity) | Informational | High
19 | `unused-state` | [Unused state variables](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#unused-state-variables) | Informational | High
1 | `shadowing-state` | [State variables shadowing](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#state-variable-shadowing) | High | High
2 | `suicidal` | [Functions allowing anyone to destruct the contract](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#suicidal) | High | High
3 | `uninitialized-state` | [Uninitialized state variables](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#uninitialized-state-variables) | High | High
4 | `uninitialized-storage` | [Uninitialized storage variables](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#uninitialized-storage-variables) | High | High
5 | `arbitrary-send` | [Functions that send ether to arbitrary destinations](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#functions-that-send-ether-to-arbitrary-destinations) | High | Medium
6 | `controlled-delegatecall` | [Controlled delegatecall destination](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#controlled-delegatecall) | High | Medium
7 | `reentrancy` | [Reentrancy vulnerabilities](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#reentrancy-vulnerabilities) | High | Medium
8 | `locked-ether` | [Contracts that lock ether](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#contracts-that-lock-ether) | Medium | High
9 | `shadowing-abstract` | [State variables shadowing from abstract contracts](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#state-variable-shadowing-from-abstract-contracts) | Medium | High
10 | `constant-function` | [Constant functions changing the state](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#constant-functions-changing-the-state) | Medium | Medium
11 | `tx-origin` | [Dangerous usage of `tx.origin`](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#dangerous-usage-of-txorigin) | Medium | Medium
12 | `uninitialized-local` | [Uninitialized local variables](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#uninitialized-local-variables) | Medium | Medium
13 | `unused-return` | [Unused return values](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#unused-return) | Medium | Medium
14 | `assembly` | [Assembly usage](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#assembly-usage) | Informational | High
15 | `constable-states` | [State variables that could be declared constant](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#state-variables-that-could-be-declared-constant) | Informational | High
16 | `external-function` | [Public function that could be declared as external](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#public-function-that-could-be-declared-as-external) | Informational | High
17 | `low-level-calls` | [Low level calls](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#low-level-calls) | Informational | High
18 | `naming-convention` | [Conformance to Solidity naming conventions](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#conformance-to-solidity-naming-conventions) | Informational | High
19 | `pragma` | [If different pragma directives are used](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#different-pragma-directives-are-used) | Informational | High
20 | `solc-version` | [Old versions of Solidity (< 0.4.23)](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#old-versions-of-solidity) | Informational | High
21 | `unused-state` | [Unused state variables](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#unused-state-variables) | Informational | High
[Contact us](https://www.trailofbits.com/contact/) to get access to additional detectors.

@ -34,5 +34,6 @@ generate_expected_json tests/naming_convention.sol "naming-convention"
generate_expected_json tests/uninitialized_local_variable.sol "uninitialized-local"
generate_expected_json tests/controlled_delegatecall.sol "controlled-delegatecall"
generate_expected_json tests/constant.sol "constant-function"
generate_expected_json tests/unused_return.sol "unused-return"
generate_expected_json tests/shadowing_state_variable.sol "shadowing-state"
generate_expected_json tests/shadowing_abstract.sol "shadowing-abstract"

@ -86,6 +86,8 @@ test_slither tests/naming_convention.sol "naming-convention"
test_slither tests/controlled_delegatecall.sol "controlled-delegatecall"
test_slither tests/constant.sol "constant-function"
test_slither tests/unused_return.sol "unused-return"
test_slither tests/shadowing_abstract.sol "shadowing-abstract"
test_slither tests/shadowing_state_variable.sol "shadowing-state"

@ -127,6 +127,10 @@ def get_detectors_and_printers():
from slither.detectors.functions.external_function import ExternalFunction
from slither.detectors.statements.controlled_delegatecall import ControlledDelegateCall
from slither.detectors.attributes.const_functions import ConstantFunctions
from slither.detectors.attributes.const_functions import ConstantFunctions
from slither.detectors.attributes.const_functions import ConstantFunctions
from slither.detectors.shadowing.abstract import ShadowingAbstractDetection
from slither.detectors.shadowing.state import StateShadowing
detectors = [Backdoor,
UninitializedStateVarsDetection,
@ -148,7 +152,9 @@ def get_detectors_and_printers():
UnusedReturnValues,
ExternalFunction,
ControlledDelegateCall,
ConstantFunctions]
ConstantFunctions,
ShadowingAbstractDetection,
StateShadowing]
from slither.printers.summary.function import FunctionSummary
from slither.printers.summary.contract import ContractSummary

@ -0,0 +1,65 @@
"""
Module detecting shadowing variables on abstract contract
Recursively check the called functions
"""
from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification
class ShadowingAbstractDetection(AbstractDetector):
"""
Shadowing detection
"""
ARGUMENT = 'shadowing-abstract'
HELP = 'State variables shadowing from abstract contracts'
IMPACT = DetectorClassification.MEDIUM
CONFIDENCE = DetectorClassification.HIGH
WIKI = 'https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#state-variable-shadowing-from-abstract-contracts'
vuln_name = "ShadowingAbstractContract"
def detect_shadowing(self, contract):
ret = []
variables_fathers = []
for father in contract.inheritance:
if all(not f.is_implemented for f in father.functions + father.modifiers):
variables_fathers += [v for v in father.variables if v.contract == father]
for var in [v for v in contract.variables if v.contract == contract]:
shadow = [v for v in variables_fathers if v.name == var.name]
if shadow:
ret.append([var] + shadow)
return ret
def detect(self):
""" Detect shadowing
Recursively visit the calls
Returns:
list: {'vuln', 'filename,'contract','func', 'shadow'}
"""
results = []
for c in self.contracts:
shadowing = self.detect_shadowing(c)
if shadowing:
for all_variables in shadowing:
shadow = all_variables[0]
variables = all_variables[1:]
info = '{}.{} ({}) shadows:\n'.format(shadow.contract.name,
shadow.name,
shadow.source_mapping_str)
for var in variables:
info += "\t- {}.{} ({})\n".format(var.contract.name,
var.name,
var.source_mapping_str)
self.log(info)
json = self.generate_json_result(info)
self.add_variables_to_json(all_variables, json)
results.append(json)
return results

@ -0,0 +1,64 @@
"""
Module detecting shadowing of state variables
"""
from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification
class StateShadowing(AbstractDetector):
"""
Shadowing of state variable
"""
ARGUMENT = 'shadowing-state'
HELP = 'State variables shadowing'
IMPACT = DetectorClassification.HIGH
CONFIDENCE = DetectorClassification.HIGH
WIKI = 'https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#state-variable-shadowing'
vuln_name = 'Shadowing'
def detect_shadowing(self, contract):
ret = []
variables_fathers = []
for father in contract.inheritance:
if any(f.is_implemented for f in father.functions + father.modifiers):
variables_fathers += [v for v in father.variables if v.contract == father]
for var in [v for v in contract.variables if v.contract == contract]:
shadow = [v for v in variables_fathers if v.name == var.name]
if shadow:
ret.append([var] + shadow)
return ret
def detect(self):
""" Detect shadowing
Recursively visit the calls
Returns:
list: {'vuln', 'filename,'contract','func', 'shadow'}
"""
results = []
for c in self.contracts:
shadowing = self.detect_shadowing(c)
if shadowing:
for all_variables in shadowing:
shadow = all_variables[0]
variables = all_variables[1:]
info = '{}.{} ({}) shadows:\n'.format(shadow.contract.name,
shadow.name,
shadow.source_mapping_str)
for var in variables:
info += "\t- {}.{} ({})\n".format(var.contract.name,
var.name,
var.source_mapping_str)
self.log(info)
json = self.generate_json_result(info)
self.add_variables_to_json(all_variables, json)
results.append(json)
return results

@ -0,0 +1 @@
[{"check": "shadowing-abstract", "impact": "Medium", "confidence": "High", "description": "DerivedContract.owner (tests/shadowing_abstract.sol#7) shadows:\n\t- BaseContract.owner (tests/shadowing_abstract.sol#2)\n", "variables": [{"name": "owner", "source_mapping": {"start": 92, "length": 13, "filename": "tests/shadowing_abstract.sol", "lines": [7]}}, {"name": "owner", "source_mapping": {"start": 27, "length": 13, "filename": "tests/shadowing_abstract.sol", "lines": [2]}}]}]

@ -0,0 +1 @@
[{"check": "shadowing-state", "impact": "High", "confidence": "High", "description": "DerivedContract.owner (tests/shadowing_state_variable.sol#12) shadows:\n\t- BaseContract.owner (tests/shadowing_state_variable.sol#2)\n", "variables": [{"name": "owner", "source_mapping": {"start": 172, "length": 13, "filename": "tests/shadowing_state_variable.sol", "lines": [12]}}, {"name": "owner", "source_mapping": {"start": 27, "length": 13, "filename": "tests/shadowing_state_variable.sol", "lines": [2]}}]}]
Loading…
Cancel
Save