django-DefectDojo icon indicating copy to clipboard operation
django-DefectDojo copied to clipboard

Checkmarx one SCA parser fix

Open adamtimmins opened this issue 1 year ago • 3 comments

Description

Describe the feature / bug fix implemented by this PR. Fixing the way the parser handles findings with CWE string value from Checkmarx One JSON report.

Test results

Added new data in many_findings.json for the unit test to show the varied CWE results in the reports.

Checklist

This checklist is for your information.

  • [x] Make sure to rebase your PR against the very latest dev.
  • [x] Features/Changes should be submitted against the dev.
  • [x] Bugfixes should be submitted against the bugfix branch.
  • [x] Give a meaningful name to your PR, as it may end up being used in the release notes.
  • [x] Your code is flake8 compliant.
  • [x] Your code is python 3.11 compliant.
  • [x] If this is a new feature and not a bug fix, you've included the proper documentation in the docs at https://github.com/DefectDojo/django-DefectDojo/tree/dev/docs as part of this PR.
  • [x] Model changes must include the necessary migrations in the dojo/db_migrations folder.
  • [x] Add applicable tests to the unit tests.
  • [x] Add the proper label to categorize your PR.

Extra information

The current implementation of the parser could not handle CWE findings in the string format. Parser would raise an exception when the CWE was not an int.

adamtimmins avatar Aug 16 '24 10:08 adamtimmins

DryRun Security Summary

The pull request focuses on improving the security and robustness of the Checkmarx One security scan parser by adding new unit tests, enhancing the parsing of CWE information, and modifying the parse_results function to accurately associate CWE data with identified vulnerabilities.

Expand for full summary

Summary:

The code changes in this pull request focus on improving the security and robustness of the Checkmarx One security scan parser. The changes include:

  1. Addition of new unit tests to cover different scenarios, such as handling scans with no findings, multiple findings, and specific vulnerabilities like SQL injection (CWE-89).
  2. Improvements to the _parse_cwe function to accurately handle different formats of CWE (Common Weakness Enumeration) values, ensuring consistent and reliable extraction of CWE information from the scan results.
  3. Modifications to the parse_results function to leverage the improved _parse_cwe function, enhancing the parser's ability to associate the correct CWE with the identified vulnerabilities.

These changes are beneficial from an application security perspective, as they improve the quality and reliability of the Checkmarx One parser, which is a crucial component in the application security process. By accurately parsing and handling CWE information, the security team can better understand the nature of the vulnerabilities and prioritize their remediation efforts more effectively.

Files Changed:

  1. unittests/tools/test_checkmarx_one_parser.py: This file contains new unit tests that cover various scenarios for the CheckmarxOneParser, including handling scans with no findings, multiple findings, and a specific SQL injection vulnerability (CWE-89). These tests help ensure the robustness and security of the parser.

  2. dojo/tools/checkmarx_one/parser.py: The changes in this file focus on improving the parsing and handling of CWE information in the Checkmarx One parser. The addition of the _parse_cwe function and the modifications to the parse_results function enhance the parser's ability to accurately extract and associate CWE data with the identified vulnerabilities.

  3. unittests/scans/checkmarx_one/checkmarx_one_sca_10770.json: This file contains changes that address security vulnerabilities related to SQL injection and potential privilege escalation issues. The SQL injection vulnerabilities in the resolveSigningKeyBytes and login methods should be addressed by properly sanitizing or parameterizing the user inputs before using them in SQL queries. Additionally, the Dockerfiles should specify a USER instruction to run the container with a non-root user, reducing the potential impact of any vulnerabilities in the application.

Code Analysis

We ran 9 analyzers against 3 files and 0 analyzers had findings. 9 analyzers had no findings.

Riskiness

:green_circle: Risk threshold not exceeded.

View PR in the DryRun Dashboard.

dryrunsecurity[bot] avatar Aug 16 '24 10:08 dryrunsecurity[bot]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

github-actions[bot] avatar Aug 19 '24 10:08 github-actions[bot]

Conflicts have been resolved. A maintainer will review the pull request shortly.

github-actions[bot] avatar Aug 21 '24 08:08 github-actions[bot]