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

chore: add CWE ids to findings CSV #15

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gsilvapt
Copy link

@gsilvapt gsilvapt commented Sep 9, 2024

Support added in both findings_csv and findings_to_defectdojo.

This information is useful to obtain in some cases, either to import into another security intelligence platform or to use as part of the vulnerability analysis.

For optimization purposes, a short "in-memory" cache is set to avoid spamming the API looking for vulnerability definition in both commands.

Also rewrote some of the logic to use Python's dictionary .get() with defaults instead of writing if statements, where applicable. Same result, simpler to read.


Another addition with the change in findings_csv to include a header row with the field names.

This was useful to use personal and seemed like a nice feature to include for everyone. Obviously, you're the ones that truly know whether this would be useful for you and your users.

I can revert part of the changes if you deemed best. Ran autopep8 in the modified files too, which added some more formatter changes but seemed the best fit for the project.

Please let me know your opinion when you have the time and thanks in advance for reviewing.

Support added in both `findings_csv` and `findings_to_defectdojo`.

This information is useful to obtain in some cases, either to import
into another security intelligence platform or to use as part of the
vulnerability analysis.

For optimization purposes, a short "in-memory" cache is set to avoid
spamming the API looking for vulnerability definition in both commands.

Also rewrote some of the logic to use Python's dictionary `.get()` with
defaults instead of writing if statements, where applicable. Same result,
simpler to read.
Copy link

dryrunsecurity bot commented Sep 9, 2024

DryRun Security Summary

The pull request enhances the functionality and security of two Python scripts, findings_csv.py and findings_to_defectdojo.py, by introducing caching mechanisms, improving error handling, and adding more detailed vulnerability information, such as CWE IDs, to the output.

Expand for full summary

Summary:

The code changes in this pull request appear to be focused on enhancing the functionality and security of two Python scripts: findings_csv.py and findings_to_defectdojo.py. The changes introduce several security-conscious improvements, including caching mechanisms to reduce API calls, improved error handling, and the addition of more detailed vulnerability information (such as CWE IDs) in the output.

In the findings_csv.py script, the changes introduce a caching mechanism for fetching definitions from the API, which helps to minimize the number of API requests and reduce the risk of rate-limiting or abuse detection. The script also now includes additional columns in the CSV output, providing more detailed information about the security findings.

The findings_to_defectdojo.py script has been updated to include the CWE ID for each finding in the DefectDojo output, which is an important security-related enhancement. The script also includes improved error handling for the API requests.

While the changes in this pull request are generally positive from an application security perspective, there are a few areas that could be further improved, such as the use of hardcoded API base URLs and JWT tokens, which could potentially introduce security risks if they are compromised or changed. Incorporating environment variables or a configuration file to store these sensitive values would be a security-conscious approach.

Files Changed:

  1. findings_csv.py:

    • Introduced a definition_cache dictionary to cache the definitions fetched from the definitions_endpoint.
    • Added a get_definition function to fetch the definition for a given definition_id, first checking the cache before making an API request.
    • Included additional columns in the CSV output, such as "cwe_id" and "definition_name", providing more detailed information about the security findings.
    • Improved handling of the "assignee" field in the API response to ensure the script can handle various API response formats.
  2. findings_to_defectdojo.py:

    • Added a new definitions_endpoint to the API base URL, which is used to retrieve the definition details for each finding, including the CWE ID.
    • Introduced a definitions_cache dictionary to cache the definition details for each finding.
    • Included the CWE ID for each finding in the DefectDojo output.
    • Improved error handling for the API requests, raising an exception if the response status code is not in the 2xx range.

Code Analysis

We ran 9 analyzers against 2 files and 1 analyzer had findings. 8 analyzers had no findings.

Analyzer Findings
Authn/Authz Analyzer 1 finding

Riskiness

🟢 Risk threshold not exceeded.

View PR in the DryRun Dashboard.

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 this pull request may close these issues.

1 participant