-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Ruff: Preparation for G004 #13076
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: Preparation for G004 #13076
Conversation
🔴 Risk threshold exceeded.This pull request contains multiple sensitive file edits across various components of the application and introduces several information disclosure risks through debug and error logging, potentially exposing sensitive details like system paths, exception traces, vulnerability scores, and user-controlled input in log files.
🔴 Configured Codepaths Edit in
|
Vulnerability | Configured Codepaths Edit |
---|---|
Description | Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in .dryrunsecurity.yaml . |
🔴 Configured Codepaths Edit in dojo/api_v2/views.py
Vulnerability | Configured Codepaths Edit |
---|---|
Description | Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in .dryrunsecurity.yaml . |
🔴 Configured Codepaths Edit in dojo/finding/views.py
Vulnerability | Configured Codepaths Edit |
---|---|
Description | Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in .dryrunsecurity.yaml . |
🔴 Configured Codepaths Edit in dojo/importers/endpoint_manager.py
Vulnerability | Configured Codepaths Edit |
---|---|
Description | Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in .dryrunsecurity.yaml . |
🔴 Configured Codepaths Edit in dojo/jira_link/helper.py
Vulnerability | Configured Codepaths Edit |
---|---|
Description | Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in .dryrunsecurity.yaml . |
🔴 Configured Codepaths Edit in dojo/models.py
Vulnerability | Configured Codepaths Edit |
---|---|
Description | Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in .dryrunsecurity.yaml . |
🔴 Configured Codepaths Edit in dojo/pipeline.py
Vulnerability | Configured Codepaths Edit |
---|---|
Description | Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in .dryrunsecurity.yaml . |
🔴 Configured Codepaths Edit in dojo/product/helpers.py
Vulnerability | Configured Codepaths Edit |
---|---|
Description | Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in .dryrunsecurity.yaml . |
🔴 Configured Codepaths Edit in dojo/search/views.py
Vulnerability | Configured Codepaths Edit |
---|---|
Description | Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in .dryrunsecurity.yaml . |
🔴 Configured Codepaths Edit in dojo/user/views.py
Vulnerability | Configured Codepaths Edit |
---|---|
Description | Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in .dryrunsecurity.yaml . |
🔴 Configured Codepaths Edit in dojo/utils.py
Vulnerability | Configured Codepaths Edit |
---|---|
Description | Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in .dryrunsecurity.yaml . |
Information Disclosure via Debug Logging in dojo/search/views.py
Vulnerability | Information Disclosure via Debug Logging |
---|---|
Description | The code change re-introduces debug logging for user-controlled search queries (clean_query ), operators, and keywords. If debug logging is enabled in a production environment, sensitive information entered by users into the search bar (e.g., internal system names, PII, specific vulnerability details) could be written to logs, leading to information disclosure. |
django-DefectDojo/dojo/search/views.py
Lines 422 to 430 in 85e6263
else: | |
keywords.append(vulnerability_id_fix(query_part)) | |
logger.debug("query: %s", clean_query) | |
logger.debug("operators: %s", operators) | |
logger.debug("keywords: %s", keywords) | |
return operators, keywords | |
Information Disclosure via Logging in dojo/pipeline.py
Vulnerability | Information Disclosure via Logging |
---|---|
Description | The change in logging from logger.error("...": %s", e.full_message) to logger.error("...": %s", e) within a broad except Exception as e: block introduces a risk of information disclosure. While the exact content of e.full_message and str(e) cannot be determined without the full code and the specific exception class from the Microsoft Graph API client library, logging the entire exception object (e ) typically provides a more verbose output than a curated attribute like full_message . This increased verbosity could expose sensitive internal details, stack traces, or other diagnostic information that was not intended for logging, potentially aiding an attacker in understanding the system's internal workings or identifying other vulnerabilities. |
django-DefectDojo/dojo/pipeline.py
Lines 99 to 105 in 85e6263
logger.debug("Skipping group " + group_name + " due to AZUREAD_TENANT_OAUTH2_GROUPS_FILTER " + settings.AZUREAD_TENANT_OAUTH2_GROUPS_FILTER) | |
continue | |
except Exception as e: | |
logger.error("Could not call microsoft graph API or save groups to member: %s", e) | |
if len(group_names) > 0: | |
assign_user_to_groups(user, group_names, Dojo_Group.AZURE) | |
if settings.AZUREAD_TENANT_OAUTH2_CLEANUP_GROUPS: |
Information Disclosure via Debug Logging in dojo/tools/anchore_grype/parser.py
Vulnerability | Information Disclosure via Debug Logging |
---|---|
Description | The code change introduces a debug log statement logger.debug("data: %s", data) within the Grype parser. Grype JSON reports contain highly sensitive information, including a comprehensive list of all detected vulnerabilities, their severities, affected components, CVSS scores, and remediation details. Logging this entire structure, even at the debug level, poses a significant risk of information disclosure. |
django-DefectDojo/dojo/tools/anchore_grype/parser.py
Lines 30 to 38 in 85e6263
) | |
def get_findings(self, file, test): | |
logger.debug("file: %s", file) | |
data = json.load(file) | |
logger.debug("data: %s", data) | |
dupes = {} | |
for item in data.get("matches", []): | |
vulnerability = item["vulnerability"] |
Information Disclosure via Debug Logging in dojo/tools/anchore_grype/parser.py
Vulnerability | Information Disclosure via Debug Logging |
---|---|
Description | The patch introduces debug logging of epss_list and epss_data within the get_epss_values function. The epss_list contains EPSS (Exploit Prediction Scoring System) scores and percentiles for vulnerabilities, along with their associated CVE IDs. While debug logs are typically not enabled in production, if they were, this information could be exposed. EPSS scores indicate the likelihood of a vulnerability being exploited, which could provide valuable intelligence to an attacker, allowing them to prioritize exploitation efforts. |
django-DefectDojo/dojo/tools/anchore_grype/parser.py
Lines 223 to 244 in 85e6263
def get_epss_values(self, vuln_id, epss_list): | |
if not isinstance(epss_list, list): | |
logger.debug("epss_list is not a list: %s", epss_list) | |
return None, None | |
if isinstance(epss_list, list): | |
logger.debug("epss_list: %s", epss_list) | |
for epss_data in epss_list: | |
if epss_data.get("cve") != vuln_id: | |
continue | |
try: | |
epss_score = float(epss_data.get("epss")) | |
epss_percentile = float(epss_data.get("percentile")) | |
except (TypeError, ValueError): | |
logger.debug("epss_data is not a float: %s", epss_data) | |
else: | |
return epss_score, epss_percentile | |
logger.debug("epss not found for vuln_id: %s in epss_list: %s", vuln_id, epss_list) | |
return None, None | |
def get_vulnerability_ids(self, vuln_id, related_vulnerabilities): |
Information Disclosure via Logging in dojo/models.py
Vulnerability | Information Disclosure via Logging |
---|---|
Description | The clean method of the Endpoint model now logs the original query string (old_value ) at the ERROR level if it contains a NULL character. The query part of a URL can often contain sensitive information such as session IDs, authentication tokens, or personally identifiable information (PII) when imported from scanner reports or API calls. Logging this potentially sensitive data to error logs creates an information disclosure risk, as these logs are typically accessible to system administrators and developers. |
django-DefectDojo/dojo/models.py
Lines 1879 to 1885 in 85e6263
action_string = "Postgres does not accept NULL character. Attempting to replace with %00..." | |
for remove_str in null_char_list: | |
self.query = self.query.replace(remove_str, "%00") | |
logger.error('Query "%s" has invalid format - It contains the NULL character. The following action was taken: %s', old_value, action_string) | |
if self.query == "": | |
self.query = None | |
Information Disclosure via Logging in dojo/models.py
Vulnerability | Information Disclosure via Logging |
---|---|
Description | The application logs the raw value of the Endpoint.fragment field at the ERROR level if it contains a NULL character. The fragment field, being part of a URL, can be populated by user-controlled input, either directly through manual entry or indirectly via imported scan results from security tools. URL fragments can sometimes contain sensitive data, such as access tokens or session identifiers. Logging this potentially sensitive data to error logs creates an information disclosure risk, as error logs are often retained for extended periods and may be accessible to a wider audience. |
django-DefectDojo/dojo/models.py
Lines 1892 to 1898 in 85e6263
action_string = "Postgres does not accept NULL character. Attempting to replace with %00..." | |
for remove_str in null_char_list: | |
self.fragment = self.fragment.replace(remove_str, "%00") | |
logger.error('Fragment "%s" has invalid format - It contains the NULL character. The following action was taken: %s', old_value, action_string) | |
if self.fragment == "": | |
self.fragment = None | |
Information Disclosure via Logging in dojo/management/commands/import_all_unittest_scans.py
Vulnerability | Information Disclosure via Logging |
---|---|
Description | The change introduces logging of error messages during scan import. The logged message can be derived from str(e) where e is an exception, or from a message field in an API response. If e is a database error or a general runtime exception, str(e) can contain sensitive information such as database schema details, SQL queries, internal file paths, or full stack traces. This information, if exposed through logs, can aid attackers in understanding the application's internal workings and identifying further vulnerabilities. |
django-DefectDojo/dojo/management/commands/import_all_unittest_scans.py
Lines 183 to 194 in 85e6263
error_messages[module_name + "/" + scan_file.name] = result.get("message", str(e)) | |
except: | |
logger.exception("failed to load %s", module_name) | |
raise | |
logger.error("Error count: %s", error_count) | |
for scan, message in error_messages.items(): | |
logger.error("Error importing scan %s: %s", scan, message) | |
def handle(self, *args, **options): | |
logger.info("EXPERIMENTAL: This command may be changed/deprecated/removed without prior notice.") |
Information Disclosure via Verbose Logging in dojo/tools/cyberwatch_galeax/parser.py
Vulnerability | Information Disclosure via Verbose Logging |
---|---|
Description | The patch logs raw exceptions from file reading operations at the ERROR level. If the file reading fails (e.g., file not found, permission denied), the string representation of the exception will include the full path of the file that was attempted to be accessed. This exposes internal file system paths in application logs. |
django-DefectDojo/dojo/tools/cyberwatch_galeax/parser.py
Lines 22 to 32 in 85e6263
return "Import Cyberwatch Cve and Security Issue data in JSON format, you can get the json from this tool : https://github.com/Galeax/Cyberwatch-API-DefectDojo" | |
def get_findings(self, filename, test): | |
logger.debug("Starting get_findings with filename: %s", filename) | |
try: | |
file_content = self.read_file_content(filename) | |
except Exception as e: | |
logger.error("Error processing file: %s", e) | |
return [] | |
else: | |
data = json.loads(file_content) |
Information Disclosure via Verbose Logging in dojo/tools/cyberwatch_galeax/parser.py
Vulnerability | Information Disclosure via Verbose Logging |
---|---|
Description | The code logs the raw, unsanitized content of cwes and cwes_ids variables when they are not of the expected type. These variables are directly populated from an input JSON file, which an attacker can likely control. This allows an attacker to inject arbitrary data into the application's error logs. |
django-DefectDojo/dojo/tools/cyberwatch_galeax/parser.py
Lines 75 to 86 in 85e6263
# Safely handle when "cwes" is null | |
cwes = json_data.get("cwes") or {} | |
if not isinstance(cwes, dict): | |
logger.error("Invalid cwes data: %s", cwes) | |
cwes = {} | |
cwes_ids = cwes.get("cwe_id", []) | |
if not isinstance(cwes_ids, list): | |
logger.error("Invalid cwe_id data: %s", cwes_ids) | |
cwes_ids = [] | |
if cwes_ids: | |
try: |
Log Injection via Unsanitized Input in dojo/tools/cyberwatch_galeax/parser.py
Vulnerability | Log Injection via Unsanitized Input |
---|---|
Description | The application logs the cvss_v3_vector directly from user-controlled input without prior sanitization. An attacker can provide a crafted JSON input file containing newline characters or other control characters within the cvss_v3_vector field. This allows for log injection, where an attacker can manipulate log entries, making them difficult to parse, potentially obscuring legitimate events, or spoofing log messages. |
django-DefectDojo/dojo/tools/cyberwatch_galeax/parser.py
Lines 519 to 525 in 85e6263
cvssv3_score = vectors[0].scores()[0] | |
severity = vectors[0].severities()[0] | |
return cvssv3, cvssv3_score, severity | |
logger.error("Invalid CVSS v3 vector: %s", cvss_v3_vector) | |
severity = self.convert_severity(json_data.get("cve_level", "Info")) | |
return None, None, severity | |
We've notified @mtesauro.
All finding details can be found in the DryRun Security Dashboard.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TIL - very nice to know!
Using f-strings to format a logging message requires that Python eagerly format the string, even if the logging statement is never executed (e.g., if the log level is above the level of the logging statement), whereas using the extra keyword argument defers formatting until required.
This is unfortunate as f-strings are much more readable. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved
Prepare for G004, there are multiple places left, but this is a first shot.