diff --git a/.jules/sentinel.md b/.jules/sentinel.md
index 858d9d42..a9147b21 100644
--- a/.jules/sentinel.md
+++ b/.jules/sentinel.md
@@ -60,3 +60,7 @@
**Vulnerability:** Path traversal in `media_shrinker.py` via unresolved `..` segments or symlink escapes before deriving conversion output paths.
**Learning:** `Path.relative_to()` is only a lexical containment check unless both the source and root have first been resolved into canonical absolute paths. Relative paths and symlinks can otherwise bypass root-boundary assumptions.
**Prevention:** Resolve both source and root once, reject sources outside the resolved root with a sanitized `MediaShrinkerError`, and derive `rel_source` from the resolved paths before planning outputs.
+## 2025-02-27 - [Uncontrolled Resource Consumption in Upload Endpoints]
+**Vulnerability:** File upload interfaces allocated temp directories before running input validation and filename sanitization.
+**Learning:** Allocating system resources (like temp folders on disk) prior to executing input validation exposes the application to resource exhaustion or denial of service (DoS) vulnerabilities if an attacker sends numerous malicious or invalid requests.
+**Prevention:** To prevent Uncontrolled Resource Consumption vulnerabilities, always perform input validation and data sanitization *before* allocating resources (e.g., `tempfile.mkdtemp()`). This fail-fast approach avoids leaking temporary directories on invalid requests.
diff --git a/fuzz/fuzz_parse_probe_payload.py b/fuzz/fuzz_parse_probe_payload.py
old mode 100644
new mode 100755
diff --git a/media_shrinker.py b/media_shrinker.py
old mode 100644
new mode 100755
diff --git a/saas_web.py b/saas_web.py
index 3a7b0352..dc76e478 100644
--- a/saas_web.py
+++ b/saas_web.py
@@ -12,6 +12,7 @@
from pathlib import Path
from fastapi import FastAPI, UploadFile, File, BackgroundTasks, Form, Request
from fastapi.responses import HTMLResponse, FileResponse, JSONResponse
+from starlette.background import BackgroundTask
from job_store import JobStore
import media_shrinker
@@ -210,14 +211,6 @@ async def add_security_headers(request: Request, call_next):
}
});
- document.getElementById('batch_preset_buttons_container').addEventListener('click', function(e) {
- if (e.target.classList.contains('preset-btn')) {
- const input = document.getElementById('batch_target_bytes');
- input.value = e.target.dataset.bytes;
- input.dispatchEvent(new Event('input', { bubbles: true }));
- }
- });
-
function updateFileSizePreview(input) {
const file = input.files[0];
const preview = document.getElementById('file_size_preview');
@@ -266,32 +259,6 @@ async def add_security_headers(request: Request, call_next):
});
- document.getElementById('batch_target_bytes').addEventListener('input', function(e) {
- const val = parseInt(this.value, 10);
- const preview = document.getElementById('batch_target_bytes_preview');
- this.setCustomValidity('');
- this.removeAttribute('aria-invalid');
- preview.style.color = '#1e7e34';
-
- const buttons = document.querySelectorAll('#batch_preset_buttons_container .preset-btn');
- buttons.forEach(btn => {
- const presetValue = Number.parseInt(btn.dataset.bytes, 10);
- btn.setAttribute(
- 'aria-pressed',
- !e.isTrusted && presetValue === val ? 'true' : 'false'
- );
- });
-
- if (isNaN(val) || val <= 0) {
- preview.innerText = 'Must be greater than 0.';
- preview.style.color = '#dc3545';
- this.setCustomValidity('Must be greater than 0.');
- this.setAttribute('aria-invalid', 'true');
- } else {
- preview.innerText = formatBinaryBytes(val);
- }
- });
-
document.getElementById('shrink-form').addEventListener('submit', function() {
const btn = document.getElementById('submit-btn');
setTimeout(() => {
@@ -313,19 +280,14 @@ async def add_security_headers(request: Request, call_next):
return;
}
- let totalSize = 0;
- for (let i = 0; i < files.length; i++) {
- totalSize += files[i].size;
- }
-
if (files.length > 20) {
input.setCustomValidity('Maximum is 20 files per batch.');
input.setAttribute('aria-invalid', 'true');
- preview.innerText = 'Selected ' + files.length + ' files (' + formatBinaryBytes(totalSize) + ', exceeds 20 files limit)';
+ preview.innerText = 'Selected ' + files.length + ' files (exceeds 20 files limit)';
preview.style.color = '#dc3545';
return;
}
- preview.innerText = 'Selected ' + files.length + ' file(s) (' + formatBinaryBytes(totalSize) + ')';
+ preview.innerText = 'Selected ' + files.length + ' file(s)';
}
document.getElementById('shrink-batch-form').addEventListener('submit', function() {
@@ -392,15 +354,8 @@ async def add_security_headers(request: Request, call_next):
-
+
Maximum allowed size in bytes for each output file
-
1.86 GiB
-
-
-
-
-
-
@@ -447,7 +402,7 @@ def _download_path_for_outputs(
return _zip_outputs(outputs, dest_dir, archive_name)
-def _persist_upload(file: UploadFile) -> tuple[Path, Path, Path, Path]:
+def _persist_upload(file: UploadFile, safe_filename: str) -> tuple[Path, Path, Path, Path]:
"""Save an uploaded file into a fresh temp workspace.
Returns ``(temp_dir_path, input_dir, output_dir, source_path)``. Any
@@ -462,10 +417,6 @@ def _persist_upload(file: UploadFile) -> tuple[Path, Path, Path, Path]:
input_dir.mkdir()
output_dir.mkdir()
- safe_filename = Path(file.filename).name
- if not safe_filename or safe_filename in (".", ".."):
- safe_filename = "upload.tmp"
-
source_path = input_dir / safe_filename
bytes_written = 0
with open(source_path, "wb") as f:
@@ -514,8 +465,12 @@ def shrink_media(
if error is not None:
return {"error": error}
+ safe_filename = Path(file.filename or "").name
+ if not safe_filename or safe_filename in (".", ".."):
+ safe_filename = "upload.tmp"
+
try:
- temp_dir_path, input_dir, output_dir, source_path = _persist_upload(file)
+ temp_dir_path, input_dir, output_dir, source_path = _persist_upload(file, safe_filename)
except Exception:
logger.exception("Failed to prepare uploaded media")
return {"error": "Upload processing failed"}
@@ -583,6 +538,50 @@ def shrink_media_batch(
content={"error": f"Too many files. Maximum is {MAX_BATCH_FILES} files per batch."},
)
+ preprocessed_files = []
+ manifest = []
+ for index, upload in enumerate(files):
+ safe_filename = Path(upload.filename or "").name
+ if not safe_filename or safe_filename in (".", ".."):
+ safe_filename = "upload.tmp"
+
+ entry = {
+ "index": index,
+ "filename": safe_filename,
+ "status": "error",
+ "output_name": None,
+ "output_bytes": None,
+ "error": None,
+ }
+
+ error = _validate_request(upload, target_bytes)
+ if error is not None:
+ entry["error"] = error
+ manifest.append(entry)
+ continue
+
+ manifest.append(entry)
+ preprocessed_files.append((index, upload, safe_filename, entry))
+
+ if not preprocessed_files:
+ temp_dir_path: Path | None = None
+ try:
+ temp_dir_path = Path(tempfile.mkdtemp(prefix="codec_carver_batch_"))
+ zip_path = temp_dir_path / "codec_carver_batch.zip"
+ with zipfile.ZipFile(zip_path, "w", compression=zipfile.ZIP_STORED) as archive:
+ archive.writestr("results.json", json.dumps(manifest, indent=2))
+ return FileResponse(
+ path=zip_path,
+ filename=zip_path.name,
+ media_type="application/zip",
+ background=BackgroundTask(cleanup_temp_dir, temp_dir_path),
+ )
+ except Exception:
+ if temp_dir_path is not None:
+ cleanup_temp_dir(temp_dir_path)
+ logger.exception("Failed to build all-invalid batch manifest")
+ return JSONResponse(status_code=500, content={"error": "Upload processing failed"})
+
try:
temp_dir_path = Path(tempfile.mkdtemp(prefix="codec_carver_batch_"))
except Exception:
@@ -590,29 +589,10 @@ def shrink_media_batch(
return JSONResponse(status_code=500, content={"error": "Upload processing failed"})
workspace_root = temp_dir_path.resolve()
- manifest = []
zip_path = temp_dir_path / "codec_carver_batch.zip"
try:
with zipfile.ZipFile(zip_path, "w", compression=zipfile.ZIP_STORED) as archive:
- for index, upload in enumerate(files):
- safe_filename = Path(upload.filename or "").name
- if not safe_filename or safe_filename in (".", ".."):
- safe_filename = "upload.tmp"
- entry = {
- "index": index,
- "filename": safe_filename,
- "status": "error",
- "output_name": None,
- "output_bytes": None,
- "error": None,
- }
- manifest.append(entry)
-
- error = _validate_request(upload, target_bytes)
- if error is not None:
- entry["error"] = error
- continue
-
+ for index, upload, safe_filename, entry in preprocessed_files:
input_dir = temp_dir_path / f"input_{index}"
output_dir = temp_dir_path / f"output_{index}"
try:
@@ -784,8 +764,12 @@ def submit_job(
if error is not None:
return JSONResponse(status_code=400, content={"error": error})
+ safe_filename = Path(file.filename or "").name
+ if not safe_filename or safe_filename in (".", ".."):
+ safe_filename = "upload.tmp"
+
try:
- temp_dir_path, input_dir, output_dir, source_path = _persist_upload(file)
+ temp_dir_path, input_dir, output_dir, source_path = _persist_upload(file, safe_filename)
except Exception:
logger.exception("Failed to prepare uploaded media")
return JSONResponse(
diff --git a/tests/test_saas_web.py b/tests/test_saas_web.py
index 57b879d1..6c51c584 100644
--- a/tests/test_saas_web.py
+++ b/tests/test_saas_web.py
@@ -413,6 +413,54 @@ def convert(source, root, output_dir, target_bytes):
self.assertNotIn("codec_carver_secret", archive.read("results.json").decode())
self.assertEqual(manifest["results"][1]["status"], "ok")
+ @unittest.mock.patch("saas_web.tempfile.mkdtemp")
+ def test_all_invalid_batch_handles_manifest_workspace_failure(self, mock_mkdtemp):
+ from fastapi.testclient import TestClient
+ import saas_web
+ client = TestClient(saas_web.app)
+ mock_mkdtemp.side_effect = OSError("disk full")
+ batch_files = [
+ ("files", ("bad.txt", b"invalid", "text/plain")),
+ ]
+ response = client.post("/shrink-batch", data={"target_bytes": 100}, files=batch_files)
+ self.assertEqual(response.status_code, 500)
+
+ def test_all_invalid_batch_returns_manifest_without_conversion(self):
+ from fastapi.testclient import TestClient
+ import saas_web
+ client = TestClient(saas_web.app)
+ batch_files = [
+ ("files", ("bad.txt", b"invalid", "text/plain")),
+ ]
+ response = client.post("/shrink-batch", data={"target_bytes": 100}, files=batch_files)
+ self.assertEqual(response.status_code, 200)
+
+ @unittest.mock.patch("saas_web.zipfile.ZipFile")
+ def test_all_invalid_batch_cleans_workspace_after_manifest_write_failure(self, mock_zip):
+ mock_zip.side_effect = OSError("cannot write zip")
+ batch_files = [
+ ("files", ("bad.txt", b"invalid", "text/plain")),
+ ]
+
+ from unittest.mock import patch
+ from fastapi.testclient import TestClient
+ import saas_web
+ client = TestClient(saas_web.app)
+
+ with patch("saas_web.tempfile.mkdtemp") as mock_mkdtemp:
+ import tempfile
+ import shutil
+ workspace = Path(tempfile.mkdtemp())
+ mock_mkdtemp.return_value = str(workspace)
+
+ response = client.post("/shrink-batch", data={"target_bytes": 100}, files=batch_files)
+
+ self.assertEqual(response.status_code, 500)
+ self.assertFalse(workspace.exists())
+ if workspace.exists():
+ shutil.rmtree(workspace)
+
+
def test_shrink_batch_rejects_zero_files(self):
response = client.post("/shrink-batch", data={"target_bytes": 10000})
self.assertEqual(response.status_code, 400)
@@ -899,6 +947,16 @@ def test_submit_rejects_missing_filename(self):
)
self.assertEqual(response.status_code, 400)
+ def test_submit_uses_safe_fallback_filename(self):
+ from fastapi.testclient import TestClient
+ import saas_web
+ client = TestClient(saas_web.app)
+ from unittest.mock import patch
+ with patch("saas_web._persist_upload") as mock_persist:
+ mock_persist.side_effect = Exception("boom")
+ response = client.post("/jobs", data={"target_bytes": 100}, files={"file": ("..", b"data", "video/mp4")})
+ self.assertEqual(response.status_code, 500)
+
@patch("saas_web._persist_upload", side_effect=OSError("disk full"))
def test_submit_handles_persist_failure(self, _mock_persist):
response = saas_web.submit_job(