Merge branch 'dev' into dev-old-solc

pull/129/head
Josselin 6 years ago
commit 369dbddeaf
  1. 57
      README.md
  2. 2
      scripts/tests_generate_expected_json_4.sh
  3. 1
      scripts/tests_generate_expected_json_5.sh
  4. 3
      scripts/travis_test_4.sh
  5. 2
      scripts/travis_test_5.sh
  6. 9
      slither/__main__.py
  7. 18
      slither/detectors/reentrancy/reentrancy_benign.py
  8. 32
      slither/detectors/reentrancy/reentrancy_eth.py
  9. 32
      slither/detectors/reentrancy/reentrancy_read_before_write.py
  10. 133
      slither/detectors/shadowing/builtin_symbols.py
  11. 109
      slither/detectors/shadowing/local.py
  12. 119
      slither/detectors/statements/incorrect_strict_equality.py
  13. 2
      slither/slithir/utils/ssa.py
  14. 1
      tests/expected_json/incorrect_equality.incorrect-equality.json
  15. 1
      tests/expected_json/shadowing_builtin_symbols.shadowing-builtin.json
  16. 1
      tests/expected_json/shadowing_local_variable.shadowing-local.json
  17. 136
      tests/incorrect_equality.sol
  18. 34
      tests/shadowing_builtin_symbols.sol
  19. 26
      tests/shadowing_local_variable.sol

@ -60,25 +60,27 @@ Num | Detector | What it Detects | Impact | Confidence
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-eth` | [Reentrancy vulnerabilities (theft of ethers)](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 | `reentrancy-no-eth` | [Reentrancy vulnerabilities (no theft of ethers)](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#reentrancy-vulnerabilities) | Medium | Medium
12 | `tx-origin` | [Dangerous usage of `tx.origin`](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#dangerous-usage-of-txorigin) | Medium | Medium
13 | `uninitialized-local` | [Uninitialized local variables](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#uninitialized-local-variables) | Medium | Medium
14 | `unused-return` | [Unused return values](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#unused-return) | Medium | Medium
15 | `calls-loop` | [Multiple calls in a loop](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description/_edit#calls-inside-a-loop) | Low | Medium
16 | `reentrancy-benign` | [Benign reentrancy vulnerabilities](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#reentrancy-vulnerabilities) | Low | Medium
17 | `timestamp` | [Dangerous usage of `block.timestamp`](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#block-timestamp) | Low | Medium
18 | `assembly` | [Assembly usage](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#assembly-usage) | Informational | High
19 | `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
20 | `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
21 | `low-level-calls` | [Low level calls](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#low-level-calls) | Informational | High
22 | `naming-convention` | [Conformance to Solidity naming conventions](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#conformance-to-solidity-naming-conventions) | Informational | High
23 | `pragma` | [If different pragma directives are used](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#different-pragma-directives-are-used) | Informational | High
24 | `solc-version` | [Old versions of Solidity (< 0.4.23)](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#old-versions-of-solidity) | Informational | High
25 | `unused-state` | [Unused state variables](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#unused-state-variables) | Informational | High
8 | `incorrect-equality` | [Dangerous strict equalities](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#dangerous-strict-equalities) | Medium | High
9 | `locked-ether` | [Contracts that lock ether](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#contracts-that-lock-ether) | Medium | High
10 | `shadowing-abstract` | [State variables shadowing from abstract contracts](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#state-variable-shadowing-from-abstract-contracts) | Medium | High
11 | `constant-function` | [Constant functions changing the state](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#constant-functions-changing-the-state) | Medium | Medium
12 | `reentrancy-no-eth` | [Reentrancy vulnerabilities (no theft of ethers)](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#reentrancy-vulnerabilities) | Medium | Medium
13 | `tx-origin` | [Dangerous usage of `tx.origin`](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#dangerous-usage-of-txorigin) | Medium | Medium
14 | `uninitialized-local` | [Uninitialized local variables](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#uninitialized-local-variables) | Medium | Medium
15 | `unused-return` | [Unused return values](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#unused-return) | Medium | Medium
16 | `shadowing-builtin` | [Built-in symbol shadowing](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#builtin-symbol-shadowing) | Low | High
17 | `shadowing-local` | [Local variables shadowing](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#local-variable-shadowing) | Low | High
18 | `calls-loop` | [Multiple calls in a loop](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description/_edit#calls-inside-a-loop) | Low | Medium
19 | `reentrancy-benign` | [Benign reentrancy vulnerabilities](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#reentrancy-vulnerabilities) | Low | Medium
20 | `timestamp` | [Dangerous usage of `block.timestamp`](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#block-timestamp) | Low | Medium
21 | `assembly` | [Assembly usage](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#assembly-usage) | Informational | High
22 | `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
23 | `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
24 | `low-level-calls` | [Low level calls](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#low-level-calls) | Informational | High
25 | `naming-convention` | [Conformance to Solidity naming conventions](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#conformance-to-solidity-naming-conventions) | Informational | High
26 | `pragma` | [If different pragma directives are used](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#different-pragma-directives-are-used) | Informational | High
27 | `solc-version` | [Old versions of Solidity (< 0.4.23)](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#old-versions-of-solidity) | Informational | High
28 | `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.
@ -89,14 +91,15 @@ To run a printer, use `--print` and a comma-separated list of printers.
Num | Printer | Description
--- | --- | ---
1 | `call-graph` | Export the call-graph of the contracts to a dot file
2 | `contract-summary` | Print a summary of the contracts
3 | `function-summary` | Print a summary of the functions
4 | `human-summary` | Print a human readable summary of the contracts
5 | `inheritance` | Print the inheritance relations between contracts
6 | `inheritance-graph` | Export the inheritance graph of each contract to a dot file
7 | `slithir` | Print the slithIR representation of the functions
8 | `vars-and-auth` | Print the state variables written and the authorization of the functions
2 | `cfg` | Export the CFG of each functions
3 | `contract-summary` | Print a summary of the contracts
4 | `function-summary` | Print a summary of the functions
5 | `human-summary` | Print a human readable summary of the contracts
6 | `inheritance` | Print the inheritance relations between contracts
7 | `inheritance-graph` | Export the inheritance graph of each contract to a dot file
8 | `slithir` | Print the slithIR representation of the functions
9 | `slithir-ssa` | Print the slithIR representation of the functions
10 | `vars-and-auth` | Print the state variables written and the authorization of the functions
## How to install
Slither requires Python 3.6+ and [solc](https://github.com/ethereum/solidity/), the Solidity compiler.

@ -40,3 +40,5 @@ generate_expected_json(){
#generate_expected_json tests/shadowing_abstract.sol "shadowing-abstract"
#generate_expected_json tests/timestamp.sol "timestamp"
#generate_expected_json tests/multiple_calls_in_loop.sol "calls-loop"
#generate_expected_json tests/shadowing_builtin_symbols.sol "shadowing-builtin"
#generate_expected_json tests/shadowing_local_variable.sol "shadowing-local"

@ -28,3 +28,4 @@ generate_expected_json(){
#generate_expected_json tests/inline_assembly_contract-0.5.1.sol "assembly"
#generate_expected_json tests/inline_assembly_library-0.5.1.sol "assembly"
#generate_expected_json tests/constant-0.5.1.sol "constant-function"
#generate_expected_json tests/incorrect_equality.sol "incorrect-equality"

@ -91,5 +91,6 @@ test_slither tests/shadowing_abstract.sol "shadowing-abstract"
test_slither tests/shadowing_state_variable.sol "shadowing-state"
test_slither tests/timestamp.sol "timestamp"
test_slither tests/multiple_calls_in_loop.sol "calls-loop"
test_slither tests/shadowing_builtin_symbols.sol "shadowing-builtin"
test_slither tests/shadowing_local_variable.sol "shadowing-local"

@ -86,7 +86,7 @@ test_slither tests/controlled_delegatecall.sol "controlled-delegatecall"
test_slither tests/constant-0.5.1.sol "constant-function"
test_slither tests/unused_return.sol "unused-return"
test_slither tests/timestamp.sol "timestamp"
test_slither tests/incorrect_equality.sol "incorrect-equality"
### Test scripts

@ -132,9 +132,11 @@ def get_detectors_and_printers():
from slither.detectors.attributes.const_functions import ConstantFunctions
from slither.detectors.shadowing.abstract import ShadowingAbstractDetection
from slither.detectors.shadowing.state import StateShadowing
from slither.detectors.shadowing.local import LocalShadowing
from slither.detectors.shadowing.builtin_symbols import BuiltinSymbolShadowing
from slither.detectors.operations.block_timestamp import Timestamp
from slither.detectors.statements.calls_in_loop import MultipleCallsInLoop
from slither.detectors.statements.incorrect_strict_equality import IncorrectStrictEquality
detectors = [Backdoor,
UninitializedStateVarsDetection,
@ -162,7 +164,10 @@ def get_detectors_and_printers():
ShadowingAbstractDetection,
StateShadowing,
Timestamp,
MultipleCallsInLoop]
MultipleCallsInLoop,
IncorrectStrictEquality,
LocalShadowing,
BuiltinSymbolShadowing]
from slither.printers.summary.function import FunctionSummary
from slither.printers.summary.contract import ContractSummary

@ -112,13 +112,7 @@ class ReentrancyBenign(AbstractDetector):
node.context[self.key] = fathers_context
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]))
state_vars_read = node.state_variables_read
# All the state variables written
state_vars_written = node.state_variables_written
@ -127,10 +121,18 @@ class ReentrancyBenign(AbstractDetector):
# 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))
node.context[self.key]['read'] = list(set(node.context[self.key]['read'] + node.state_variables_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

@ -88,36 +88,33 @@ class ReentrancyEth(AbstractDetector):
# 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':[]}
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']):
return
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':[]}
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
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]))
state_vars_read = node.state_variables_read
# All the state variables written
state_vars_written = node.state_variables_written
@ -126,10 +123,21 @@ class ReentrancyEth(AbstractDetector):
# 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']]
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'] + node.state_variables_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

@ -89,37 +89,33 @@ class ReentrancyReadBeforeWritten(AbstractDetector):
# 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':[]}
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']):
return
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':[]}
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
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]))
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
@ -127,10 +123,20 @@ class ReentrancyReadBeforeWritten(AbstractDetector):
# 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']]
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'] + node.state_variables_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

@ -0,0 +1,133 @@
"""
Module detecting reserved keyword shadowing
"""
from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification
class BuiltinSymbolShadowing(AbstractDetector):
"""
Built-in symbol shadowing
"""
ARGUMENT = 'shadowing-builtin'
HELP = 'Built-in symbol shadowing'
IMPACT = DetectorClassification.LOW
CONFIDENCE = DetectorClassification.HIGH
WIKI = 'https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#builtin-symbol-shadowing'
vuln_name = 'ShadowingBuiltinSymbols'
SHADOWING_FUNCTION = "function"
SHADOWING_MODIFIER = "modifier"
SHADOWING_LOCAL_VARIABLE = "local variable"
SHADOWING_STATE_VARIABLE = "state variable"
SHADOWING_EVENT = "event"
# Reserved keywords reference: https://solidity.readthedocs.io/en/v0.5.2/units-and-global-variables.html
BUILTIN_SYMBOLS = ["assert", "require", "revert", "block", "blockhash",
"gasleft", "msg", "now", "tx", "this", "addmod", "mulmod",
"keccak256", "sha256", "sha3", "ripemd160", "ecrecover",
"selfdestruct", "suicide", "abi"]
# https://solidity.readthedocs.io/en/v0.5.2/miscellaneous.html#reserved-keywords
RESERVED_KEYWORDS = ['abstract', 'after', 'alias', 'apply', 'auto', 'case', 'catch', 'copyof',
'default', 'define', 'final', 'immutable', 'implements', 'in', 'inline',
'let', 'macro', 'match', 'mutable', 'null', 'of', 'override', 'partial',
'promise', 'reference', 'relocatable', 'sealed', 'sizeof', 'static',
'supports', 'switch', 'try', 'type', 'typedef', 'typeof', 'unchecked']
def is_builtin_symbol(self, word):
""" Detects if a given word is a built-in symbol.
Returns:
boolean: True if the given word represents a built-in symbol."""
return word in self.BUILTIN_SYMBOLS or word in self.RESERVED_KEYWORDS
def detect_builtin_shadowing_locals(self, function_or_modifier):
""" Detects if local variables in a given function/modifier are named after built-in symbols.
Any such items are returned in a list.
Returns:
list of tuple: (type, definition, local variable parent)"""
results = []
for local in function_or_modifier.variables:
if self.is_builtin_symbol(local.name):
results.append((self.SHADOWING_LOCAL_VARIABLE, local, function_or_modifier))
return results
def detect_builtin_shadowing_definitions(self, contract):
""" Detects if functions, access modifiers, events, state variables, or local variables are named after built-in
symbols. Any such definitions are returned in a list.
Returns:
list of tuple: (type, definition, [local variable parent])"""
result = []
# Loop through all functions, modifiers, variables (state and local) to detect any built-in symbol keywords.
for function in contract.functions:
if function.contract == contract:
if self.is_builtin_symbol(function.name):
result.append((self.SHADOWING_FUNCTION, function, None))
result += self.detect_builtin_shadowing_locals(function)
for modifier in contract.modifiers:
if modifier.contract == contract:
if self.is_builtin_symbol(modifier.name):
result.append((self.SHADOWING_MODIFIER, modifier, None))
result += self.detect_builtin_shadowing_locals(modifier)
for variable in contract.variables:
if variable.contract == contract:
if self.is_builtin_symbol(variable.name):
result.append((self.SHADOWING_STATE_VARIABLE, variable, None))
for event in contract.events:
if event.contract == contract:
if self.is_builtin_symbol(event.name):
result.append((self.SHADOWING_EVENT, event, None))
return result
def detect(self):
""" Detect shadowing of built-in symbols
Recursively visit the calls
Returns:
list: {'vuln', 'filename,'contract','func', 'shadow'}
"""
results = []
for contract in self.contracts:
shadows = self.detect_builtin_shadowing_definitions(contract)
if shadows:
for shadow in shadows:
# Obtain components
shadow_type = shadow[0]
shadow_object = shadow[1]
local_variable_parent = shadow[2]
# Build the path for our info string
local_variable_path = contract.name + "."
if local_variable_parent is not None:
local_variable_path += local_variable_parent.name + "."
local_variable_path += shadow_object.name
info = '{} ({} @ {}) shadows built-in symbol \"{}"\n'.format(local_variable_path,
shadow_type,
shadow_object.source_mapping_str,
shadow_object.name)
# Print relevant information to the log
self.log(info)
# Generate relevant JSON data for this shadowing definition.
json = self.generate_json_result(info)
if shadow_type in [self.SHADOWING_FUNCTION, self.SHADOWING_MODIFIER, self.SHADOWING_EVENT]:
self.add_function_to_json(shadow_object, json)
elif shadow_type in [self.SHADOWING_STATE_VARIABLE, self.SHADOWING_LOCAL_VARIABLE]:
self.add_variable_to_json(shadow_object, json)
results.append(json)
return results

@ -0,0 +1,109 @@
"""
Module detecting local variable shadowing
"""
from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification
class LocalShadowing(AbstractDetector):
"""
Local variable shadowing
"""
ARGUMENT = 'shadowing-local'
HELP = 'Local variables shadowing'
IMPACT = DetectorClassification.LOW
CONFIDENCE = DetectorClassification.HIGH
WIKI = 'https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#local-variable-shadowing'
vuln_name = 'ShadowingLocalVariable'
OVERSHADOWED_FUNCTION = "function"
OVERSHADOWED_MODIFIER = "modifier"
OVERSHADOWED_STATE_VARIABLE = "state variable"
OVERSHADOWED_EVENT = "event"
def detect_shadowing_definitions(self, contract):
""" Detects if functions, access modifiers, events, state variables, and local variables are named after
reserved keywords. Any such definitions are returned in a list.
Returns:
list of tuple: (type, contract name, definition)"""
result = []
# 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
# This function was declared in this contract, we check what its local variables might shadow.
for variable in function.variables:
overshadowed = []
for scope_contract in [contract] + contract.inheritance:
# Check functions
for scope_function in scope_contract.functions:
if variable.name == scope_function.name and scope_function.contract == scope_contract:
overshadowed.append((self.OVERSHADOWED_FUNCTION, scope_contract.name, scope_function))
# Check modifiers
for scope_modifier in scope_contract.modifiers:
if variable.name == scope_modifier.name and scope_modifier.contract == scope_contract:
overshadowed.append((self.OVERSHADOWED_MODIFIER, scope_contract.name, scope_modifier))
# Check events
for scope_event in scope_contract.events:
if variable.name == scope_event.name and scope_event.contract == scope_contract:
overshadowed.append((self.OVERSHADOWED_EVENT, scope_contract.name, scope_event))
# Check state variables
for scope_state_variable in scope_contract.variables:
if variable.name == scope_state_variable.name and scope_state_variable.contract == scope_contract:
overshadowed.append((self.OVERSHADOWED_STATE_VARIABLE, scope_contract.name, scope_state_variable))
# If we have found any overshadowed objects, we'll want to add it to our result list.
if overshadowed:
result.append((contract.name, function.name, variable, overshadowed))
return result
def detect(self):
""" Detect shadowing local variables
Recursively visit the calls
Returns:
list: {'vuln', 'filename,'contract','func', 'shadow'}
"""
results = []
for contract in self.contracts:
shadows = self.detect_shadowing_definitions(contract)
if shadows:
for shadow in shadows:
local_parent_name = shadow[1]
local_variable = shadow[2]
overshadowed = shadow[3]
info = '{}.{}.{} (local variable @ {}) shadows:\n'.format(contract.name,
local_parent_name,
local_variable.name,
local_variable.source_mapping_str)
for overshadowed_entry in overshadowed:
info += "\t- {}.{} ({} @ {})\n".format(overshadowed_entry[1],
overshadowed_entry[2],
overshadowed_entry[0],
overshadowed_entry[2].source_mapping_str)
# Print relevant information to the log
self.log(info)
# Generate relevant JSON data for this shadowing definition.
json = self.generate_json_result(info)
self.add_variable_to_json(local_variable, json)
for overshadowed_entry in overshadowed:
if overshadowed_entry[0] in [self.OVERSHADOWED_FUNCTION, self.OVERSHADOWED_MODIFIER,
self.OVERSHADOWED_EVENT]:
self.add_function_to_json(overshadowed_entry[2], json)
elif overshadowed_entry[0] == self.OVERSHADOWED_STATE_VARIABLE:
self.add_variable_to_json(overshadowed_entry[2], json)
results.append(json)
return results

@ -0,0 +1,119 @@
"""
Module detecting dangerous strict equality
"""
import itertools
from slither.analyses.data_dependency.data_dependency import is_dependent_ssa
from slither.core.declarations import Function
from slither.detectors.abstract_detector import (AbstractDetector,
DetectorClassification)
from slither.slithir.operations import (Assignment, Balance, Binary, BinaryType,
HighLevelCall)
from slither.core.solidity_types import MappingType, ElementaryType
from slither.core.variables.state_variable import StateVariable
from slither.core.declarations.solidity_variables import SolidityVariable, SolidityVariableComposed
from slither.slithir.variables import ReferenceVariable
class IncorrectStrictEquality(AbstractDetector):
ARGUMENT = 'incorrect-equality'
HELP = 'Dangerous strict equalities'
IMPACT = DetectorClassification.MEDIUM
CONFIDENCE = DetectorClassification.HIGH
WIKI = 'https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#dangerous-strict-equalities'
sources_taint = [SolidityVariable('now'),
SolidityVariableComposed('block.number'),
SolidityVariableComposed('block.timestamp')]
@staticmethod
def is_direct_comparison(ir):
return isinstance(ir, Binary) and ir.type == BinaryType.EQUAL
@staticmethod
def is_any_tainted(variables, taints, function):
return any([is_dependent_ssa(var, taint, function.contract) for var in variables for taint in taints])
def taint_balance_equalities(self, functions):
taints = []
for func in functions:
for node in func.nodes:
for ir in node.irs_ssa:
if isinstance(ir, Balance):
taints.append(ir.lvalue)
if isinstance(ir, HighLevelCall):
#print(ir.function.full_name)
if isinstance(ir.function, Function) and\
ir.function.full_name == 'balanceOf(address)':
taints.append(ir.lvalue)
if isinstance(ir.function, StateVariable) and\
isinstance(ir.function.type, MappingType) and\
ir.function.name == 'balanceOf' and\
ir.function.type.type_from == ElementaryType('address') and\
ir.function.type.type_to == ElementaryType('uint256'):
taints.append(ir.lvalue)
if isinstance(ir, Assignment):
if ir.rvalue in self.sources_taint:
taints.append(ir.lvalue)
return taints
# Retrieve all tainted (node, function) pairs
def tainted_equality_nodes(self, funcs, taints):
results = dict()
taints += self.sources_taint
for func in funcs:
for node in func.nodes:
for ir in node.irs_ssa:
# Filter to only tainted equality (==) comparisons
if self.is_direct_comparison(ir) and self.is_any_tainted(ir.used, taints, func):
if func not in results:
results[func] = []
results[func].append(node)
return results
def detect_strict_equality(self, contract):
funcs = contract.all_functions_called + contract.modifiers
# Taint all BALANCE accesses
taints = self.taint_balance_equalities(funcs)
# Accumulate tainted (node,function) pairs involved in strict equality (==) comparisons
results = self.tainted_equality_nodes(funcs, taints)
return results
def detect(self):
results = []
for c in self.slither.contracts_derived:
ret = self.detect_strict_equality(c)
info = ''
# sort ret to get deterministic results
ret = sorted(list(ret.items()), key=lambda x:x[0].name)
for f, nodes in ret:
info += "{}.{} ({}) uses a dangerous strict equality:\n".format(f.contract.name,
f.name,
f.source_mapping_str)
# sort the nodes to get deterministic results
nodes.sort(key=lambda x: x.node_id)
for node in nodes:
info += "\t- {}\n".format(str(node.expression))
json = self.generate_json_result(info)
self.add_function_to_json(f, json)
self.add_nodes_to_json(nodes, json)
results.append(json)
if info:
self.log(info)
return results

@ -469,7 +469,7 @@ def copy_ir(ir, local_variables_instances, state_variables_instances, temporary_
new_ir.call_value = get_variable(ir, lambda x: ir.call_value)
new_ir.call_gas = get_variable(ir, lambda x: ir.call_gas)
new_ir.arguments = get_arguments(ir)
new_ir.function_instance = ir.function
new_ir.function = ir.function
return new_ir
elif isinstance(ir, Index):
lvalue = get_variable(ir, lambda x: ir.lvalue)

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

@ -0,0 +1 @@
[{"check": "shadowing-local", "impact": "Low", "confidence": "High", "description": "FurtherExtendedContract.shadowingParent.x (local variable @ tests/shadowing_local_variable.sol#25) shadows:\n\t- FurtherExtendedContract.x (state variable @ tests/shadowing_local_variable.sol#17)\n\t- ExtendedContract.x (state variable @ tests/shadowing_local_variable.sol#9)\n\t- BaseContract.x (state variable @ tests/shadowing_local_variable.sol#4)\n", "elements": [{"type": "variable", "name": "x", "source_mapping": {"start": 376, "length": 6, "filename": "tests/shadowing_local_variable.sol", "lines": [25]}}, {"type": "variable", "name": "x", "source_mapping": {"start": 256, "length": 10, "filename": "tests/shadowing_local_variable.sol", "lines": [17]}}, {"type": "variable", "name": "x", "source_mapping": {"start": 133, "length": 10, "filename": "tests/shadowing_local_variable.sol", "lines": [9]}}, {"type": "variable", "name": "x", "source_mapping": {"start": 54, "length": 10, "filename": "tests/shadowing_local_variable.sol", "lines": [4]}}]}, {"check": "shadowing-local", "impact": "Low", "confidence": "High", "description": "FurtherExtendedContract.shadowingParent.y (local variable @ tests/shadowing_local_variable.sol#25) shadows:\n\t- BaseContract.y (state variable @ tests/shadowing_local_variable.sol#5)\n", "elements": [{"type": "variable", "name": "y", "source_mapping": {"start": 398, "length": 5, "filename": "tests/shadowing_local_variable.sol", "lines": [25]}}, {"type": "variable", "name": "y", "source_mapping": {"start": 70, "length": 10, "filename": "tests/shadowing_local_variable.sol", "lines": [5]}}]}, {"check": "shadowing-local", "impact": "Low", "confidence": "High", "description": "FurtherExtendedContract.shadowingParent.z (local variable @ tests/shadowing_local_variable.sol#25) shadows:\n\t- ExtendedContract.z (function @ tests/shadowing_local_variable.sol#11)\n", "elements": [{"type": "variable", "name": "z", "source_mapping": {"start": 405, "length": 6, "filename": "tests/shadowing_local_variable.sol", "lines": [25]}}, {"type": "function", "name": "z", "source_mapping": {"start": 150, "length": 27, "filename": "tests/shadowing_local_variable.sol", "lines": [11]}, "contract": {"type": "contract", "name": "ExtendedContract", "source_mapping": {"start": 85, "length": 110, "filename": "tests/shadowing_local_variable.sol", "lines": [8, 9, 10, 11, 12, 13, 14]}}}]}, {"check": "shadowing-local", "impact": "Low", "confidence": "High", "description": "FurtherExtendedContract.shadowingParent.w (local variable @ tests/shadowing_local_variable.sol#25) shadows:\n\t- FurtherExtendedContract.w (modifier @ tests/shadowing_local_variable.sol#20-23)\n", "elements": [{"type": "variable", "name": "w", "source_mapping": {"start": 413, "length": 6, "filename": "tests/shadowing_local_variable.sol", "lines": [25]}}, {"type": "function", "name": "w", "source_mapping": {"start": 274, "length": 71, "filename": "tests/shadowing_local_variable.sol", "lines": [20, 21, 22, 23]}, "contract": {"type": "contract", "name": "FurtherExtendedContract", "source_mapping": {"start": 197, "length": 235, "filename": "tests/shadowing_local_variable.sol", "lines": [16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26]}}}]}, {"check": "shadowing-local", "impact": "Low", "confidence": "High", "description": "FurtherExtendedContract.shadowingParent.v (local variable @ tests/shadowing_local_variable.sol#25) shadows:\n\t- ExtendedContract.v (event @ tests/shadowing_local_variable.sol#13)\n", "elements": [{"type": "variable", "name": "v", "source_mapping": {"start": 421, "length": 6, "filename": "tests/shadowing_local_variable.sol", "lines": [25]}}, {"type": "function", "name": "v", "source_mapping": {"start": 183, "length": 10, "filename": "tests/shadowing_local_variable.sol", "lines": [13]}, "contract": {"type": "contract", "name": "ExtendedContract", "source_mapping": {"start": 85, "length": 110, "filename": "tests/shadowing_local_variable.sol", "lines": [8, 9, 10, 11, 12, 13, 14]}}}]}]

@ -0,0 +1,136 @@
contract ERC20Function{
function balanceOf(address _owner) external returns(uint);
}
contract ERC20Variable{
mapping(address => uint) public balanceOf;
}
contract ERC20TestBalance{
function good0(ERC20Function erc) external{
require(erc.balanceOf(msg.sender) > 0);
}
function good1(ERC20Variable erc) external{
require(erc.balanceOf(msg.sender) > 0);
}
function bad0(ERC20Function erc) external{
require(erc.balanceOf(address(this)) == 10);
}
function bad1(ERC20Variable erc) external{
require(erc.balanceOf(msg.sender) ==10);
}
}
contract TestContractBalance {
function bad0() external {
require(address(address(this)).balance == 10 ether);
msg.sender.transfer(0.1 ether);
}
function bad1() external {
require(10 ether == address(address(this)).balance);
msg.sender.transfer(0.1 ether);
}
function bad2() external {
require(address(this).balance == 10 ether);
msg.sender.transfer(0.1 ether);
}
function bad3() external {
require(10 ether == address(this).balance);
msg.sender.transfer(0.1 ether);
}
function bad4() external {
uint256 balance = address(this).balance;
if (balance == 10 ether) {
msg.sender.transfer(0.1 ether);
}
}
function bad5() external {
uint256 balance = address(this).balance;
if (10 ether == balance) {
msg.sender.transfer(0.1 ether);
}
}
function bad6() external {
uint256 balance = address(address(this)).balance;
if (balance == 10 ether) {
msg.sender.transfer(0.1 ether);
}
}
function myfunc(uint256 balance) pure internal returns (uint256) {
return balance - balance;
}
function good1() external {
require (address(address(this)).balance >= 10 ether);
msg.sender.transfer(0.1 ether);
}
function good2() external {
require (10 <= address(address(this)).balance);
msg.sender.transfer(0.1 ether);
}
function good3() external {
require (address(this).balance >= 10 ether);
msg.sender.transfer(0.1 ether);
}
function good4() external {
require (10 <= address(this).balance);
msg.sender.transfer(0.1 ether);
}
}
contract TestSolidityKeyword{
function good0() external{
require(now > 0);
}
function good1() external{
require(block.number > 0);
}
function good2() external{
require(block.timestamp > 0);
}
function good3(uint param) public{
// address(this) simulate a particular corner case
// where the SSA is better
// the naive data dependency without SSA
// will consider param and block.number to have a dep
if(param == 0){
param = block.number;
}
}
function bad0() external{
require(now == 0);
}
function bad1() external{
require(block.number== 0);
}
function bad2() external{
require(block.number == 0);
}
}

@ -0,0 +1,34 @@
pragma solidity ^0.4.25;
contract BaseContract {
uint blockhash;
uint now;
event revert(bool condition);
}
contract ExtendedContract is BaseContract {
uint ecrecover = 7;
function assert(bool condition) public {
uint msg;
}
}
contract FurtherExtendedContract is ExtendedContract {
uint blockhash = 7;
uint this = 5;
uint abi;
modifier require {
assert(msg.sender != address(0));
uint keccak256;
uint sha3;
_;
}
}
contract Reserved{
address mutable;
}

@ -0,0 +1,26 @@
pragma solidity ^0.4.24;
contract BaseContract {
uint x = 5;
uint y = 5;
}
contract ExtendedContract is BaseContract {
uint x = 7;
function z() public pure {}
event v();
}
contract FurtherExtendedContract is ExtendedContract {
uint x = 7;
modifier w {
assert(msg.sender != address(0));
_;
}
function shadowingParent(uint x) public pure { int y; uint z; uint w; uint v; }
}
Loading…
Cancel
Save