🛡️ Sentinel: [HIGH] 업로드 엔드포인트의 리소스 제어 불능(DoS) 취약점 수정

.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.

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):
             </p>
             <p>
                 <label for="batch_target_bytes">Target Bytes (per file): <span class="required-star" aria-hidden="true">*</span></label><br>
-                <input type="number" id="batch_target_bytes" name="target_bytes" value="2000000000" min="1" aria-describedby="batch_target_bytes_help batch_target_bytes_preview" required>
+                <input type="number" id="batch_target_bytes" name="target_bytes" value="2000000000" min="1" aria-describedby="batch_target_bytes_help" required>
                 <br><span id="batch_target_bytes_help" class="help-text">Maximum allowed size in bytes for each output file</span>
-                <br><span id="batch_target_bytes_preview" class="help-text" aria-live="polite" style="font-weight: bold; color: #1e7e34;">1.86 GiB</span>
-                <div id="batch_preset_buttons_container" class="preset-container" role="group" aria-label="Preset target sizes for batch">
-                    <button type="button" class="preset-btn" data-bytes="26214400" aria-pressed="false">25 MiB</button>
-                    <button type="button" class="preset-btn" data-bytes="104857600" aria-pressed="false">100 MiB</button>
-                    <button type="button" class="preset-btn" data-bytes="524288000" aria-pressed="false">500 MiB</button>
-                    <button type="button" class="preset-btn" data-bytes="1073741824" aria-pressed="false">1 GiB</button>
-                </div>
             </p>
             <button type="submit" id="batch-submit-btn">Upload and Shrink Batch</button>
         </form>
@@ -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,36 +538,61 @@ 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:
         logger.exception("Failed to create batch upload workspace")
         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(

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(