From a9974bcbf1e1e49a2262b407786d45e9db6b4409 Mon Sep 17 00:00:00 2001 From: tduhamel42 Date: Wed, 12 Nov 2025 14:16:02 +0100 Subject: [PATCH] fix: Resolve critical bugs - file handle leaks and IndexError issues Fixed multiple critical bugs identified during comprehensive code audit: **Critical Fixes:** - Fix file handle leaks in SDK client upload methods (sync and async) - Use context managers to ensure file handles are properly closed - Affects: sdk/src/fuzzforge_sdk/client.py lines 397, 484 **High Priority Fixes:** - Fix IndexError in OSS-Fuzz stats parsing when accessing array elements - Add bounds checking before accessing parts[i+1] - Affects: workers/ossfuzz/activities.py lines 372-376 - Fix IndexError in exception handling URL parsing - Add empty string validation before splitting URL segments - Prevents crash when parsing malformed URLs - Affects: sdk/src/fuzzforge_sdk/exceptions.py lines 419-426 **Medium Priority Fixes:** - Fix IndexError in Android workflow SARIF report parsing - Check if runs list is empty before accessing first element - Affects: backend/toolbox/workflows/android_static_analysis/workflow.py line 270 All fixes follow defensive programming practices with proper bounds checking and resource management using context managers. --- .../android_static_analysis/workflow.py | 3 +- sdk/src/fuzzforge_sdk/client.py | 32 ++++++++----------- sdk/src/fuzzforge_sdk/exceptions.py | 16 ++++++---- workers/ossfuzz/activities.py | 6 ++-- 4 files changed, 29 insertions(+), 28 deletions(-) diff --git a/backend/toolbox/workflows/android_static_analysis/workflow.py b/backend/toolbox/workflows/android_static_analysis/workflow.py index 8376cd2..e7e8929 100644 --- a/backend/toolbox/workflows/android_static_analysis/workflow.py +++ b/backend/toolbox/workflows/android_static_analysis/workflow.py @@ -267,7 +267,8 @@ class AndroidStaticAnalysisWorkflow: ) # Calculate summary - total_findings = len(sarif_report.get("runs", [{}])[0].get("results", [])) + runs = sarif_report.get("runs", []) + total_findings = len(runs[0].get("results", [])) if runs else 0 summary = { "workflow": "android_static_analysis", diff --git a/sdk/src/fuzzforge_sdk/client.py b/sdk/src/fuzzforge_sdk/client.py index c4f29d3..8e1f5aa 100644 --- a/sdk/src/fuzzforge_sdk/client.py +++ b/sdk/src/fuzzforge_sdk/client.py @@ -393,10 +393,6 @@ class FuzzForgeClient: # Prepare multipart form data url = urljoin(self.base_url, f"/workflows/{workflow_name}/upload-and-submit") - files = { - "file": (filename, open(upload_file, "rb"), "application/gzip") - } - data = {} if parameters: @@ -418,13 +414,15 @@ class FuzzForgeClient: # This is a placeholder - real implementation would need custom approach pass - response = self._client.post(url, files=files, data=data) + # Use context manager to ensure file handle is closed + with open(upload_file, "rb") as f: + files = { + "file": (filename, f, "application/gzip") + } + response = self._client.post(url, files=files, data=data) - # Close file handle - files["file"][1].close() - - data = self._handle_response(response) - return RunSubmissionResponse(**data) + response_data = self._handle_response(response) + return RunSubmissionResponse(**response_data) finally: # Cleanup temporary tarball @@ -480,10 +478,6 @@ class FuzzForgeClient: # Prepare multipart form data url = urljoin(self.base_url, f"/workflows/{workflow_name}/upload-and-submit") - files = { - "file": (filename, open(upload_file, "rb"), "application/gzip") - } - data = {} if parameters: @@ -494,10 +488,12 @@ class FuzzForgeClient: logger.info(f"Uploading {filename} to {workflow_name}...") - response = await self._async_client.post(url, files=files, data=data) - - # Close file handle - files["file"][1].close() + # Use context manager to ensure file handle is closed + with open(upload_file, "rb") as f: + files = { + "file": (filename, f, "application/gzip") + } + response = await self._async_client.post(url, files=files, data=data) response_data = await self._ahandle_response(response) return RunSubmissionResponse(**response_data) diff --git a/sdk/src/fuzzforge_sdk/exceptions.py b/sdk/src/fuzzforge_sdk/exceptions.py index c587658..31b3184 100644 --- a/sdk/src/fuzzforge_sdk/exceptions.py +++ b/sdk/src/fuzzforge_sdk/exceptions.py @@ -415,16 +415,20 @@ def from_http_error(status_code: int, response_text: str, url: str) -> FuzzForge if "/workflows/" in url and "/submit" not in url: # Extract workflow name from URL parts = url.split("/workflows/") - if len(parts) > 1: - workflow_name = parts[1].split("/")[0] - return WorkflowNotFoundError(workflow_name, context=context) + if len(parts) > 1 and parts[1]: + workflow_segments = parts[1].split("/") + if workflow_segments and workflow_segments[0]: + workflow_name = workflow_segments[0] + return WorkflowNotFoundError(workflow_name, context=context) elif "/runs/" in url: # Extract run ID from URL parts = url.split("/runs/") - if len(parts) > 1: - run_id = parts[1].split("/")[0] - return RunNotFoundError(run_id, context) + if len(parts) > 1 and parts[1]: + run_segments = parts[1].split("/") + if run_segments and run_segments[0]: + run_id = run_segments[0] + return RunNotFoundError(run_id, context) elif status_code == 400: # Check for specific error patterns in response diff --git a/workers/ossfuzz/activities.py b/workers/ossfuzz/activities.py index 7b0ef7c..664a879 100644 --- a/workers/ossfuzz/activities.py +++ b/workers/ossfuzz/activities.py @@ -368,11 +368,11 @@ def parse_fuzzing_stats(stdout: str, stderr: str, engine: str) -> Dict[str, Any] # Example: #8192 NEW cov: 1234 ft: 5678 corp: 89/10KB parts = line.split() for i, part in enumerate(parts): - if part.startswith("cov:"): + if part.startswith("cov:") and i+1 < len(parts): stats["coverage"] = int(parts[i+1]) - elif part.startswith("corp:"): + elif part.startswith("corp:") and i+1 < len(parts): stats["corpus_entries"] = int(parts[i+1].split('/')[0]) - elif part.startswith("exec/s:"): + elif part.startswith("exec/s:") and i+1 < len(parts): stats["executions_per_sec"] = float(parts[i+1]) elif part.startswith("#"): stats["total_executions"] = int(part[1:])