Skip to content

Commit 9488ccf

Browse files
committed
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.
1 parent b2a720b commit 9488ccf

File tree

4 files changed

+29
-28
lines changed

4 files changed

+29
-28
lines changed

backend/toolbox/workflows/android_static_analysis/workflow.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,8 @@ async def run(
267267
)
268268

269269
# Calculate summary
270-
total_findings = len(sarif_report.get("runs", [{}])[0].get("results", []))
270+
runs = sarif_report.get("runs", [])
271+
total_findings = len(runs[0].get("results", [])) if runs else 0
271272

272273
summary = {
273274
"workflow": "android_static_analysis",

sdk/src/fuzzforge_sdk/client.py

Lines changed: 14 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -393,10 +393,6 @@ def submit_workflow_with_upload(
393393
# Prepare multipart form data
394394
url = urljoin(self.base_url, f"/workflows/{workflow_name}/upload-and-submit")
395395

396-
files = {
397-
"file": (filename, open(upload_file, "rb"), "application/gzip")
398-
}
399-
400396
data = {}
401397

402398
if parameters:
@@ -418,13 +414,15 @@ def track_progress(monitor):
418414
# This is a placeholder - real implementation would need custom approach
419415
pass
420416

421-
response = self._client.post(url, files=files, data=data)
422-
423-
# Close file handle
424-
files["file"][1].close()
417+
# Use context manager to ensure file handle is closed
418+
with open(upload_file, "rb") as f:
419+
files = {
420+
"file": (filename, f, "application/gzip")
421+
}
422+
response = self._client.post(url, files=files, data=data)
425423

426-
data = self._handle_response(response)
427-
return RunSubmissionResponse(**data)
424+
response_data = self._handle_response(response)
425+
return RunSubmissionResponse(**response_data)
428426

429427
finally:
430428
# Cleanup temporary tarball
@@ -480,10 +478,6 @@ async def asubmit_workflow_with_upload(
480478
# Prepare multipart form data
481479
url = urljoin(self.base_url, f"/workflows/{workflow_name}/upload-and-submit")
482480

483-
files = {
484-
"file": (filename, open(upload_file, "rb"), "application/gzip")
485-
}
486-
487481
data = {}
488482

489483
if parameters:
@@ -494,10 +488,12 @@ async def asubmit_workflow_with_upload(
494488

495489
logger.info(f"Uploading {filename} to {workflow_name}...")
496490

497-
response = await self._async_client.post(url, files=files, data=data)
498-
499-
# Close file handle
500-
files["file"][1].close()
491+
# Use context manager to ensure file handle is closed
492+
with open(upload_file, "rb") as f:
493+
files = {
494+
"file": (filename, f, "application/gzip")
495+
}
496+
response = await self._async_client.post(url, files=files, data=data)
501497

502498
response_data = await self._ahandle_response(response)
503499
return RunSubmissionResponse(**response_data)

sdk/src/fuzzforge_sdk/exceptions.py

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -415,16 +415,20 @@ def from_http_error(status_code: int, response_text: str, url: str) -> FuzzForge
415415
if "/workflows/" in url and "/submit" not in url:
416416
# Extract workflow name from URL
417417
parts = url.split("/workflows/")
418-
if len(parts) > 1:
419-
workflow_name = parts[1].split("/")[0]
420-
return WorkflowNotFoundError(workflow_name, context=context)
418+
if len(parts) > 1 and parts[1]:
419+
workflow_segments = parts[1].split("/")
420+
if workflow_segments and workflow_segments[0]:
421+
workflow_name = workflow_segments[0]
422+
return WorkflowNotFoundError(workflow_name, context=context)
421423

422424
elif "/runs/" in url:
423425
# Extract run ID from URL
424426
parts = url.split("/runs/")
425-
if len(parts) > 1:
426-
run_id = parts[1].split("/")[0]
427-
return RunNotFoundError(run_id, context)
427+
if len(parts) > 1 and parts[1]:
428+
run_segments = parts[1].split("/")
429+
if run_segments and run_segments[0]:
430+
run_id = run_segments[0]
431+
return RunNotFoundError(run_id, context)
428432

429433
elif status_code == 400:
430434
# Check for specific error patterns in response

workers/ossfuzz/activities.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -368,11 +368,11 @@ def parse_fuzzing_stats(stdout: str, stderr: str, engine: str) -> Dict[str, Any]
368368
# Example: #8192 NEW cov: 1234 ft: 5678 corp: 89/10KB
369369
parts = line.split()
370370
for i, part in enumerate(parts):
371-
if part.startswith("cov:"):
371+
if part.startswith("cov:") and i+1 < len(parts):
372372
stats["coverage"] = int(parts[i+1])
373-
elif part.startswith("corp:"):
373+
elif part.startswith("corp:") and i+1 < len(parts):
374374
stats["corpus_entries"] = int(parts[i+1].split('/')[0])
375-
elif part.startswith("exec/s:"):
375+
elif part.startswith("exec/s:") and i+1 < len(parts):
376376
stats["executions_per_sec"] = float(parts[i+1])
377377
elif part.startswith("#"):
378378
stats["total_executions"] = int(part[1:])

0 commit comments

Comments
 (0)