Merge branch 'dev' into dev-fix-external-function-detector

6 years ago · bac034810c
parent b79846a12c e2948fa2b4
commit bac034810c
11 changed files with 46 additions and 30 deletions
--- a/README.md
+++ b/README.md
@ -65,16 +65,16 @@ Num | Detector | What it Detects | Impact | Confidence
 24 | `reentrancy-benign` | [Benign reentrancy vulnerabilities](https://github.com/crytic/slither/wiki/Detector-Documentation#reentrancy-vulnerabilities-2) | Low | Medium
 25 | `timestamp` | [Dangerous usage of `block.timestamp`](https://github.com/crytic/slither/wiki/Detector-Documentation#block-timestamp) | Low | Medium
 26 | `assembly` | [Assembly usage](https://github.com/crytic/slither/wiki/Detector-Documentation#assembly-usage) | Informational | High
-27 | `constable-states` | [State variables that could be declared constant](https://github.com/crytic/slither/wiki/Detector-Documentation#state-variables-that-could-be-declared-constant) | Informational | High
-28 | `deprecated-standards` | [Deprecated Solidity Standards](https://github.com/crytic/slither/wiki/Detector-Documentation#deprecated-standards) | Informational | High
-29 | `erc20-indexed` | [Un-indexed ERC20 event parameters](https://github.com/crytic/slither/wiki/Detector-Documentation#unindexed-erc20-event-parameters) | Informational | High
-30 | `external-function` | [Public function that could be declared as external](https://github.com/crytic/slither/wiki/Detector-Documentation#public-function-that-could-be-declared-as-external) | Informational | High
-31 | `low-level-calls` | [Low level calls](https://github.com/crytic/slither/wiki/Detector-Documentation#low-level-calls) | Informational | High
-32 | `naming-convention` | [Conformance to Solidity naming conventions](https://github.com/crytic/slither/wiki/Detector-Documentation#conformance-to-solidity-naming-conventions) | Informational | High
-33 | `pragma` | [If different pragma directives are used](https://github.com/crytic/slither/wiki/Detector-Documentation#different-pragma-directives-are-used) | Informational | High
-34 | `solc-version` | [Incorrect Solidity version (< 0.4.24 or complex pragma)](https://github.com/crytic/slither/wiki/Detector-Documentation#incorrect-version-of-solidity) | Informational | High
-35 | `unused-state` | [Unused state variables](https://github.com/crytic/slither/wiki/Detector-Documentation#unused-state-variables) | Informational | High
-36 | `too-many-digits` | [Conformance to numeric notation best practices](https://github.com/crytic/slither/wiki/Detector-Documentation#too-many-digits) | Informational | Medium
+27 | `deprecated-standards` | [Deprecated Solidity Standards](https://github.com/crytic/slither/wiki/Detector-Documentation#deprecated-standards) | Informational | High
+28 | `erc20-indexed` | [Un-indexed ERC20 event parameters](https://github.com/crytic/slither/wiki/Detector-Documentation#unindexed-erc20-event-parameters) | Informational | High
+29 | `low-level-calls` | [Low level calls](https://github.com/crytic/slither/wiki/Detector-Documentation#low-level-calls) | Informational | High
+30 | `naming-convention` | [Conformance to Solidity naming conventions](https://github.com/crytic/slither/wiki/Detector-Documentation#conformance-to-solidity-naming-conventions) | Informational | High
+31 | `pragma` | [If different pragma directives are used](https://github.com/crytic/slither/wiki/Detector-Documentation#different-pragma-directives-are-used) | Informational | High
+32 | `solc-version` | [Incorrect Solidity version (< 0.4.24 or complex pragma)](https://github.com/crytic/slither/wiki/Detector-Documentation#incorrect-version-of-solidity) | Informational | High
+33 | `unused-state` | [Unused state variables](https://github.com/crytic/slither/wiki/Detector-Documentation#unused-state-variables) | Informational | High
+34 | `too-many-digits` | [Conformance to numeric notation best practices](https://github.com/crytic/slither/wiki/Detector-Documentation#too-many-digits) | Informational | Medium
+35 | `constable-states` | [State variables that could be declared constant](https://github.com/crytic/slither/wiki/Detector-Documentation#state-variables-that-could-be-declared-constant) | Optimization | High
+36 | `external-function` | [Public function that could be declared as external](https://github.com/crytic/slither/wiki/Detector-Documentation#public-function-that-could-be-declared-as-external) | Optimization | High

 [Contact us](https://www.trailofbits.com/contact/) to get access to additional detectors.

--- a/scripts/tests_generate_expected_json_4.sh
+++ b/scripts/tests_generate_expected_json_4.sh
@ -40,8 +40,8 @@ generate_expected_json(){
 #generate_expected_json tests/inline_assembly_library.sol "assembly"
 #generate_expected_json tests/low_level_calls.sol "low-level-calls"
 #generate_expected_json tests/const_state_variables.sol "constable-states"
-generate_expected_json tests/external_function.sol "external-function"
-generate_expected_json tests/external_function_2.sol "external-function"
+#generate_expected_json tests/external_function.sol "external-function"
+#generate_expected_json tests/external_function_2.sol "external-function"
 #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"
--- a/scripts/travis_test_etherscan.sh
+++ b/scripts/travis_test_etherscan.sh
@ -10,7 +10,7 @@ chmod +x solc-0.4.25

 slither 0x7F37f78cBD74481E593F9C737776F7113d76B315 --solc "./solc-0.4.25"

-if [ $? -ne 6 ]
+if [ $? -ne 5 ]
 then
    echo "Etherscan test failed"
    exit -1
--- a/slither/detectors/abstract_detector.py
+++ b/slither/detectors/abstract_detector.py
@ -14,17 +14,20 @@ class DetectorClassification:
    MEDIUM = 1
    LOW = 2
    INFORMATIONAL = 3
+    OPTIMIZATION = 4


 classification_colors = {
    DetectorClassification.INFORMATIONAL: green,
+    DetectorClassification.OPTIMIZATION: green,
    DetectorClassification.LOW: green,
    DetectorClassification.MEDIUM: yellow,
-    DetectorClassification.HIGH: red,
+    DetectorClassification.HIGH: red
 }

 classification_txt = {
    DetectorClassification.INFORMATIONAL: 'Informational',
+    DetectorClassification.OPTIMIZATION: 'Optimization',
    DetectorClassification.LOW: 'Low',
    DetectorClassification.MEDIUM: 'Medium',
    DetectorClassification.HIGH: 'High',
@ -65,7 +68,8 @@ class AbstractDetector(metaclass=abc.ABCMeta):
        if not self.WIKI_DESCRIPTION:
            raise IncorrectDetectorInitialization('WIKI_DESCRIPTION is not initialized {}'.format(self.__class__.__name__))

-        if not self.WIKI_EXPLOIT_SCENARIO and self.IMPACT != DetectorClassification.INFORMATIONAL:
+        if not self.WIKI_EXPLOIT_SCENARIO and self.IMPACT not in [DetectorClassification.INFORMATIONAL,
+                                                                      DetectorClassification.OPTIMIZATION]:
            raise IncorrectDetectorInitialization('WIKI_EXPLOIT_SCENARIO is not initialized {}'.format(self.__class__.__name__))

        if not self.WIKI_RECOMMENDATION:
@ -77,13 +81,15 @@ class AbstractDetector(metaclass=abc.ABCMeta):
        if self.IMPACT not in [DetectorClassification.LOW,
                                       DetectorClassification.MEDIUM,
                                       DetectorClassification.HIGH,
-                                       DetectorClassification.INFORMATIONAL]:
+                                       DetectorClassification.INFORMATIONAL,
+                                       DetectorClassification.OPTIMIZATION]:
            raise IncorrectDetectorInitialization('IMPACT is not initialized {}'.format(self.__class__.__name__))

        if self.CONFIDENCE not in [DetectorClassification.LOW,
                                       DetectorClassification.MEDIUM,
                                       DetectorClassification.HIGH,
-                                       DetectorClassification.INFORMATIONAL]:
+                                       DetectorClassification.INFORMATIONAL,
+                                       DetectorClassification.OPTIMIZATION]:
            raise IncorrectDetectorInitialization('CONFIDENCE is not initialized {}'.format(self.__class__.__name__))


--- a/slither/detectors/functions/external_function.py
+++ b/slither/detectors/functions/external_function.py
@ -13,7 +13,7 @@ class ExternalFunction(AbstractDetector):

    ARGUMENT = 'external-function'
    HELP = 'Public function that could be declared as external'
-    IMPACT = DetectorClassification.INFORMATIONAL
+    IMPACT = DetectorClassification.OPTIMIZATION
    CONFIDENCE = DetectorClassification.HIGH

    WIKI = 'https://github.com/crytic/slither/wiki/Detector-Documentation#public-function-that-could-be-declared-as-external'
--- a/slither/detectors/naming_convention/naming_convention.py
+++ b/slither/detectors/naming_convention/naming_convention.py
@ -104,6 +104,9 @@ Solidity defines a [naming convention](https://solidity.readthedocs.io/en/v0.4.2
                    results.append(json)

                for argument in func.parameters:
+                    # Ignore parameter names that are not specified i.e. empty strings
+                    if argument.name == "":
+                        continue
                    if argument in func.variables_read_or_written:
                        correct_naming = self.is_mixed_case(argument.name)
                    else:
--- a/slither/detectors/variables/possible_const_state_variables.py
+++ b/slither/detectors/variables/possible_const_state_variables.py
@ -18,7 +18,7 @@ class ConstCandidateStateVars(AbstractDetector):

    ARGUMENT = 'constable-states'
    HELP = 'State variables that could be declared constant'
-    IMPACT = DetectorClassification.INFORMATIONAL
+    IMPACT = DetectorClassification.OPTIMIZATION
    CONFIDENCE = DetectorClassification.HIGH

    WIKI = 'https://github.com/crytic/slither/wiki/Detector-Documentation#state-variables-that-could-be-declared-constant'
--- a/tests/expected_json/const_state_variables.constable-states.json
+++ b/tests/expected_json/const_state_variables.constable-states.json
@ -5,7 +5,7 @@
    "detectors": [
      {
        "check": "constable-states",
-        "impact": "Informational",
+        "impact": "Optimization",
        "confidence": "High",
        "description": "A.myFriendsAddress should be constant (tests/const_state_variables.sol#7)\n",
        "elements": [
@ -66,7 +66,7 @@
      },
      {
        "check": "constable-states",
-        "impact": "Informational",
+        "impact": "Optimization",
        "confidence": "High",
        "description": "A.test should be constant (tests/const_state_variables.sol#10)\n",
        "elements": [
@ -127,7 +127,7 @@
      },
      {
        "check": "constable-states",
-        "impact": "Informational",
+        "impact": "Optimization",
        "confidence": "High",
        "description": "A.text2 should be constant (tests/const_state_variables.sol#14)\n",
        "elements": [
@ -188,7 +188,7 @@
      },
      {
        "check": "constable-states",
-        "impact": "Informational",
+        "impact": "Optimization",
        "confidence": "High",
        "description": "B.mySistersAddress should be constant (tests/const_state_variables.sol#26)\n",
        "elements": [
@ -245,7 +245,7 @@
      },
      {
        "check": "constable-states",
-        "impact": "Informational",
+        "impact": "Optimization",
        "confidence": "High",
        "description": "MyConc.should_be_constant should be constant (tests/const_state_variables.sol#42)\n",
        "elements": [
@ -302,7 +302,7 @@
      },
      {
        "check": "constable-states",
-        "impact": "Informational",
+        "impact": "Optimization",
        "confidence": "High",
        "description": "MyConc.should_be_constant_2 should be constant (tests/const_state_variables.sol#43)\n",
        "elements": [
--- a/tests/expected_json/external_function.external-function.json
+++ b/tests/expected_json/external_function.external-function.json
@ -5,7 +5,7 @@
    "detectors": [
      {
        "check": "external-function",
-        "impact": "Informational",
+        "impact": "Optimization",
        "confidence": "High",
        "description": "ContractWithFunctionNotCalled.funcNotCalled3() (tests/external_function.sol#13-15) should be declared external\n",
        "elements": [
@ -70,7 +70,7 @@
      },
      {
        "check": "external-function",
-        "impact": "Informational",
+        "impact": "Optimization",
        "confidence": "High",
        "description": "ContractWithFunctionNotCalled.funcNotCalled2() (tests/external_function.sol#17-19) should be declared external\n",
        "elements": [
@ -135,7 +135,7 @@
      },
      {
        "check": "external-function",
-        "impact": "Informational",
+        "impact": "Optimization",
        "confidence": "High",
        "description": "ContractWithFunctionNotCalled.funcNotCalled() (tests/external_function.sol#21-23) should be declared external\n",
        "elements": [
@ -200,7 +200,7 @@
      },
      {
        "check": "external-function",
-        "impact": "Informational",
+        "impact": "Optimization",
        "confidence": "High",
        "description": "ContractWithFunctionNotCalled2.funcNotCalled() (tests/external_function.sol#32-39) should be declared external\n",
        "elements": [
--- a/tests/expected_json/naming_convention.naming-convention.txt
+++ b/tests/expected_json/naming_convention.naming-convention.txt
@ -12,4 +12,4 @@ Parameter '_used' of _used (tests/naming_convention.sol#59) is not in mixedCase
 Variable 'T._myPublicVar' (tests/naming_convention.sol#56) is not in mixedCase
 Variable 'T.l' (tests/naming_convention.sol#67) used l, O, I, which should not be used
 Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#conformance-to-solidity-naming-conventions[0m
-INFO:Slither:tests/naming_convention.sol analyzed (3 contracts), 12 result(s) found
+INFO:Slither:tests/naming_convention.sol analyzed (4 contracts), 12 result(s) found
--- a/tests/naming_convention.sol
+++ b/tests/naming_convention.sol
@ -66,3 +66,10 @@ contract T {

    uint l = 1;
 }
+
+contract ParameterNameEmptyString {
+
+  function foo (uint) public {
+  }
+  
+}