diff --git a/README.md b/README.md index 1c654acd5..8fe629cd7 100644 --- a/README.md +++ b/README.md @@ -47,21 +47,21 @@ By default, all the detectors are run. Num | Detector | What it Detects | Impact | Confidence --- | --- | --- | --- | --- -1 | `suicidal` | Suicidal functions | High | High -2 | `uninitialized-state` | Uninitialized state variables | High | High -3 | `uninitialized-storage` | Uninitialized storage variables | High | High -4 | `arbitrary-send` | Functions that send ether to arbitrary destinations | High | Medium -5 | `reentrancy` | Reentrancy vulnerabilities | High | Medium -6 | `locked-ether` | Contracts that lock ether | Medium | High -7 | `tx-origin` | Dangerous usage of `tx.origin` | Medium | Medium -8 | `assembly` | Assembly usage | Informational | High -9 | `constable-states` | State variables that could be declared constant | Informational | High -10 | `external-function` | Public function that could be declared as external | Informational | High -11 | `low-level-calls` | Low level calls | Informational | High -12 | `naming-convention` | Conformance to Solidity naming conventions | Informational | High -13 | `pragma` | If different pragma directives are used | Informational | High -14 | `solc-version` | Old versions of Solidity (< 0.4.23) | Informational | High -15 | `unused-state` | Unused state variables | Informational | High +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 | `reentrancy` | [Reentrancy vulnerabilities](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#reentrancy-vulnerabilities) | High | Medium +6 | `locked-ether` | [Contracts that lock ether](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#contracts-that-lock-ether) | Medium | High +7 | `tx-origin` | [Dangerous usage of `tx.origin`](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#dangerous-usage-of-txorigin) | Medium | Medium +8 | `assembly` | [Assembly usage](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#assembly-usage) | Informational | High +9 | `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 +10 | `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 +11 | `low-level-calls` | [Low level calls](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#low-level-calls) | Informational | High +12 | `naming-convention` | [Conformance to Solidity naming conventions](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#conformance-to-solidity-naming-conventions) | Informational | High +13 | `pragma` | [If different pragma directives are used](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#state-variables-that-could-be-declared-constant) | Informational | High +14 | `solc-version` | [Old versions of Solidity (< 0.4.23)](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#old-versions-of-solidity) | Informational | High +15 | `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. diff --git a/slither/__main__.py b/slither/__main__.py index 017bebaa6..f287ae679 100644 --- a/slither/__main__.py +++ b/slither/__main__.py @@ -65,17 +65,15 @@ def process_truffle(dirname, args, detector_classes, printer_classes): filenames = glob.glob(os.path.join(dirname,'build','contracts', '*.json')) all_contracts = [] + all_filenames = [] for filename in filenames: with open(filename) as f: contract_loaded = json.load(f) - all_contracts += contract_loaded['ast']['nodes'] + all_contracts.append(contract_loaded['ast']) + all_filenames.append(contract_loaded['sourcePath']) - contract = { - "nodeType": "SourceUnit", - "nodes" : all_contracts} - - slither = Slither(contract, args.solc, args.disable_solc_warnings, args.solc_args) + slither = Slither(all_contracts, args.solc, args.disable_solc_warnings, args.solc_args) return _process(slither, detector_classes, printer_classes) diff --git a/slither/core/declarations/import_directive.py b/slither/core/declarations/import_directive.py index 069558257..8e22eb8c9 100644 --- a/slither/core/declarations/import_directive.py +++ b/slither/core/declarations/import_directive.py @@ -4,7 +4,7 @@ class Import(SourceMapping): def __init__(self, filename): super(Import, self).__init__() - self._fimename = filename + self._filename = filename @property def filename(self): diff --git a/slither/core/declarations/pragma_directive.py b/slither/core/declarations/pragma_directive.py index 4a949cd0d..1747b9229 100644 --- a/slither/core/declarations/pragma_directive.py +++ b/slither/core/declarations/pragma_directive.py @@ -18,4 +18,4 @@ class Pragma(SourceMapping): return ''.join(self.directive[1:]) def __str__(self): - return 'pragma '+str(self.directive) + return 'pragma '+''.join(self.directive) diff --git a/slither/core/slither_core.py b/slither/core/slither_core.py index ba8342237..cd8f8a113 100644 --- a/slither/core/slither_core.py +++ b/slither/core/slither_core.py @@ -17,6 +17,7 @@ class Slither(Context): self._solc_version = None # '0.3' or '0.4':! self._pragma_directives = [] self._import_directives = [] + self._raw_source_code = {} @property def source_units(self): @@ -58,6 +59,11 @@ class Slither(Context): """ list(str): Import directives""" return self._import_directives + @property + def source_code(self): + """ {filename: source_code}: source code """ + return self._raw_source_code + def get_contract_from_name(self, contract_name): """ Return a contract from a name diff --git a/slither/core/source_mapping/source_mapping.py b/slither/core/source_mapping/source_mapping.py index fc546d425..88d66db5f 100644 --- a/slither/core/source_mapping/source_mapping.py +++ b/slither/core/source_mapping/source_mapping.py @@ -11,6 +11,26 @@ class SourceMapping(Context): def source_mapping(self): return self._source_mapping + @staticmethod + def _compute_line(source_code, start, length): + """ + Compute line(s) number from a start/end offset + Not done in an efficient way + """ + total_length = len(source_code) + source_code = source_code.split('\n') + counter = 0 + i = 0 + lines = [] + while counter < total_length: + counter += len(source_code[i]) +1 + i = i+1 + if counter > start: + lines.append(i) + if counter > start+length: + break + return lines + @staticmethod def _convert_source_mapping(offset, slither): ''' @@ -33,8 +53,37 @@ class SourceMapping(Context): if f not in sourceUnits: return {'start':s, 'length':l} filename = sourceUnits[f] - return {'start':s, 'length':l, 'filename': filename} + + lines = [] + + if filename in slither.source_code: + lines = SourceMapping._compute_line(slither.source_code[filename], s, l) + + return {'start':s, 'length':l, 'filename': filename, 'lines' : lines } def set_offset(self, offset, slither): - self._source_mapping = self._convert_source_mapping(offset, slither) + if isinstance(offset, dict): + self._source_mapping = offset + else: + self._source_mapping = self._convert_source_mapping(offset, slither) + + + @property + def source_mapping_str(self): + + def relative_path(path): + # Remove absolute path for printing + # Truffle returns absolutePath + if '/contracts/' in path: + return path[path.find('/contracts/'):] + return path + + lines = self.source_mapping['lines'] + if not lines: + lines = '' + elif len(lines) == 1: + lines = '#{}'.format(lines[0]) + else: + lines = '#{}-{}'.format(lines[0], lines[-1]) + return '{}{}'.format(relative_path(self.source_mapping['filename']), lines) diff --git a/slither/detectors/abstract_detector.py b/slither/detectors/abstract_detector.py index 2575bc652..d0015d09b 100644 --- a/slither/detectors/abstract_detector.py +++ b/slither/detectors/abstract_detector.py @@ -35,6 +35,8 @@ class AbstractDetector(metaclass=abc.ABCMeta): IMPACT = None CONFIDENCE = None + WIKI = '' + def __init__(self, slither, logger): self.slither = slither self.contracts = slither.contracts @@ -64,7 +66,9 @@ class AbstractDetector(metaclass=abc.ABCMeta): def log(self, info): if self.logger: - info = " "+info + info = "\n"+info + if self.WIKI != '': + info += 'Reference: {}'.format(self.WIKI) self.logger.info(self.color(info)) @abc.abstractmethod diff --git a/slither/detectors/attributes/constant_pragma.py b/slither/detectors/attributes/constant_pragma.py index 7a1e272c0..0d96e93e9 100644 --- a/slither/detectors/attributes/constant_pragma.py +++ b/slither/detectors/attributes/constant_pragma.py @@ -15,6 +15,8 @@ class ConstantPragma(AbstractDetector): IMPACT = DetectorClassification.INFORMATIONAL CONFIDENCE = DetectorClassification.HIGH + WIKI = 'https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#state-variables-that-could-be-declared-constant' + def detect(self): results = [] pragma = self.slither.pragma_directives @@ -22,7 +24,9 @@ class ConstantPragma(AbstractDetector): versions = list(set(versions)) if len(versions) > 1: - info = "Different version of Solidity used in {}: {}".format(self.filename, versions) + info = "Different versions of Solidity is used in {}:\n".format(self.filename) + for p in pragma: + info += "\t- {} declares {}\n".format(p.source_mapping_str, str(p)) self.log(info) source = [p.source_mapping for p in pragma] diff --git a/slither/detectors/attributes/locked_ether.py b/slither/detectors/attributes/locked_ether.py index 682224f02..7f3b75024 100644 --- a/slither/detectors/attributes/locked_ether.py +++ b/slither/detectors/attributes/locked_ether.py @@ -17,6 +17,8 @@ class LockedEther(AbstractDetector): IMPACT = DetectorClassification.MEDIUM CONFIDENCE = DetectorClassification.HIGH + WIKI = 'https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#contracts-that-lock-ether' + @staticmethod def do_no_send_ether(contract): functions = contract.all_functions_called @@ -44,7 +46,11 @@ class LockedEther(AbstractDetector): funcs_payable = [function for function in contract.functions if function.payable] if funcs_payable: if self.do_no_send_ether(contract): - txt = "Contract locked ether in {}, Contract {}, Functions {}" + txt = "Contract locking ether found in {}:\n".format(self.filename) + txt += "\tContract {} has payable functions:\n".format(contract.name) + for function in funcs_payable: + txt += "\t - {} ({})\n".format(function.name, function.source_mapping_str) + txt += "\tBut has not function to withdraw the ether\n" info = txt.format(self.filename, contract.name, [f.name for f in funcs_payable]) diff --git a/slither/detectors/attributes/old_solc.py b/slither/detectors/attributes/old_solc.py index a14a1df50..dc6bd51c8 100644 --- a/slither/detectors/attributes/old_solc.py +++ b/slither/detectors/attributes/old_solc.py @@ -16,16 +16,21 @@ class OldSolc(AbstractDetector): IMPACT = DetectorClassification.INFORMATIONAL CONFIDENCE = DetectorClassification.HIGH + WIKI = 'https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#old-versions-of-solidity' + + @staticmethod + def _convert_pragma(version): + return version.replace('solidity', '').replace('^', '') + def detect(self): results = [] pragma = self.slither.pragma_directives - versions = [p.version for p in pragma] - versions = [p.replace('solidity', '').replace('^', '') for p in versions] - versions = list(set(versions)) - old_pragma = [p for p in versions if p not in ['0.4.23', '0.4.24']] + old_pragma = [p for p in pragma if self._convert_pragma(p.version) not in ['0.4.23', '0.4.24']] if old_pragma: - info = "Old version of Solidity used in {}: {}".format(self.filename, old_pragma) + info = "Old version (<0.4.23) of Solidity used in {}:\n".format(self.filename) + for p in old_pragma: + info += "\t- {} declares {}\n".format(p.source_mapping_str, str(p)) self.log(info) source = [p.source_mapping for p in pragma] diff --git a/slither/detectors/examples/backdoor.py b/slither/detectors/examples/backdoor.py index 9e29724f9..76fe19fcc 100644 --- a/slither/detectors/examples/backdoor.py +++ b/slither/detectors/examples/backdoor.py @@ -19,7 +19,8 @@ class Backdoor(AbstractDetector): for f in contract.functions: if 'backdoor' in f.name: # Info to be printed - info = 'Backdoor function found in {}.{}'.format(contract.name, f.name) + info = 'Backdoor function found in {}.{} ({})\n' + info = info.format(contract.name, f.name, f.source_mapping_str) # Print the info self.log(info) # Add the result in ret diff --git a/slither/detectors/functions/arbitrary_send.py b/slither/detectors/functions/arbitrary_send.py index 920351d41..e509f5d16 100644 --- a/slither/detectors/functions/arbitrary_send.py +++ b/slither/detectors/functions/arbitrary_send.py @@ -31,6 +31,8 @@ class ArbitrarySend(AbstractDetector): IMPACT = DetectorClassification.HIGH CONFIDENCE = DetectorClassification.MEDIUM + WIKI = 'https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#functions-that-send-ether-to-arbitrary-destinations' + @staticmethod def arbitrary_send(func): """ @@ -97,24 +99,24 @@ class ArbitrarySend(AbstractDetector): for c in self.contracts: arbitrary_send = self.detect_arbitrary_send(c) for (func, nodes) in arbitrary_send: - func_name = func.name calls_str = [str(node.expression) for node in nodes] - txt = "Arbitrary send in {} Contract: {}, Function: {}, Calls: {}" - info = txt.format(self.filename, - c.name, - func_name, - calls_str) + info = "{}{} sends eth to arbirary user\n" + info = info.format(func.contract.name, + func.name) + info += '\tDangerous calls:\n' + for node in nodes: + info += '\t- {} ({})\n'.format(node.expression, node.source_mapping_str) self.log(info) source_mapping = [node.source_mapping for node in nodes] - results.append({'vuln': 'SuicidalFunc', + results.append({'vuln': 'ArbitrarySend', 'sourceMapping': source_mapping, 'filename': self.filename, - 'contract': c.name, - 'func': func_name, + 'contract': func.contract.name, + 'func': func.name, 'calls': calls_str}) return results diff --git a/slither/detectors/functions/complex_function.py b/slither/detectors/functions/complex_function.py index 355f1865b..ab1260e21 100644 --- a/slither/detectors/functions/complex_function.py +++ b/slither/detectors/functions/complex_function.py @@ -90,19 +90,21 @@ class ComplexFunction(AbstractDetector): for issue in issues: func, cause = issue.values() func_name = func.name - - txt = "Complex function in {} Contract: {}, Function: {}" + + txt = "Complex function in {}\n\t- {}.{} ({})\n" if cause == self.CAUSE_EXTERNAL_CALL: - txt += ", Reason: High number of external calls" + txt += "\t- Reason: High number of external calls" if cause == self.CAUSE_CYCLOMATIC: - txt += ", Reason: High number of branches" + txt += "\t- Reason: High number of branches" if cause == self.CAUSE_STATE_VARS: - txt += ", Reason: High number of modified state variables" + txt += "\t- Reason: High number of modified state variables" info = txt.format(self.filename, contract.name, - func_name) + func_name, + func.source_mapping_str) + info = info + "\n" self.log(info) results.append({'vuln': 'ComplexFunc', diff --git a/slither/detectors/functions/external_function.py b/slither/detectors/functions/external_function.py index 99ffe40fe..d2f1cc322 100644 --- a/slither/detectors/functions/external_function.py +++ b/slither/detectors/functions/external_function.py @@ -16,6 +16,8 @@ class ExternalFunction(AbstractDetector): IMPACT = DetectorClassification.INFORMATIONAL CONFIDENCE = DetectorClassification.HIGH + WIKI = 'https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#public-function-that-could-be-declared-as-external' + @staticmethod def detect_functions_called(contract): """ Returns a list of InternallCall, SolidityCall @@ -45,6 +47,7 @@ class ExternalFunction(AbstractDetector): results = [] public_function_calls = [] + all_info = '' for contract in self.slither.contracts_derived: if self._contains_internal_dynamic_call(contract): @@ -56,15 +59,16 @@ class ExternalFunction(AbstractDetector): for func in [f for f in contract.functions if f.visibility == 'public' and\ not f in public_function_calls and\ not f.is_constructor]: - func_name = func.name - txt = "Public function in {} Contract: {}, Function: {} should be declared external" - info = txt.format(self.filename, - contract.name, - func_name) - self.log(info) + txt = "{}.{} ({}) should be declared external\n" + info = txt.format(func.contract.name, + func.name, + func.source_mapping_str) + all_info += info results.append({'vuln': 'ExternalFunc', 'sourceMapping': func.source_mapping, 'filename': self.filename, - 'contract': contract.name, - 'func': func_name}) + 'contract': func.contract.name, + 'func': func.name}) + if all_info != '': + self.log(all_info) return results diff --git a/slither/detectors/functions/suicidal.py b/slither/detectors/functions/suicidal.py index 41b6f9bc5..8cac68977 100644 --- a/slither/detectors/functions/suicidal.py +++ b/slither/detectors/functions/suicidal.py @@ -12,10 +12,12 @@ class Suicidal(AbstractDetector): """ ARGUMENT = 'suicidal' - HELP = 'Suicidal functions' + HELP = 'Functions allowing anyone to destruct the contract' IMPACT = DetectorClassification.HIGH CONFIDENCE = DetectorClassification.HIGH + WIKI = 'https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#suicidal' + @staticmethod def detect_suicidal_func(func): """ Detect if the function is suicidal @@ -54,12 +56,11 @@ class Suicidal(AbstractDetector): for c in self.contracts: functions = self.detect_suicidal(c) for func in functions: - func_name = func.name - txt = "Suicidal function in {} Contract: {}, Function: {}" - info = txt.format(self.filename, - c.name, - func_name) + txt = "{}.{} ({}) allows anyone to destruct the contract\n" + info = txt.format(func.contract.name, + func.name, + func.source_mapping_str) self.log(info) @@ -67,6 +68,6 @@ class Suicidal(AbstractDetector): 'sourceMapping': func.source_mapping, 'filename': self.filename, 'contract': c.name, - 'func': func_name}) + 'func': func.name}) return results diff --git a/slither/detectors/naming_convention/naming_convention.py b/slither/detectors/naming_convention/naming_convention.py index f3312c1f0..a2f37ef84 100644 --- a/slither/detectors/naming_convention/naming_convention.py +++ b/slither/detectors/naming_convention/naming_convention.py @@ -17,6 +17,8 @@ class NamingConvention(AbstractDetector): IMPACT = DetectorClassification.INFORMATIONAL CONFIDENCE = DetectorClassification.HIGH + WIKI = 'https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#conformance-to-solidity-naming-conventions' + @staticmethod def is_cap_words(name): return re.search('^[A-Z]([A-Za-z0-9]+)?_?$', name) is not None @@ -42,11 +44,12 @@ class NamingConvention(AbstractDetector): def detect(self): results = [] + all_info = '' for contract in self.contracts: if not self.is_cap_words(contract.name): - info = "Contract '{}' is not in CapWords".format(contract.name) - self.log(info) + info = "Contract '{}' ({}) is not in CapWords\n".format(contract.name, contract.source_mapping_str) + all_info += info results.append({'vuln': 'NamingConvention', 'filename': self.filename, @@ -58,8 +61,9 @@ class NamingConvention(AbstractDetector): continue if not self.is_cap_words(struct.name): - info = "Struct '{}' is not in CapWords, Contract: '{}' ".format(struct.name, contract.name) - self.log(info) + info = "Struct '{}.{}' ({}) is not in CapWords\n" + info = info.format(struct.contract.name, struct.name, struct.source_mapping_str) + all_info += info results.append({'vuln': 'NamingConvention', 'filename': self.filename, @@ -72,8 +76,9 @@ class NamingConvention(AbstractDetector): continue if not self.is_cap_words(event.name): - info = "Event '{}' is not in CapWords, Contract: '{}' ".format(event.name, contract.name) - self.log(info) + info = "Event '{}.{}' ({}) is not in CapWords\n" + info = info.format(event.contract.name, event.name, event.source_mapping_str) + all_info += info results.append({'vuln': 'NamingConvention', 'filename': self.filename, @@ -86,8 +91,9 @@ class NamingConvention(AbstractDetector): continue if not self.is_mixed_case(func.name): - info = "Function '{}' is not in mixedCase, Contract: '{}' ".format(func.name, contract.name) - self.log(info) + info = "Function '{}.{}' ({}) is not in mixedCase\n" + info = info.format(func.contract.name, func.name, func.source_mapping_str) + all_info += info results.append({'vuln': 'NamingConvention', 'filename': self.filename, @@ -101,9 +107,12 @@ class NamingConvention(AbstractDetector): else: correct_naming = self.is_mixed_case_with_underscore(argument.name) if not correct_naming: - info = "Parameter '{}' is not in mixedCase, Contract: '{}', Function: '{}'' " \ - .format(argument.name, argument.name, contract.name) - self.log(info) + info = "Parameter '{}' of {}.{} ({}) is not in mixedCase\n" + info = info.format(argument.name, + argument.function.contract.name, + argument.function, + argument.source_mapping_str) + all_info += info results.append({'vuln': 'NamingConvention', 'filename': self.filename, @@ -118,9 +127,9 @@ class NamingConvention(AbstractDetector): if self.should_avoid_name(var.name): if not self.is_upper_case_with_underscores(var.name): - info = "Variable '{}' l, O, I should not be used, Contract: '{}' " \ - .format(var.name, contract.name) - self.log(info) + info = "Variable '{}.{}' ({}) used l, O, I, which should not be used\n" + info = info.format(var.contract.name, var.name, var.source_mapping_str) + all_info += info results.append({'vuln': 'NamingConvention', 'filename': self.filename, @@ -134,9 +143,9 @@ class NamingConvention(AbstractDetector): continue if not self.is_upper_case_with_underscores(var.name): - info = "Constant '{}' is not in UPPER_CASE_WITH_UNDERSCORES, Contract: '{}' " \ - .format(var.name, contract.name) - self.log(info) + info = "Constant '{}.{}' ({}) is not in UPPER_CASE_WITH_UNDERSCORES\n" + info = info.format(var.contract.name, var.name, var.source_mapping_str) + all_info += info results.append({'vuln': 'NamingConvention', 'filename': self.filename, @@ -149,8 +158,10 @@ class NamingConvention(AbstractDetector): else: correct_naming = self.is_mixed_case(var.name) if not correct_naming: - info = "Variable '{}' is not in mixedCase, Contract: '{}' ".format(var.name, contract.name) - self.log(info) + info = "Variable '{}.{}' ({}) is not in mixedCase\n" + info = info.format(var.contract.name, var.name, var.source_mapping_str) + all_info += info + results.append({'vuln': 'NamingConvention', 'filename': self.filename, @@ -163,8 +174,9 @@ class NamingConvention(AbstractDetector): continue if not self.is_cap_words(enum.name): - info = "Enum '{}' is not in CapWords, Contract: '{}' ".format(enum.name, contract.name) - self.log(info) + info = "Enum '{}.{}' ({}) is not in CapWords\n" + info = info.format(enum.contract.name, enum.name, enum.source_mapping_str) + all_info += info results.append({'vuln': 'NamingConvention', 'filename': self.filename, @@ -177,13 +189,16 @@ class NamingConvention(AbstractDetector): continue if not self.is_mixed_case(modifier.name): - info = "Modifier '{}' is not in mixedCase, Contract: '{}' ".format(modifier.name, contract.name) - self.log(info) + info = "Modifier '{}.{}' ({}) is not in mixedCase\n" + info = info.format(modifier.contract.name, modifier.name, modifier.source_mapping_str) + all_info += info results.append({'vuln': 'NamingConvention', 'filename': self.filename, 'contract': contract.name, 'modifier': modifier.name, 'sourceMapping': modifier.source_mapping}) + if all_info != '': + self.log(all_info) return results diff --git a/slither/detectors/operations/low_level_calls.py b/slither/detectors/operations/low_level_calls.py index ec0f546a7..509536244 100644 --- a/slither/detectors/operations/low_level_calls.py +++ b/slither/detectors/operations/low_level_calls.py @@ -16,6 +16,8 @@ class LowLevelCalls(AbstractDetector): IMPACT = DetectorClassification.INFORMATIONAL CONFIDENCE = DetectorClassification.HIGH + WIKI = 'https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#low-level-calls' + @staticmethod def _contains_low_level_calls(node): """ @@ -27,7 +29,7 @@ class LowLevelCalls(AbstractDetector): def detect_low_level_calls(self, contract): ret = [] - for f in contract.functions: + for f in [f for f in contract.functions if contract == f.contract]: nodes = f.nodes assembly_nodes = [n for n in nodes if self._contains_low_level_calls(n)] @@ -39,21 +41,22 @@ class LowLevelCalls(AbstractDetector): """ Detect the functions that use low level calls """ results = [] + all_info = '' for c in self.contracts: values = self.detect_low_level_calls(c) for func, nodes in values: - func_name = func.name - info = "Low level call in %s, Contract: %s, Function: %s" % (self.filename, - c.name, - func_name) - self.log(info) + info = "Low level call in {}.{} ({})\n" + info = info.format(func.contract.name, func.name, func.source_mapping_str) + all_info += info sourceMapping = [n.source_mapping for n in nodes] results.append({'vuln': 'Low level call', 'sourceMapping': sourceMapping, 'filename': self.filename, - 'contract': c.name, - 'function_name': func_name}) + 'contract': func.contract.name, + 'function_name': func.name}) + if all_info != '': + self.log(all_info) return results diff --git a/slither/detectors/reentrancy/reentrancy.py b/slither/detectors/reentrancy/reentrancy.py index e5c9966a7..847cdc17c 100644 --- a/slither/detectors/reentrancy/reentrancy.py +++ b/slither/detectors/reentrancy/reentrancy.py @@ -21,6 +21,8 @@ class Reentrancy(AbstractDetector): IMPACT = DetectorClassification.HIGH CONFIDENCE = DetectorClassification.MEDIUM + WIKI = 'https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#reentrancy-vulnerabilities' + key = 'REENTRANCY' @staticmethod @@ -126,7 +128,7 @@ class Reentrancy(AbstractDetector): if isinstance(internal_call, Function): state_vars_written += internal_call.all_state_variables_written() - read_then_written = [v for v in state_vars_written if v in node.context[self.key]['read']] + read_then_written = [(v, node.source_mapping_str) for v in state_vars_written if v in node.context[self.key]['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 @@ -136,8 +138,7 @@ class Reentrancy(AbstractDetector): node.context[self.key]['calls'] and node.context[self.key]['send_eth']): # calls are ordered - finding_key = (node.function.contract.name, - node.function.full_name, + finding_key = (node.function, tuple(set(node.context[self.key]['calls'])), tuple(set(node.context[self.key]['send_eth']))) finding_vars = read_then_written @@ -176,32 +177,38 @@ class Reentrancy(AbstractDetector): results = [] - for (contract, func, calls, send_eth), varsWritten in self.result.items(): - varsWritten_str = list(set([str(x) for x in list(varsWritten)])) - calls_str = list(set([str(x.expression) for x in list(calls)])) - send_eth_str = list(set([str(x.expression) for x in list(send_eth)])) - - if calls == send_eth: - call_info = 'Call: {},'.format(calls_str) - else: - call_info = 'Call: {}, Ether sent: {},'.format(calls_str, send_eth_str) - info = 'Reentrancy in {}, Contract: {}, '.format(self.filename, contract) + \ - 'Func: {}, '.format(func) + \ - '{}'.format(call_info) + \ - 'Vars Written: {}'.format(str(varsWritten_str)) + for (func, calls, send_eth), varsWritten in self.result.items(): + calls = list(set(calls)) + send_eth = list(set(send_eth)) +# if calls == send_eth: +# calls_info = 'Call: {},'.format(calls_str) +# else: +# calls_info = 'Call: {}, Ether sent: {},'.format(calls_str, send_eth_str) + info = 'Reentrancy in {}.{} ({}):\n' + info = info.format(func.contract.name, func.name, func.source_mapping_str) + info += '\tExternal calls:\n' + for call_info in calls: + info += '\t- {} ({})\n'.format(call_info.expression, call_info.source_mapping_str) + if calls != send_eth: + info += '\tExternal calls sending eth:\n' + for call_info in send_eth: + info += '\t- {} ({})\n'.format(call_info.expression, call_info.source_mapping_str) + info += '\tState variables written after the call(s):\n' + for (v, mapping) in varsWritten: + info += '\t- {} ({})\n'.format(v, mapping) self.log(info) - source = [v.source_mapping for v in varsWritten] + source = [v.source_mapping for (v,_) in varsWritten] source += [node.source_mapping for node in calls] source += [node.source_mapping for node in send_eth] results.append({'vuln': 'Reentrancy', 'sourceMapping': source, 'filename': self.filename, - 'contract': contract, - 'function_name': func, - 'calls': calls_str, - 'send_eth': send_eth_str, - 'varsWritten': varsWritten_str}) + 'contract': func.contract.name, + 'function_name': func.name, + 'calls': [str(x.expression) for x in calls], + 'send_eth': [str(x.expression) for x in send_eth], + 'varsWritten': [str(x) for (x,_) in varsWritten]}) return results diff --git a/slither/detectors/statements/assembly.py b/slither/detectors/statements/assembly.py index 93e9ea598..a10697d3a 100644 --- a/slither/detectors/statements/assembly.py +++ b/slither/detectors/statements/assembly.py @@ -16,6 +16,8 @@ class Assembly(AbstractDetector): IMPACT = DetectorClassification.INFORMATIONAL CONFIDENCE = DetectorClassification.HIGH + WIKI = 'https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#assembly-usage' + @staticmethod def _contains_inline_assembly_use(node): """ @@ -39,21 +41,22 @@ class Assembly(AbstractDetector): """ Detect the functions that use inline assembly """ results = [] + all_info = '' for c in self.contracts: values = self.detect_assembly(c) for func, nodes in values: - func_name = func.name - info = "Assembly in %s, Contract: %s, Function: %s" % (self.filename, - c.name, - func_name) - self.log(info) + info = "{}.{} uses assembly ({})\n" + info = info.format(func.contract.name, func.name, func.source_mapping_str) + all_info += info sourceMapping = [n.source_mapping for n in nodes] results.append({'vuln': 'Assembly', 'sourceMapping': sourceMapping, 'filename': self.filename, - 'contract': c.name, - 'function_name': func_name}) + 'contract': func.contract.name, + 'function_name': func.name}) + if all_info != '': + self.log(all_info) return results diff --git a/slither/detectors/statements/tx_origin.py b/slither/detectors/statements/tx_origin.py index f5c474f8f..fa5ba4938 100644 --- a/slither/detectors/statements/tx_origin.py +++ b/slither/detectors/statements/tx_origin.py @@ -14,6 +14,8 @@ class TxOrigin(AbstractDetector): IMPACT = DetectorClassification.MEDIUM CONFIDENCE = DetectorClassification.MEDIUM + WIKI = 'https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#dangerous-usage-of-txorigin' + @staticmethod def _contains_incorrect_tx_origin_use(node): """ @@ -48,10 +50,12 @@ class TxOrigin(AbstractDetector): for c in self.contracts: values = self.detect_tx_origin(c) for func, nodes in values: - func_name = func.name - info = "tx.origin in %s, Contract: %s, Function: %s" % (self.filename, - c.name, - func_name) + info = "{}.{} uses tx.origin for authorization:\n" + info = info.format(func.contract.name, func.name) + + for node in nodes: + info += "\t- {} ({})\n".format(node.expression, node.source_mapping_str) + self.log(info) sourceMapping = [n.source_mapping for n in nodes] @@ -59,7 +63,7 @@ class TxOrigin(AbstractDetector): results.append({'vuln': 'TxOrigin', 'sourceMapping': sourceMapping, 'filename': self.filename, - 'contract': c.name, - 'function_name': func_name}) + 'contract': func.contract.name, + 'function_name': func.name}) return results diff --git a/slither/detectors/variables/possible_const_state_variables.py b/slither/detectors/variables/possible_const_state_variables.py index e19a389d1..61d34efff 100644 --- a/slither/detectors/variables/possible_const_state_variables.py +++ b/slither/detectors/variables/possible_const_state_variables.py @@ -23,6 +23,8 @@ class ConstCandidateStateVars(AbstractDetector): IMPACT = DetectorClassification.INFORMATIONAL CONFIDENCE = DetectorClassification.HIGH + WIKI = 'https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#state-variables-that-could-be-declared-constant' + @staticmethod def lvalues_of_operations_with_lvalue(contract): ret = [] @@ -54,6 +56,7 @@ class ConstCandidateStateVars(AbstractDetector): """ Detect state variables that could be const """ results = [] + all_info = '' for c in self.slither.contracts_derived: const_candidates = self.detect_const_candidates(c) if const_candidates: @@ -64,10 +67,8 @@ class ConstCandidateStateVars(AbstractDetector): for contract, variables in variables_by_contract.items(): variable_names = [v.name for v in variables] - info = "State variables that could be const in %s, Contract: %s, Vars %s" % (self.filename, - contract, - str(variable_names)) - self.log(info) + for v in variables: + all_info += "{}.{} should be constant ({})\n".format(contract, v.name, v.source_mapping_str) sourceMapping = [v.source_mapping for v in const_candidates] @@ -76,4 +77,6 @@ class ConstCandidateStateVars(AbstractDetector): 'filename': self.filename, 'contract': c.name, 'unusedVars': variable_names}) + if all_info != '': + self.log(all_info) return results diff --git a/slither/detectors/variables/uninitialized_state_variables.py b/slither/detectors/variables/uninitialized_state_variables.py index 86f54b048..3046268a8 100644 --- a/slither/detectors/variables/uninitialized_state_variables.py +++ b/slither/detectors/variables/uninitialized_state_variables.py @@ -28,6 +28,8 @@ class UninitializedStateVarsDetection(AbstractDetector): IMPACT = DetectorClassification.HIGH CONFIDENCE = DetectorClassification.HIGH + WIKI = 'https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#uninitialized-state-variables' + @staticmethod def written_variables(contract): ret = [] @@ -72,10 +74,10 @@ class UninitializedStateVarsDetection(AbstractDetector): for c in self.slither.contracts_derived: ret = self.detect_uninitialized(c) for variable, functions in ret: - info = "Uninitialized state variable in %s, " % self.filename + \ - "Contract: %s, Variable: %s, Used in %s" % (c.name, - str(variable), - [str(f) for f in functions]) + info = "{}.{} ({}) is never initialized. It is used in:\n" + info = info.format(variable.contract.name, variable.name, variable.source_mapping_str) + for f in functions: + info += "\t- {} ({})\n".format(f.name, f.source_mapping_str) self.log(info) source = [variable.source_mapping] diff --git a/slither/detectors/variables/uninitialized_storage_variables.py b/slither/detectors/variables/uninitialized_storage_variables.py index 37201b33c..ccc3fbc77 100644 --- a/slither/detectors/variables/uninitialized_storage_variables.py +++ b/slither/detectors/variables/uninitialized_storage_variables.py @@ -19,6 +19,7 @@ class UninitializedStorageVars(AbstractDetector): IMPACT = DetectorClassification.HIGH CONFIDENCE = DetectorClassification.HIGH + WIKI = 'https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#uninitialized-storage-variables' # node.context[self.key] contains the uninitialized storage variables key = "UNINITIALIZEDSTORAGE" @@ -82,10 +83,9 @@ class UninitializedStorageVars(AbstractDetector): for(function, uninitialized_storage_variable) in self.results: var_name = uninitialized_storage_variable.name - info = "Uninitialized storage variables in %s, " % self.filename + \ - "Contract: %s, Function: %s, Variable %s" % (function.contract.name, - function.name, - var_name) + info = "{} in {}.{} ({}) is a storage variable never initialiazed\n" + info = info.format(var_name, function.contract.name, function.name, uninitialized_storage_variable.source_mapping_str) + self.log(info) source = [function.source_mapping, uninitialized_storage_variable.source_mapping] diff --git a/slither/detectors/variables/unused_state_variables.py b/slither/detectors/variables/unused_state_variables.py index 5be36163e..36a42d267 100644 --- a/slither/detectors/variables/unused_state_variables.py +++ b/slither/detectors/variables/unused_state_variables.py @@ -14,6 +14,8 @@ class UnusedStateVars(AbstractDetector): IMPACT = DetectorClassification.INFORMATIONAL CONFIDENCE = DetectorClassification.HIGH + WIKI = 'https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#unused-state-variables' + def detect_unused(self, contract): if contract.is_signature_only(): return None @@ -30,14 +32,16 @@ class UnusedStateVars(AbstractDetector): """ Detect unused state variables """ results = [] + all_info = '' for c in self.slither.contracts_derived: unusedVars = self.detect_unused(c) if unusedVars: unusedVarsName = [v.name for v in unusedVars] - info = "Unused state variables in %s, Contract: %s, Vars %s" % (self.filename, - c.name, - str(unusedVarsName)) - self.log(info) + info = '' + for var in unusedVars: + info += "{}.{} ({}) is never used\n".format(var.contract.name, var.name, var.source_mapping_str) + + all_info += info sourceMapping = [v.source_mapping for v in unusedVars] @@ -46,4 +50,6 @@ class UnusedStateVars(AbstractDetector): 'filename': self.filename, 'contract': c.name, 'unusedVars': unusedVarsName}) + if all_info != '': + self.log(all_info) return results diff --git a/slither/slither.py b/slither/slither.py index eb9cc64d4..463eebd80 100644 --- a/slither/slither.py +++ b/slither/slither.py @@ -22,9 +22,10 @@ class Slither(SlitherSolc): self._printers = [] # json text provided - if isinstance(contract, dict): + if isinstance(contract, list): super(Slither, self).__init__('') - self._parse_contracts_from_loaded_json(contract, '') + for c in contract: + self._parse_contracts_from_loaded_json(c, c['absolutePath']) # .json or .sol provided else: contracts_json = self._run_solc(contract, solc, disable_solc_warnings, solc_arguments, ast_format) diff --git a/slither/solc_parsing/declarations/contract.py b/slither/solc_parsing/declarations/contract.py index 087750577..03ff2dc54 100644 --- a/slither/solc_parsing/declarations/contract.py +++ b/slither/solc_parsing/declarations/contract.py @@ -218,6 +218,7 @@ class ContractSolc04(Contract): event = EventSolc(event_to_parse, self) event.analyze(self) event.set_contract(self) + event.set_offset(event_to_parse['src'], self.slither) self._events[event.full_name] = event self._eventsNotParsed = None @@ -234,9 +235,14 @@ class ContractSolc04(Contract): self._variables[var.name] = var def analyze_constant_state_variables(self): + from slither.solc_parsing.expressions.expression_parsing import VariableNotFound for var in self.variables: if var.is_constant: - var.analyze(self) + # cant parse constant expression based on function calls + try: + var.analyze(self) + except VariableNotFound: + pass return def analyze_state_variables(self): diff --git a/slither/solc_parsing/declarations/function.py b/slither/solc_parsing/declarations/function.py index 4a2e5d926..5b3952917 100644 --- a/slither/solc_parsing/declarations/function.py +++ b/slither/solc_parsing/declarations/function.py @@ -92,8 +92,9 @@ class FunctionSolc(Function): if 'payable' in attributes: self._payable = attributes['payable'] - def _new_node(self, node_type): + def _new_node(self, node_type, src): node = NodeSolc(node_type, self._counter_nodes) + node.set_offset(src, self.slither) self._counter_nodes += 1 node.set_function(self) self._nodes.append(node) @@ -107,7 +108,7 @@ class FunctionSolc(Function): condition = ifStatement['condition'] # Note: check if the expression could be directly # parsed here - condition_node = self._new_node(NodeType.IF) + condition_node = self._new_node(NodeType.IF, ifStatement['src']) condition_node.add_unparsed_expression(condition) link_nodes(node, condition_node) trueStatement = self._parse_statement(ifStatement['trueBody'], condition_node) @@ -118,14 +119,14 @@ class FunctionSolc(Function): condition = children[0] # Note: check if the expression could be directly # parsed here - condition_node = self._new_node(NodeType.IF) + condition_node = self._new_node(NodeType.IF, ifStatement['src']) condition_node.add_unparsed_expression(condition) link_nodes(node, condition_node) trueStatement = self._parse_statement(children[1], condition_node) if len(children) == 3: falseStatement = self._parse_statement(children[2], condition_node) - endIf_node = self._new_node(NodeType.ENDIF) + endIf_node = self._new_node(NodeType.ENDIF, ifStatement['src']) link_nodes(trueStatement, endIf_node) if falseStatement: @@ -164,8 +165,8 @@ class FunctionSolc(Function): def _parse_while(self, whileStatement, node): # WhileStatement = 'while' '(' Expression ')' Statement - node_startWhile = self._new_node(NodeType.STARTLOOP) - node_condition = self._new_node(NodeType.IFLOOP) + node_startWhile = self._new_node(NodeType.STARTLOOP, whileStatement['src']) + node_condition = self._new_node(NodeType.IFLOOP, whileStatement['src']) if self.is_compact_ast: node_condition.add_unparsed_expression(whileStatement['condition']) @@ -176,7 +177,7 @@ class FunctionSolc(Function): node_condition.add_unparsed_expression(expression) statement = self._parse_statement(children[1], node_condition) - node_endWhile = self._new_node(NodeType.ENDLOOP) + node_endWhile = self._new_node(NodeType.ENDLOOP, whileStatement['src']) link_nodes(node, node_startWhile) link_nodes(node_startWhile, node_condition) @@ -191,8 +192,8 @@ class FunctionSolc(Function): condition = statement['condition'] loop_expression = statement['loopExpression'] - node_startLoop = self._new_node(NodeType.STARTLOOP) - node_endLoop = self._new_node(NodeType.ENDLOOP) + node_startLoop = self._new_node(NodeType.STARTLOOP, statement['src']) + node_endLoop = self._new_node(NodeType.ENDLOOP, statement['src']) if init_expression: node_init_expression = self._parse_statement(init_expression, node) @@ -201,7 +202,7 @@ class FunctionSolc(Function): link_nodes(node, node_startLoop) if condition: - node_condition = self._new_node(NodeType.IFLOOP) + node_condition = self._new_node(NodeType.IFLOOP, statement['src']) node_condition.add_unparsed_expression(condition) link_nodes(node_startLoop, node_condition) link_nodes(node_condition, node_endLoop) @@ -254,8 +255,8 @@ class FunctionSolc(Function): hasLoopExpression = False - node_startLoop = self._new_node(NodeType.STARTLOOP) - node_endLoop = self._new_node(NodeType.ENDLOOP) + node_startLoop = self._new_node(NodeType.STARTLOOP, statement['src']) + node_endLoop = self._new_node(NodeType.ENDLOOP, statement['src']) children = statement[self.get_children('children')] @@ -283,7 +284,7 @@ class FunctionSolc(Function): if candidate[self.get_key()] not in ['VariableDefinitionStatement', 'VariableDeclarationStatement', 'ExpressionStatement']: - node_condition = self._new_node(NodeType.IFLOOP) + node_condition = self._new_node(NodeType.IFLOOP, statement['src']) #expression = parse_expression(candidate, self) expression = candidate node_condition.add_unparsed_expression(expression) @@ -313,8 +314,8 @@ class FunctionSolc(Function): def _parse_dowhile(self, doWhilestatement, node): - node_startDoWhile = self._new_node(NodeType.STARTLOOP) - node_condition = self._new_node(NodeType.IFLOOP) + node_startDoWhile = self._new_node(NodeType.STARTLOOP, doWhilestatement['src']) + node_condition = self._new_node(NodeType.IFLOOP, doWhilestatement['src']) if self.is_compact_ast: node_condition.add_unparsed_expression(doWhilestatement['condition']) @@ -326,7 +327,7 @@ class FunctionSolc(Function): node_condition.add_unparsed_expression(expression) statement = self._parse_statement(children[1], node_condition) - node_endDoWhile = self._new_node(NodeType.ENDLOOP) + node_endDoWhile = self._new_node(NodeType.ENDLOOP, doWhilestatement['src']) link_nodes(node, node_startDoWhile) link_nodes(node_startDoWhile, statement) @@ -344,7 +345,7 @@ class FunctionSolc(Function): self._variables[local_var.name] = local_var #local_var.analyze(self) - new_node = self._new_node(NodeType.VARIABLE) + new_node = self._new_node(NodeType.VARIABLE, statement['src']) new_node.add_variable_declaration(local_var) link_nodes(node, new_node) return new_node @@ -418,7 +419,7 @@ class FunctionSolc(Function): 'typeDescriptions': {'typeString':'tuple()'} } node = new_node - new_node = self._new_node(NodeType.EXPRESSION) + new_node = self._new_node(NodeType.EXPRESSION, statement['src']) new_node.add_unparsed_expression(expression) link_nodes(node, new_node) @@ -490,7 +491,7 @@ class FunctionSolc(Function): self.get_children('children'): var_identifiers}, tuple_vars]} node = new_node - new_node = self._new_node(NodeType.EXPRESSION) + new_node = self._new_node(NodeType.EXPRESSION, statement['src']) new_node.add_unparsed_expression(expression) link_nodes(node, new_node) @@ -506,7 +507,7 @@ class FunctionSolc(Function): self._variables[local_var.name] = local_var # local_var.analyze(self) - new_node = self._new_node(NodeType.VARIABLE) + new_node = self._new_node(NodeType.VARIABLE, statement['src']) new_node.add_variable_declaration(local_var) link_nodes(node, new_node) return new_node @@ -534,7 +535,7 @@ class FunctionSolc(Function): elif name == 'Block': node = self._parse_block(statement, node) elif name == 'InlineAssembly': - break_node = self._new_node(NodeType.ASSEMBLY) + break_node = self._new_node(NodeType.ASSEMBLY, statement['src']) link_nodes(node, break_node) node = break_node elif name == 'DoWhileStatement': @@ -542,15 +543,15 @@ class FunctionSolc(Function): # For Continue / Break / Return / Throw # The is fixed later elif name == 'Continue': - continue_node = self._new_node(NodeType.CONTINUE) + continue_node = self._new_node(NodeType.CONTINUE, statement['src']) link_nodes(node, continue_node) node = continue_node elif name == 'Break': - break_node = self._new_node(NodeType.BREAK) + break_node = self._new_node(NodeType.BREAK, statement['src']) link_nodes(node, break_node) node = break_node elif name == 'Return': - return_node = self._new_node(NodeType.RETURN) + return_node = self._new_node(NodeType.RETURN, statement['src']) link_nodes(node, return_node) if self.is_compact_ast: if statement['expression']: @@ -562,7 +563,7 @@ class FunctionSolc(Function): return_node.add_unparsed_expression(expression) node = return_node elif name == 'Throw': - throw_node = self._new_node(NodeType.THROW) + throw_node = self._new_node(NodeType.THROW, statement['src']) link_nodes(node, throw_node) node = throw_node elif name == 'EmitStatement': @@ -571,7 +572,7 @@ class FunctionSolc(Function): expression = statement['eventCall'] else: expression = statement[self.get_children('children')][0] - new_node = self._new_node(NodeType.EXPRESSION) + new_node = self._new_node(NodeType.EXPRESSION, statement['src']) new_node.add_unparsed_expression(expression) link_nodes(node, new_node) node = new_node @@ -585,7 +586,7 @@ class FunctionSolc(Function): expression = statement[self.get_children('expression')] else: expression = statement[self.get_children('expression')][0] - new_node = self._new_node(NodeType.EXPRESSION) + new_node = self._new_node(NodeType.EXPRESSION, statement['src']) new_node.add_unparsed_expression(expression) link_nodes(node, new_node) node = new_node @@ -615,7 +616,7 @@ class FunctionSolc(Function): assert cfg[self.get_key()] == 'Block' - node = self._new_node(NodeType.ENTRYPOINT) + node = self._new_node(NodeType.ENTRYPOINT, cfg['src']) self._entry_point = node if self.is_compact_ast: @@ -866,23 +867,23 @@ class FunctionSolc(Function): def split_ternary_node(self, node, condition, true_expr, false_expr): - condition_node = self._new_node(NodeType.IF) + condition_node = self._new_node(NodeType.IF, node.source_mapping) condition_node.add_expression(condition) condition_node.analyze_expressions(self) - true_node = self._new_node(node.type) + true_node = self._new_node(node.type, node.source_mapping) if node.type == NodeType.VARIABLE: true_node.add_variable_declaration(node.variable_declaration) true_node.add_expression(true_expr) true_node.analyze_expressions(self) - false_node = self._new_node(node.type) + false_node = self._new_node(node.type, node.source_mapping) if node.type == NodeType.VARIABLE: false_node.add_variable_declaration(node.variable_declaration) false_node.add_expression(false_expr) false_node.analyze_expressions(self) - endif_node = self._new_node(NodeType.ENDIF) + endif_node = self._new_node(NodeType.ENDIF, node.source_mapping) for father in node.fathers: father.remove_son(node) diff --git a/slither/solc_parsing/declarations/modifier.py b/slither/solc_parsing/declarations/modifier.py index 527c439e8..1341779c3 100644 --- a/slither/solc_parsing/declarations/modifier.py +++ b/slither/solc_parsing/declarations/modifier.py @@ -65,7 +65,7 @@ class ModifierSolc(Modifier, FunctionSolc): def _parse_statement(self, statement, node): name = statement[self.get_key()] if name == 'PlaceholderStatement': - placeholder_node = self._new_node(NodeType.PLACEHOLDER) + placeholder_node = self._new_node(NodeType.PLACEHOLDER, statement['src']) link_nodes(node, placeholder_node) return placeholder_node return super(ModifierSolc, self)._parse_statement(statement, node) diff --git a/slither/solc_parsing/slitherSolc.py b/slither/solc_parsing/slitherSolc.py index 28108f3fd..ec7383fe2 100644 --- a/slither/solc_parsing/slitherSolc.py +++ b/slither/solc_parsing/slitherSolc.py @@ -1,3 +1,4 @@ +import os import json import re import logging @@ -56,6 +57,13 @@ class SlitherSolc(Slither): if 'nodeType' in data_loaded: self._is_compact_ast = True + if 'sourcePaths' in data_loaded: + for sourcePath in data_loaded['sourcePaths']: + if os.path.isfile(sourcePath): + with open(sourcePath) as f: + source_code = f.read() + self.source_code[sourcePath] = source_code + if data_loaded[self.get_key()] == 'root': self._solc_version = '0.3' logger.error('solc <0.4 is not supported') @@ -105,7 +113,7 @@ class SlitherSolc(Slither): assert len(name) == 1 name = name[0] else: - name =filename + name = filename sourceUnit = -1 # handle old solc, or error if 'src' in data: @@ -114,6 +122,12 @@ class SlitherSolc(Slither): sourceUnit = int(sourceUnit[0]) self._source_units[sourceUnit] = name + if os.path.isfile(name) and not name in self.source_code: + with open(name) as f: + source_code = f.read() + self.source_code[name] = source_code + + def _analyze_contracts(self): if self._analyzed: diff --git a/slither/utils/command_line.py b/slither/utils/command_line.py index 7233b541e..57542901d 100644 --- a/slither/utils/command_line.py +++ b/slither/utils/command_line.py @@ -3,13 +3,19 @@ from prettytable import PrettyTable from slither.detectors.abstract_detector import classification_txt def output_to_markdown(detector_classes, printer_classes): + + def extract_help(detector): + if detector.WIKI == '': + return detector.HELP + return '[{}]({})'.format(detector.HELP, detector.WIKI) + detectors_list = [] for detector in detector_classes: argument = detector.ARGUMENT # dont show the backdoor example if argument == 'backdoor': continue - help_info = detector.HELP + help_info = extract_help(detector) impact = detector.IMPACT confidence = classification_txt[detector.CONFIDENCE] detectors_list.append((argument, help_info, impact, confidence))