From 7c7a9a3e053e6fb124f7c9f579ecb238b840cf37 Mon Sep 17 00:00:00 2001 From: Dominik Muhs Date: Mon, 2 Mar 2020 16:23:00 +0100 Subject: [PATCH 1/5] Update code documentation and type hints for MythX plugin --- brownie/_cli/analyze.py | 100 ++++++++++++++++++++++++++++++++------ tests/cli/test_analyze.py | 6 +-- 2 files changed, 85 insertions(+), 21 deletions(-) diff --git a/brownie/_cli/analyze.py b/brownie/_cli/analyze.py index 93b69b3ea..5ecf9574f 100644 --- a/brownie/_cli/analyze.py +++ b/brownie/_cli/analyze.py @@ -46,6 +46,8 @@ class SubmissionPipeline: + """A helper class to submit MythX analysis requests and retrieve reports.""" + BYTECODE_ADDRESS_PATCH = re.compile(r"__\w{38}") DEPLOYED_ADDRESS_PATCH = re.compile(r"__\$\w{34}\$__") @@ -59,8 +61,17 @@ def __init__(self, build, client: Client = None): self.stdout_report: Dict[str, dict] = {} @staticmethod - def get_mythx_client(): - """Generate a MythX client instance.""" + def get_mythx_client() -> Client: + """Generate a MythX client instance. + + This method will look for an API key passed as a parameter, and if none + is found, look for a key in the environment variable :code:`MYTHX_API_KEY`. + If a key is detected, a PythX client instance is returned, otherwise a + :code:`ValidationError` is raised. + + :raises: ValidationError if no valid API key is provided + :return: A PythX client instance + """ if ARGV["api-key"]: auth_args = {"api_key": ARGV["api-key"]} @@ -75,8 +86,16 @@ def get_mythx_client(): **auth_args, middlewares=[ClientToolNameMiddleware(name=f"brownie-{__version__}")] ) - def prepare_requests(self): - """Transform an artifact into a MythX payload.""" + def prepare_requests(self) -> None: + """Transform an artifact into a MythX payload. + + This will enumerate all the contracts and libraries across the Brownie + artifacts. For each contract, the dependencies are recursively listed and + attached to the contract's MythX payload to paint the most precise picture + possible. + + :return: None + """ contracts = {n: d for n, d in self.build.items() if d["type"] == "contract"} libraries = {n: d for n, d in self.build.items() if d["type"] == "library"} @@ -103,7 +122,15 @@ def prepare_requests(self): @classmethod def construct_request_from_artifact(cls, artifact) -> AnalysisSubmissionRequest: - """Construct a raw submission request from an artifact JSON file.""" + """Construct a raw submission request from an artifact JSON file. + + This will transform a Brownie contract JSON artifact into a MythX payload + object. Additionally, bytecode-level placeholders will be patched to provide + valid bytecode to the MythX analysis API. + + :param artifact: The Brownie JSON artifact + :return: :code:`AnalysisSubmissionRequest` for the artifact + """ bytecode = artifact.get("bytecode", "") deployed_bytecode = artifact.get("deployedBytecode", "") @@ -132,8 +159,15 @@ def construct_request_from_artifact(cls, artifact) -> AnalysisSubmissionRequest: analysis_mode=ARGV["mode"] or ANALYSIS_MODES[0], ) - def send_requests(self): - """Send the prepared requests to MythX.""" + def send_requests(self) -> None: + """Send the prepared requests to MythX. + + This will send the requests in the class' requests dict to MythX for + analysis. The API's response is parsed and added to the internal + :code:`responses` dict. + + :return: None + """ for contract_name, request in self.requests.items(): response = self.client.analyze(payload=request) @@ -144,8 +178,19 @@ def send_requests(self): ) print(f"You can also check the results at {DASHBOARD_BASE_URL}{response.uuid}\n") - def wait_for_jobs(self): - """Poll the MythX API and returns once all requests have been processed.""" + def wait_for_jobs(self) -> None: + """Poll the MythX API and returns once all requests have been processed. + + This will wait for all analysis requests in the internal :code:`responses` + dict to finish. If a user passes the :code:`--interval` option, the plugin + will poll the MythX API in the user-specified interval (in seconds) to see + whether the analysis request is done processing. + + If the internal responses dict is empty, a :code:`ValidationError` is raised. + + :raise: :code:`ValidationError` + :return: None + """ if not self.responses: raise ValidationError("No requests given") @@ -153,10 +198,17 @@ def wait_for_jobs(self): while not self.client.analysis_ready(response.uuid): time.sleep(int(ARGV["interval"])) self.reports[contract_name] = self.client.report(response.uuid) - # TODO: log message - def generate_highlighting_report(self): - """Generate a Brownie highlighting report from a MythX issue report.""" + def generate_highlighting_report(self) -> None: + """Generate a Brownie highlighting report from a MythX issue report. + + This will convert a MythX issue report into a Brownie report that allows + issues to be highlighted in the native Brownie GUI. It iterates over the + internal :code:`reports` dictionary and fills the :code:`highlight_report` + instance variable. + + :return: None + """ source_to_name = {d["sourcePath"]: d["contractName"] for _, d in self.build.items()} for idx, (contract_name, issue_report) in enumerate(self.reports.items()): @@ -197,8 +249,16 @@ def generate_highlighting_report(self): ] ) - def generate_stdout_report(self): - """Generated a stdout report overview from a MythX issue report.""" + def generate_stdout_report(self) -> None: + """Generated a stdout report overview from a MythX issue report. + + This will convert a MythX issue report into a Brownie report that is + printed on stdout when analysis results have been received. It iterates + over the internal :code:`reports` dictionary and fills the + :code:`highlight_report` instance variable. + + :return: None + """ for contract_name, issue_report in self.reports.items(): for issue in issue_report: @@ -207,8 +267,14 @@ def generate_stdout_report(self): self.stdout_report[contract_name][severity] += 1 -def print_console_report(stdout_report): - """Highlight and print a given stdout report to the console.""" +def print_console_report(stdout_report) -> None: + """Highlight and print a given stdout report to the console. + + This adds color formatting to the given stdout report and prints + a summary of the vulnerabilities MythX has detected. + + :return: None + """ total_issues = sum(x for i in stdout_report.values() for x in i.values()) if not total_issues: @@ -231,6 +297,8 @@ def print_console_report(stdout_report): def main(): + """The main entry point of the MythX plugin for Brownie.""" + args = docopt(__doc__) _update_argv_from_docopt(args) diff --git a/tests/cli/test_analyze.py b/tests/cli/test_analyze.py index c9c54a2a9..bf60043d7 100644 --- a/tests/cli/test_analyze.py +++ b/tests/cli/test_analyze.py @@ -205,11 +205,7 @@ def test_prepare_requests(monkeypatch): "contractName": "SafeMath", "type": "library", }, - "Token": { - "sourcePath": "contracts/Token.sol", - "contractName": "Token", - "type": "contract", - }, + "Token": {"sourcePath": "contracts/Token.sol", "contractName": "Token", "type": "contract"}, }.items build_mock.get_dependents.return_value = ["Token"] submission = SubmissionPipeline(build_mock) From eeac6396783ef793e7ec67fcc37eb7ed776d4e95 Mon Sep 17 00:00:00 2001 From: Dominik Muhs Date: Mon, 2 Mar 2020 16:30:03 +0100 Subject: [PATCH 2/5] Refresh security analysis documentation page --- docs/tests-security-analysis.rst | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/docs/tests-security-analysis.rst b/docs/tests-security-analysis.rst index 0f4cabedf..9e1d8899a 100644 --- a/docs/tests-security-analysis.rst +++ b/docs/tests-security-analysis.rst @@ -18,10 +18,11 @@ Security Analysis with MythX Brownie is integrated with the `MythX `_ analysis API to allow automated security scans of your project. -MythX is a smart contract security service that scans your project for vulnerabilities using static analysis, dynamic analysis, and symbolic execution. It runs in two modes: +MythX is a smart contract security service that scans your project for vulnerabilities using static analysis, dynamic analysis, and symbolic execution. It runs in three modes: - 1. **Quick mode** which is effective at finding bad coding patterns and low complexity-bugs - 2. **Full mode** which takes longer to run, but can locate complex security issues + 1. **Quick mode** which is effective at finding bad coding patterns and low complexity-bugs (available to free users) + 2. **Standard mode** which takes longer to run, but can locate complex security issues (available to Dev users) + 3. **Deep mode** which takes even longer to run, but is able to find deep, hidden vulnerabilities (available to Pro users) MythX offers both free and paid services. To learn more about how it works you may wish to read `MythX Pro Security Analysis Explained `_ by Bernhard Mueller. @@ -53,13 +54,13 @@ To quickly scan your project for vulnerabilities: This will send the compiled build artifacts to MythX for analysis. You will receive updates on the status of the scan; the entire process should take around three minutes. -To perform a full scan: +To perform a standard scan: :: - $ brownie analyze --full + $ brownie analyze --mode=standard -Note that a full scan requires authentication and takes approximately half an hour to complete. +Note that a deep scan requires authentication and takes approximately half an hour to complete. If you include the ``--async`` flag Brownie will submit the job, output the pending ID and exit. You can view the finished report later through the MythX dashboard. @@ -75,6 +76,12 @@ To view your report in the GUI, first open the GUI: brownie gui +Alternatively, the :code:`--gui` flag can be passed to the :code:`analyze` subcommand to open the Brownie GUI right away after the analysis results have been received. + +:: + + brownie analyze --gui + Click on the drop-down list in the upper right that says "Select Report" and choose "security". Then choose ``MythX`` in the new dropdown that appears. If any vulnerabilities have been found, they will be highlighted based on their severity: From 6fc28124d17b5f245b4cd9c4dd2dc51465a45c9f Mon Sep 17 00:00:00 2001 From: Dominik Muhs Date: Mon, 2 Mar 2020 17:54:03 +0100 Subject: [PATCH 3/5] Open a new analysis group for each request submission run --- brownie/_cli/analyze.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/brownie/_cli/analyze.py b/brownie/_cli/analyze.py index 5ecf9574f..18c054885 100644 --- a/brownie/_cli/analyze.py +++ b/brownie/_cli/analyze.py @@ -11,7 +11,7 @@ from mythx_models.request import AnalysisSubmissionRequest from mythx_models.response import AnalysisSubmissionResponse, DetectedIssuesResponse from pythx import Client, ValidationError -from pythx.middleware.toolname import ClientToolNameMiddleware +from pythx.middleware import ClientToolNameMiddleware, GroupDataMiddleware from brownie import project from brownie._cli.__main__ import __version__ @@ -169,6 +169,10 @@ def send_requests(self) -> None: :return: None """ + group_resp = self.client.create_group() + self.client.handler.middlewares.append( + GroupDataMiddleware(group_id=group_resp.group.identifier) + ) for contract_name, request in self.requests.items(): response = self.client.analyze(payload=request) self.responses[contract_name] = response @@ -177,6 +181,8 @@ def send_requests(self) -> None: f"contract {color('bright magenta')}{request.contract_name}{color})" ) print(f"You can also check the results at {DASHBOARD_BASE_URL}{response.uuid}\n") + self.client.seal_group(group_id=group_resp.group.identifier) + self.client.handler.middlewares.pop(-1) def wait_for_jobs(self) -> None: """Poll the MythX API and returns once all requests have been processed. From d1940a0e9736d0b07460c5cedcaed23c89180563 Mon Sep 17 00:00:00 2001 From: Dominik Muhs Date: Mon, 2 Mar 2020 18:21:36 +0100 Subject: [PATCH 4/5] Add analysis group response mocks to test --- tests/cli/test_analyze.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/cli/test_analyze.py b/tests/cli/test_analyze.py index bf60043d7..1edba6c67 100644 --- a/tests/cli/test_analyze.py +++ b/tests/cli/test_analyze.py @@ -185,10 +185,14 @@ def test_send_requests(monkeypatch): ) submission.requests = {"test": MagicMock()} analyze_mock = MagicMock() + group_mock = MagicMock() + group_mock.group.identifier = "test-gid" response_mock = MagicMock() response_mock.uuid = "test-uuid" analyze_mock.return_value = response_mock submission.client.analyze = analyze_mock + submission.client.create_group = group_mock + submission.client.seal_group = group_mock submission.send_requests() From e245ca1854e32cd8c9678f7ebc7a4ec63dde0074 Mon Sep 17 00:00:00 2001 From: Dominik Muhs Date: Mon, 2 Mar 2020 18:22:28 +0100 Subject: [PATCH 5/5] Add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d18030cb..06fe4866d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - MythX plugin update (PR)[https://github.com/iamdefinitelyahuman/brownie/pull/365] +- MythX plugin documentation update (PR)[https://github.com/iamdefinitelyahuman/brownie/pull/366] ## [1.6.5](https://github.com/iamdefinitelyahuman/brownie/tree/v1.6.5) - 2020-02-19