Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integrate Bandit into CI #1110

Closed
terriko opened this issue Mar 23, 2021 · 2 comments · Fixed by #1523
Closed

Integrate Bandit into CI #1110

terriko opened this issue Mar 23, 2021 · 2 comments · Fixed by #1523
Milestone

Comments

@terriko
Copy link
Contributor

terriko commented Mar 23, 2021

Not 100% sure if this is a brilliant idea, but it would probably be helpful for us to integrate Bandit into our CI and potentially pre-commit rules to get early warning on potential security flaws.

Bandit has a number of rules that aren't "don't do this" but "make sure you do secure code review on this component" so I definitely don't want CI to fail unless we get rid of all messages from Bandit, so we'll have to be a bit clever about how we set it up. I want it to give us useful feedback but hopefully not block PRs unnecessarily and I'm not sure what the best tuning for that is (yet).

@terriko terriko added this to the future milestone Mar 23, 2021
@terriko
Copy link
Contributor Author

terriko commented Jul 22, 2021

Update on this:

  • I've added https://github.com/intel/cve-bin-tool/blob/main/bandit.conf that skips the subprocess and path related issues. We could also mark these with #nosec to indicate that they had been reviewed. I didn't do the ignore marks on these because we were refactoring so I figured it was best to review them after the refactor was done.
  • I did mark a number of SQL issues as nosec to indicate that they had been reviewed
  • We have a couple of issues that are multi-line, and nosec doens't work on those. They would need to be refactored so nosec works -- I think for most of them that would mean putting the query into a separate variable. I did put comments in so they would appear when bandit was run

Overall, we're at the point where we could definitely run bandit in CI as long as we didn't block pull requests on finding 0 issues. It would take a bit more work (in terms of marking things as #nosec) before we could require a bandit pass before code could be merged.

@terriko
Copy link
Contributor Author

terriko commented Jul 22, 2021

Current output:

(venv3.8) [terri@cedar cve-bin-tool]$ bandit -c bandit.conf -r cve_bin_tool/
[main]  INFO    profile include tests: None
[main]  INFO    profile exclude tests: B607,B603,B404
[main]  INFO    cli include tests: None
[main]  INFO    cli exclude tests: None
[main]  INFO    using config: bandit.conf
[main]  INFO    running on Python 3.8.10
133 [0.. 50.. 100.. ]
Run started:2021-07-22 21:44:19.548770

Test results:                                                                         
>> Issue: [B608:hardcoded_sql_expressions] Possible SQL injection vector through string-based query construction.                                                           
   Severity: Medium   Confidence: Low                                                 
   Location: cve_bin_tool/cve_scanner.py:162                                          
   More Info: https://bandit.readthedocs.io/en/latest/plugins/b608_hardcoded_sql_expressions.html                                                                           
161             if cve_list:
162                 query = f"""
163                 SELECT CVE_number, severity, description, score, cvss_version, cvss_vector
164                 FROM cve_severity
165                 WHERE CVE_number IN ({",".join(["?"] * len(cve_list))}) AND score >= ?
166                 ORDER BY CVE_number

--------------------------------------------------
>> Issue: [B608:hardcoded_sql_expressions] Possible SQL injection vector through string-based query construction.                                                           
   Severity: Medium   Confidence: Low                                                 
   Location: cve_bin_tool/helper_script.py:213                                        
   More Info: https://bandit.readthedocs.io/en/latest/plugins/b608_hardcoded_sql_expressions.html                                                                           
212             # finding out all distinct (vendor, product) pairs with the help of product_name
213             query = f"""
214                 SELECT distinct vendor, product FROM cve_range

--------------------------------------------------
>> Issue: [B608:hardcoded_sql_expressions] Possible SQL injection vector through string-based query construction.                                                           
   Severity: Medium   Confidence: Medium
   Location: cve_bin_tool/version_signature.py:75
   More Info: https://bandit.readthedocs.io/en/latest/plugins/b608_hardcoded_sql_expressions.html
74              datestamp = self.cursor.execute(
75                  f"SELECT * FROM {self.update_table_name}"
76              ).fetchone()  #  update_table_name validated in __init__

--------------------------------------------------
>> Issue: [B608:hardcoded_sql_expressions] Possible SQL injection vector through string-based query construction.
   Severity: Medium   Confidence: Medium
   Location: cve_bin_tool/version_signature.py:102
   More Info: https://bandit.readthedocs.io/en/latest/plugins/b608_hardcoded_sql_expressions.html
101             data = self.cursor.execute(
102                 f"SELECT * FROM {self.table_name}"
103             ).fetchall()  # table_name validated in __init__

--------------------------------------------------

Code scanned:
        Total lines of code: 6460
        Total lines skipped (#nosec): 4

Run metrics:
        Total issues (by severity):
                Undefined: 0.0
                Low: 0.0
                Medium: 4.0
                High: 0.0
        Total issues (by confidence):
                Undefined: 0.0
                Low: 2.0
                Medium: 2.0
                High: 0.0
Files skipped (0):

terriko added a commit that referenced this issue Jan 26, 2022
* fixes #1110

Co-authored-by: Bread Genie <63963181+BreadGenie@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant