Consensys / truffle-security

MythX smart contract security verification plugin for Truffle Framework
https://mythx.io
124 stars 28 forks source link

Display the rerrors of multiple analysis requests distinctly #79

Closed nbanmp closed 5 years ago

nbanmp commented 5 years ago

doAnalysis clobbers together the results of all the analysis requests, and errors can not be displayed with their associated analysis requests.

Print errors and results distinctly, with information about the analysis request next to the error or result itself.

daniyarchambylov commented 5 years ago

@nbanmp thanks for raising this issue. If you think this is much convenient way to show results to user then I'd be glad to help. But there are few issues you might run into.

Becuase one solidity file might contain multiple contracts truffle outputs compile results into separate json files. Then we use details of contract to MythX API. So what happens is that you have multiple responses pointing to same solidity file. truffle-security output will look like this:

/home/daniyar/tmp/truffle-security-test/contracts/03-auction.sol
   1:0   warning  A floating pragma is set                       SWC-103
   5:2   warning  State variable shadows another state variable  SWC-119
   5:10  warning  The state variable visibility is not set       SWC-108
   6:2   warning  State variable shadows another state variable  SWC-119
   6:7   warning  The state variable visibility is not set       SWC-108
   9:2   warning  The function visibility is not set             SWC-100
  19:4   error    persistent state read after call               SWC-107
  20:4   error    persistent state write after call              SWC-107
  27:10  warning  The state variable visibility is not set       SWC-108
  28:10  warning  The state variable visibility is not set       SWC-108
  30:27  warning  The state variable visibility is not set       SWC-108
  51:4   warning  Unchecked return value from low level call     SWC-104

/home/daniyar/tmp/truffle-security-test/contracts/03-auction.sol
   1:0   warning  A floating pragma is set                           SWC-103
   5:2   warning  State variable shadows another state variable      SWC-119
   5:10  warning  The state variable visibility is not set           SWC-108
   6:2   warning  State variable shadows another state variable      SWC-119
   6:7   warning  The state variable visibility is not set           SWC-108
   9:2   warning  The function visibility is not set                 SWC-100
  27:10  warning  The state variable visibility is not set           SWC-108
  28:10  warning  The state variable visibility is not set           SWC-108
  30:27  warning  The state variable visibility is not set           SWC-108
  51:4   warning  Unchecked return value from low level call         SWC-104
  51:4   warning  The return value of a message call is not checked  SWC-104

✖ 23 problems (2 errors, 21 warnings)

Because we have multiple analysis results pointing to same file and duplicate SWC issues we group analysis results and remove duplicate entries. I think it would be great to work on output format as well. Even come up with custom formatter style. By default eslint's stylish formatter is used. What do you think?

nbanmp commented 5 years ago

The main issue that I see is that it's not clear which analysis request errors belong to. It doesn't really matter which request the results belong to because the results should be able to speak for themselves. For now, I'll continue to merge multiple analyses for a single file, but when a timeout or another error occurs, I'd specify the file/analysis request when printing the errors.

Looking into this more, armlet does provide enough information to determine which analysis request the error belongs to when it is returned by the api, we're just not printing anything other than the error message. When armlet itself throws the error, however, it's not clear which analysis request the error belongs to.