🛡️ Sentinel: [CRITICAL] Fix IDOR in file deletion

.jules/sentinel.md

@@ -6,4 +6,9 @@
 ## 2025-03-08 - Unsafe SVG Rendering via dangerouslySetInnerHTML
 **Vulnerability:** User-controlled SVG strings (`avatar.svg_data`) were being injected directly into the DOM using `dangerouslySetInnerHTML`. An attacker could inject an SVG containing embedded malicious `<script>` tags or `onload` event handlers, leading to Stored Cross-Site Scripting (XSS).
 **Learning:** Rendering raw SVGs inline is inherently dangerous. SVGs are essentially XML documents capable of embedding JavaScript.
-**Prevention:** When you need to display user-provided SVGs and you don't need to manipulate their internal paths via CSS/JS, do not use `dangerouslySetInnerHTML`. Instead, render the SVG securely by encoding it as a data URI (`data:image/svg+xml;charset=utf-8,${encodeURIComponent(svgData)}`) and using it as the `src` attribute of a standard `<img>` tag. This forces the browser to treat the SVG strictly as an image, entirely disabling any embedded script execution.
+**Prevention:** When you need to display user-provided SVGs and you don't need to manipulate their internal paths via CSS/JS, do not use `dangerouslySetInnerHTML`. Instead, render the SVG securely by encoding it as a data URI (`data:image/svg+xml;charset=utf-8,${encodeURIComponent(svgData)}`) and using it as the `src` attribute of a standard `<img>` tag. This forces the browser to treat the SVG strictly as an image, entirely disabling any embedded script execution.
+
+## 2025-03-08 - IDOR Vulnerability in File Deletion via string.includes()
+**Vulnerability:** In `app/api/upload/route.ts`, the `DELETE` endpoint used `fileName.includes('submissions/${user.id}/')` to verify that the file being deleted belonged to the requesting user. Since cloud storage keys (like S3/B2) are treated as literal strings, an attacker could theoretically bypass this by constructing a key that contains their user ID as a substring (e.g., `other-user/submissions/${attacker_id}/target-file.jpg`).
+**Learning:** `string.includes()` is an inherently weak authorization check for hierarchical paths because it matches anywhere in the string.
+**Prevention:** Always use exact string matching or prefix matching (`.startsWith()`) when verifying ownership via path keys to enforce strict hierarchical boundaries.

app/api/upload/route.ts

@@ -75,7 +75,9 @@ export async function DELETE(request: NextRequest) {
       return NextResponse.json({ error: "Valid fileName is required" }, { status: 400 });
     }
 
-    if (!fileName.includes(`submissions/${user.id}/`)) {
+    // 🛡️ SECURITY: Use startsWith() instead of includes() to prevent path traversal/IDOR.
+    // An attacker could bypass includes() by naming a file like `otheruser/submissions/${user.id}/file.ext`
+    if (!fileName.startsWith(`submissions/${user.id}/`)) {
       return NextResponse.json({ error: "Access denied" }, { status: 403 });
     }