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

Ruff: Add and fix S110 (+ merge all S1 rules) #11256

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

Conversation

kiblik
Copy link
Contributor

@kiblik kiblik commented Nov 13, 2024

Copy link

dryrunsecurity bot commented Nov 13, 2024

DryRun Security Summary

The provided code changes cover a wide range of improvements and bug fixes across various components of the DefectDojo application, focusing on enhancing the application's security, reliability, and overall functionality.

Expand for full summary

Summary:

The provided code changes cover a wide range of improvements and bug fixes across various components of the DefectDojo application. The changes focus on enhancing the application's security, reliability, and overall functionality. Key areas addressed include:

  1. Credential Management: Improvements to the handling of credentials, including deletion validation, encryption, and association management.
  2. Finding Groups: Enhancements to the finding group functionality, including better exception handling and JIRA integration.
  3. Benchmark Management: Improvements to the benchmark management feature, with a focus on data validation and access control.
  4. Middleware Enhancements: Numerous improvements to the application's middleware, including better handling of unauthenticated requests, password reset enforcement, and logging capabilities.
  5. Parser Improvements: Enhancements to various parsers, such as the GitLab API Fuzzing, Kiuwan, and Veracode JSON parsers, to improve their robustness and error handling.
  6. Test Suite Enhancements: Improvements to the test suite, including better handling of scan type mapping and timeout management.

Overall, the changes demonstrate a strong focus on improving the security, reliability, and maintainability of the DefectDojo application. The application security engineer's review has highlighted several areas where the changes could be further improved, such as input validation, error handling, and dependency management. Continued attention to these aspects will help ensure the ongoing security and integrity of the application.

Files Changed:

  1. dojo/cred/views.py: Improvements to the management of credentials, including deletion validation and encryption.
  2. dojo/finding/helper.py: Enhancements to the finding group functionality, including better exception handling and JIRA integration.
  3. dojo/benchmark/views.py: Improvements to the benchmark management feature, with a focus on data validation and access control.
  4. dojo/middleware.py: Numerous improvements to the application's middleware, including better handling of unauthenticated requests, password reset enforcement, and logging capabilities.
  5. dojo/tools/h1/parser.py: Enhancements to the HackerOne vulnerability disclosure program parser.
  6. dojo/tools/gitlab_api_fuzzing/parser.py: Improvements to the GitLab API Fuzzing report parser.
  7. dojo/templatetags/display_tags.py: Minor optimization to the inline_image function.
  8. dojo/tools/kiuwan/parser.py: Enhancements to the Kiuwan parser to improve its robustness.
  9. dojo/product/views.py: Improvements to the GitHub integration functionality.
  10. dojo/tools/veracode/json_parser.py: Enhancements to the Veracode JSON parser to improve the handling of finding dates.
  11. ruff.toml: Changes to the Ruff linter configuration, which may have security implications.
  12. tests/base_test_class.py: Improvements to the test utility functions.
  13. tests/Import_scanner_test.py: Enhancements to the scan type mapping functionality in the test suite.

Code Analysis

We ran 9 analyzers against 13 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.

@kiblik kiblik force-pushed the ruff_S1 branch 5 times, most recently from b28dd82 to 586b87c Compare November 13, 2024 18:15
@kiblik kiblik marked this pull request as ready for review November 13, 2024 18:54
Copy link
Contributor

@Maffooch Maffooch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of passing, could we get by on logging the exception? That seems safer than removing the exception handling altogether

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants