securesauce / precaution-beta

Precaution provides a simple, automated code review for GitHub projects by running code linters with a security focus on pull requests.
Other
1 stars 0 forks source link

Precaution doesn't catch when there are problems parsing the AST #144

Closed MVrachev closed 5 years ago

MVrachev commented 5 years ago

Describe the bug When you run Precaution against files with syntax or other problems Precaution doesn't catch when there are problems parsing the AST.

The result from that is a "No issues found" report by Precaution on files with syntax or possibly other security issues. That way you can bypass the Precaution scan.

To Reproduce Steps to reproduce the behavior:

  1. Create a pull request with a file with syntax problem
  2. Go to checks
  3. The result from Precaution will be "No issues found".

Expected behavior Normally I would expect the "App error" conclusion from the Precaution scan and in detail information like "syntax error while parsing AST from file".

MVrachev commented 5 years ago

I noticed this is a problem for both Go and Python code. When scanning python code Bandit gives you that information about the problems when parsing the AST but in Gosec when you use the JSON output there is no information about this problem.

I start and generate results from gosec with this command: gosec -fmt=json --out=result.json testData/app on this file which clearly misses the curly bracket of the main function (line 11):

image

and the results.json content is: image

If you do the same command but without generating an output on the command line you will see the problem:

image

So the report we get from Gosec can be really missleading. For Bandit we will fix it easily.

ericwb commented 5 years ago

Duplicate of #142