Consensys / mythx-cli

A command line interface for the MythX smart contract security analysis API
https://mythx-cli.readthedocs.io/
MIT License
84 stars 29 forks source link

"analyze --remap-import" containing "migration" gets ignored #101

Closed kelonye closed 4 years ago

kelonye commented 4 years ago

Description

analyze --remap-import ignores imports if one of them contains "migration". Please see these images:

Screenshot 2020-03-20 at 03 02 01 Screenshot 2020-03-20 at 03 05 09

What I Did

Works:

mythx -y --api-key=$MYTHX_API_KEY analyze \
  --remap-import "@openzeppelin/=/home/circleci/repo/node_modules/@openzeppelin/" \
  --remap-import "@kleros/=/home/circleci/repo/node_modules/@kleros/" \
  --remap-import "fixidity/=/home/circleci/repo/node_modules/fixidity/" \
  --remap-import "scd-mcd-migratio/=/home/circleci/repo/node_modules/scd-mcd-migration/" \
  contracts/*

Fails:

mythx -y --api-key=$MYTHX_API_KEY analyze \
  --remap-import "@openzeppelin/=/home/circleci/repo/node_modules/@openzeppelin/" \
  --remap-import "@kleros/=/home/circleci/repo/node_modules/@kleros/" \
  --remap-import "fixidity/=/home/circleci/repo/node_modules/fixidity/" \
  --remap-import "scd-mcd-migration/=/home/circleci/repo/node_modules/scd-mcd-migration/" \
  contracts/*
kelonye commented 4 years ago

I think i found the issue https://github.com/dmuhs/mythx-cli/blob/master/mythx_cli/analyze/util.py#L211 ... submitting a PR

dmuhs commented 4 years ago

Thanks for fixing the issue right away! :fire: Some small notes on the command: You can omit the --api-key parameter if the MYTHX_API_KEY environment variable is defined. The CLI picks up on it automatically. Also, you can simplify the contracts/* target to just contracts/. In any case, when given a directory that is not a Truffle project, the CLI will recursively walk the directory, enumerate all Solidity files in it, and submit them to MythX for analysis. These payloads can get fairly large and include util contracts - which you might not want because they can clutter your analysis results. Do fix that, you might want to explicitly define the contracts you want to analyze by pointing the CLI directly to the Solidity file and contract name target. A small example would be:

mythx analyze contracts/token.sol:MyToken

This will also prevent recursive walks through large directory structures and results in a slightly faster submission :slightly_smiling_face:

kelonye commented 4 years ago

Awesome! Thanks for merging, and updated at https://github.com/vbstreetz/pooltogether-contracts/commit/464f691d09244461b3451b0dbf77316138898ea7 ..