Merge pull request #380 from crytic/uivlis-const-func/split

Split const_functions into asm and state cases.
pull/386/head
Feist Josselin 5 years ago committed by GitHub
commit 5b90d2fa72
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 3
      scripts/tests_generate_expected_json_4.sh
  2. 3
      scripts/tests_generate_expected_json_5.sh
  3. 3
      scripts/travis_test_4.sh
  4. 3
      scripts/travis_test_5.sh
  5. 3
      slither/detectors/all_detectors.py
  6. 72
      slither/detectors/attributes/const_functions_asm.py
  7. 16
      slither/detectors/attributes/const_functions_state.py
  8. 3
      slither/tools/slither_format/__main__.py
  9. 6
      slither/tools/slither_format/slither_format.py
  10. 2
      tests/constant-0.5.1.sol
  11. 5
      tests/expected_json/constant-0.5.1.constant-function-asm.json
  12. 0
      tests/expected_json/constant-0.5.1.constant-function-asm.txt
  13. 5
      tests/expected_json/constant-0.5.1.constant-function-state.json
  14. 0
      tests/expected_json/constant-0.5.1.constant-function-state.txt
  15. 4
      tests/expected_json/constant-0.5.1.constant-function.txt
  16. 41
      tests/expected_json/constant.constant-function-asm.json
  17. 4
      tests/expected_json/constant.constant-function-asm.txt
  18. 82
      tests/expected_json/constant.constant-function-state.json
  19. 1
      tests/expected_json/constant.constant-function-state.txt

@ -46,7 +46,8 @@ generate_expected_json tests/reentrancy.sol "reentrancy-unlimited-gas"
#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/constant.sol "constant-function-asm"
#generate_expected_json tests/constant.sol "constant-function-state"
#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"

@ -32,7 +32,8 @@ generate_expected_json(){
#generate_expected_json tests/arbitrary_send-0.5.1.sol "arbitrary-send"
#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/constant-0.5.1.sol "constant-function-asm"
#generate_expected_json tests/constant-0.5.1.sol "constant-function-state"
#generate_expected_json tests/incorrect_equality.sol "incorrect-equality"
#generate_expected_json tests/too_many_digits.sol "too-many-digits"
#generate_expected_json tests/unchecked_lowlevel-0.5.1.sol "unchecked-lowlevel"

@ -95,7 +95,8 @@ test_slither tests/naming_convention.sol "naming-convention"
#test_slither tests/complex_func.sol "complex-function"
test_slither tests/controlled_delegatecall.sol "controlled-delegatecall"
test_slither tests/uninitialized_local_variable.sol "uninitialized-local"
test_slither tests/constant.sol "constant-function"
test_slither tests/constant.sol "constant-function-asm"
test_slither tests/constant.sol "constant-function-state"
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"

@ -91,7 +91,8 @@ test_slither tests/external_function_2.sol "external-function"
test_slither tests/naming_convention.sol "naming-convention"
#test_slither tests/complex_func.sol "complex-function"
test_slither tests/controlled_delegatecall.sol "controlled-delegatecall"
test_slither tests/constant-0.5.1.sol "constant-function"
test_slither tests/constant-0.5.1.sol "constant-function-asm"
test_slither tests/constant-0.5.1.sol "constant-function-state"
test_slither tests/unused_return.sol "unused-return"
test_slither tests/timestamp.sol "timestamp"
test_slither tests/incorrect_equality.sol "incorrect-equality"

@ -22,7 +22,8 @@ from .operations.unused_return_values import UnusedReturnValues
from .naming_convention.naming_convention import NamingConvention
from .functions.external_function import ExternalFunction
from .statements.controlled_delegatecall import ControlledDelegateCall
from .attributes.const_functions import ConstantFunctions
from .attributes.const_functions_asm import ConstantFunctionsAsm
from .attributes.const_functions_state import ConstantFunctionsState
from .shadowing.abstract import ShadowingAbstractDetection
from .shadowing.state import StateShadowing
from .shadowing.local import LocalShadowing

@ -0,0 +1,72 @@
"""
Module detecting constant functions
Recursively check the called functions
"""
from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification
from slither.formatters.attributes.const_functions import format
class ConstantFunctionsAsm(AbstractDetector):
"""
Constant function detector
"""
ARGUMENT = 'constant-function-asm' # run the detector with slither.py --ARGUMENT
HELP = 'Constant functions changing the state' # help information
IMPACT = DetectorClassification.MEDIUM
CONFIDENCE = DetectorClassification.MEDIUM
WIKI = 'https://github.com/crytic/slither/wiki/Detector-Documentation#constant-functions-changing-the-state'
WIKI_TITLE = 'Constant functions using assembly code'
WIKI_DESCRIPTION = '''
Functions declared as `constant`/`pure`/`view` using assembly code.
`constant`/`pure`/`view` was not enforced prior Solidity 0.5.
Starting from Solidity 0.5, a call to a `constant`/`pure`/`view` function uses the `STATICCALL` opcode, which reverts in case of state modification.
As a result, a call to an [incorrectly labeled function may trap a contract compiled with Solidity 0.5](https://solidity.readthedocs.io/en/develop/050-breaking-changes.html#interoperability-with-older-contracts).'''
WIKI_EXPLOIT_SCENARIO = '''
```solidity
contract Constant{
uint counter;
function get() public view returns(uint){
counter = counter +1;
return counter
}
}
```
`Constant` was deployed with Solidity 0.4.25. Bob writes a smart contract interacting with `Constant` in Solidity 0.5.0.
All the calls to `get` revert, breaking Bob's smart contract execution.'''
WIKI_RECOMMENDATION = 'Ensure that the attributes of contracts compiled prior to Solidity 0.5.0 are correct.'
def _detect(self):
""" Detect the constant function using assembly code
Recursively visit the calls
Returns:
list: {'vuln', 'filename,'contract','func','#varsWritten'}
"""
results = []
if self.slither.solc_version and self.slither.solc_version >= "0.5.0":
return results
for c in self.contracts:
for f in c.functions:
if f.contract_declarer != c:
continue
if f.view or f.pure:
if f.contains_assembly:
attr = 'view' if f.view else 'pure'
info = [f, f' is declared {attr} but contains assembly code\n']
res = self.generate_result(info, {'contains_assembly': True})
results.append(res)
return results
@staticmethod
def _format(slither, result):
format(slither, result)

@ -6,12 +6,12 @@ from slither.detectors.abstract_detector import AbstractDetector, DetectorClassi
from slither.formatters.attributes.const_functions import format
class ConstantFunctions(AbstractDetector):
class ConstantFunctionsState(AbstractDetector):
"""
Constant function detector
"""
ARGUMENT = 'constant-function' # run the detector with slither.py --ARGUMENT
ARGUMENT = 'constant-function-state' # run the detector with slither.py --ARGUMENT
HELP = 'Constant functions changing the state' # help information
IMPACT = DetectorClassification.MEDIUM
CONFIDENCE = DetectorClassification.MEDIUM
@ -20,7 +20,7 @@ class ConstantFunctions(AbstractDetector):
WIKI_TITLE = 'Constant functions changing the state'
WIKI_DESCRIPTION = '''
Functions declared as `constant`/`pure`/`view` changing the state or using assembly code.
Functions declared as `constant`/`pure`/`view` changing the state.
`constant`/`pure`/`view` was not enforced prior Solidity 0.5.
Starting from Solidity 0.5, a call to a `constant`/`pure`/`view` function uses the `STATICCALL` opcode, which reverts in case of state modification.
@ -50,19 +50,13 @@ All the calls to `get` revert, breaking Bob's smart contract execution.'''
list: {'vuln', 'filename,'contract','func','#varsWritten'}
"""
results = []
if self.slither.solc_version and self.slither.solc_version >= "0.5.0":
return results
for c in self.contracts:
for f in c.functions:
if f.contract_declarer != c:
continue
if f.view or f.pure:
if f.contains_assembly:
attr = 'view' if f.view else 'pure'
info = [f, f' is declared {attr} but contains assembly code\n']
res = self.generate_result(info, {'contains_assembly': True})
results.append(res)
variables_written = f.all_state_variables_written()
if variables_written:
attr = 'view' if f.view else 'pure'

@ -16,7 +16,8 @@ available_detectors = ["unused-state",
"naming-convention",
"external-function",
"constable-states",
"constant-function"]
"constant-function-asm",
"constatnt-function-state"]
detectors_to_run = []

@ -6,7 +6,8 @@ from slither.detectors.attributes.constant_pragma import ConstantPragma
from slither.detectors.naming_convention.naming_convention import NamingConvention
from slither.detectors.functions.external_function import ExternalFunction
from slither.detectors.variables.possible_const_state_variables import ConstCandidateStateVars
from slither.detectors.attributes.const_functions import ConstantFunctions
from slither.detectors.attributes.const_functions_asm import ConstantFunctionsAsm
from slither.detectors.attributes.const_functions_state import ConstantFunctionsState
from slither.utils.colors import yellow
logging.basicConfig(level=logging.INFO)
@ -19,7 +20,8 @@ all_detectors = {
'naming-convention': NamingConvention,
'external-function': ExternalFunction,
'constable-states' : ConstCandidateStateVars,
'constant-function': ConstantFunctions
'constant-function-asm': ConstantFunctionsAsm,
'constant-functions-state': ConstantFunctionsState
}
def slither_format(slither, **kwargs):

@ -1,3 +1,5 @@
pragma solidity 0.5.1;
contract Constant {
uint a;

@ -0,0 +1,5 @@
{
"success": true,
"error": null,
"results": {}
}

@ -0,0 +1,5 @@
{
"success": true,
"error": null,
"results": {}
}

@ -1,4 +0,0 @@

Constant.test_assembly_bug() (tests/constant-0.5.1.sol#15-17) is declared view but contains assembly code
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#constant-functions-changing-the-state
tests/constant-0.5.1.sol analyzed (1 contracts with 1 detectors), 1 result(s) found

@ -9,17 +9,17 @@
"type": "function",
"name": "test_assembly_bug",
"source_mapping": {
"start": 185,
"start": 324,
"length": 66,
"filename_used": "/home/travis/build/crytic/slither/tests/constant-0.5.1.sol",
"filename_relative": "tests/constant-0.5.1.sol",
"filename_absolute": "/home/travis/build/crytic/slither/tests/constant-0.5.1.sol",
"filename_short": "tests/constant-0.5.1.sol",
"filename_used": "/home/travis/build/crytic/slither/tests/constant.sol",
"filename_relative": "tests/constant.sol",
"filename_absolute": "/home/travis/build/crytic/slither/tests/constant.sol",
"filename_short": "tests/constant.sol",
"is_dependency": false,
"lines": [
15,
16,
17
22,
23,
24
],
"starting_column": 5,
"ending_column": 6
@ -30,11 +30,11 @@
"name": "Constant",
"source_mapping": {
"start": 0,
"length": 253,
"filename_used": "/home/travis/build/crytic/slither/tests/constant-0.5.1.sol",
"filename_relative": "tests/constant-0.5.1.sol",
"filename_absolute": "/home/travis/build/crytic/slither/tests/constant-0.5.1.sol",
"filename_short": "tests/constant-0.5.1.sol",
"length": 392,
"filename_used": "/home/travis/build/crytic/slither/tests/constant.sol",
"filename_relative": "tests/constant.sol",
"filename_absolute": "/home/travis/build/crytic/slither/tests/constant.sol",
"filename_short": "tests/constant.sol",
"is_dependency": false,
"lines": [
1,
@ -54,7 +54,14 @@
15,
16,
17,
18
18,
19,
20,
21,
22,
23,
24,
25
],
"starting_column": 1,
"ending_column": 2
@ -64,13 +71,13 @@
}
}
],
"description": "Constant.test_assembly_bug() (tests/constant-0.5.1.sol#15-17) is declared view but contains assembly code\n",
"markdown": "[Constant.test_assembly_bug()](tests/constant-0.5.1.sol#L15-L17) is declared view but contains assembly code\n",
"description": "Constant.test_assembly_bug() (tests/constant.sol#22-24) is declared view but contains assembly code\n",
"markdown": "[Constant.test_assembly_bug()](tests/constant.sol#L22-L24) is declared view but contains assembly code\n",
"id": "1f892cae08b89096bdc4d6ecdf55a3adc4b4314390e054fe2547d9c8e9f76e23",
"additional_fields": {
"contains_assembly": true
},
"check": "constant-function",
"check": "constant-function-asm",
"impact": "Medium",
"confidence": "Medium"
}

@ -0,0 +1,4 @@

Constant.test_assembly_bug() (tests/constant.sol#22-24) is declared view but contains assembly code
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#constant-functions-changing-the-state
tests/constant.sol analyzed (1 contracts with 1 detectors), 3 result(s) found

@ -139,7 +139,7 @@
"additional_fields": {
"contains_assembly": false
},
"check": "constant-function",
"check": "constant-function-state",
"impact": "Medium",
"confidence": "Medium"
},
@ -279,85 +279,7 @@
"additional_fields": {
"contains_assembly": false
},
"check": "constant-function",
"impact": "Medium",
"confidence": "Medium"
},
{
"elements": [
{
"type": "function",
"name": "test_assembly_bug",
"source_mapping": {
"start": 324,
"length": 66,
"filename_used": "/home/travis/build/crytic/slither/tests/constant.sol",
"filename_relative": "tests/constant.sol",
"filename_absolute": "/home/travis/build/crytic/slither/tests/constant.sol",
"filename_short": "tests/constant.sol",
"is_dependency": false,
"lines": [
22,
23,
24
],
"starting_column": 5,
"ending_column": 6
},
"type_specific_fields": {
"parent": {
"type": "contract",
"name": "Constant",
"source_mapping": {
"start": 0,
"length": 392,
"filename_used": "/home/travis/build/crytic/slither/tests/constant.sol",
"filename_relative": "tests/constant.sol",
"filename_absolute": "/home/travis/build/crytic/slither/tests/constant.sol",
"filename_short": "tests/constant.sol",
"is_dependency": false,
"lines": [
1,
2,
3,
4,
5,
6,
7,
8,
9,
10,
11,
12,
13,
14,
15,
16,
17,
18,
19,
20,
21,
22,
23,
24,
25
],
"starting_column": 1,
"ending_column": 2
}
},
"signature": "test_assembly_bug()"
}
}
],
"description": "Constant.test_assembly_bug() (tests/constant.sol#22-24) is declared view but contains assembly code\n",
"markdown": "[Constant.test_assembly_bug()](tests/constant.sol#L22-L24) is declared view but contains assembly code\n",
"id": "1f892cae08b89096bdc4d6ecdf55a3adc4b4314390e054fe2547d9c8e9f76e23",
"additional_fields": {
"contains_assembly": true
},
"check": "constant-function",
"check": "constant-function-state",
"impact": "Medium",
"confidence": "Medium"
}

@ -3,6 +3,5 @@ Constant.test_view_bug() (tests/constant.sol#5-7) is declared view but changes s
- Constant.a (tests/constant.sol#3)
Constant.test_constant_bug() (tests/constant.sol#9-11) is declared view but changes state variables:
- Constant.a (tests/constant.sol#3)
Constant.test_assembly_bug() (tests/constant.sol#22-24) is declared view but contains assembly code
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#constant-functions-changing-the-state
tests/constant.sol analyzed (1 contracts with 1 detectors), 3 result(s) found
Loading…
Cancel
Save