🛡️ Sentinel: [CRITICAL] Fix IDOR in file deletion#154
Conversation
Co-authored-by: xb1g <70068561+xb1g@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
53f5ed0 to
4de54f3
Compare
|
Unable to deploy a commit from a private repository on your GitHub organization to the wachaa1319's projects team on Vercel, which is currently on the Hobby plan. In order to deploy, you can:
To read more about collaboration on Vercel, click here. |
There was a problem hiding this comment.
Pull request overview
This PR hardens the DELETE handler in the upload API to prevent an IDOR authorization bypass when deleting user submission files stored in Backblaze B2 (S3-compatible), ensuring users can only delete objects within their own submissions prefix.
Changes:
- Replaced the ownership check from
fileName.includes(...)tofileName.startsWith(...)to enforce a strict prefix boundary forsubmissions/${user.id}/. - Added inline security commentary explaining the rationale and bypass scenario.
- Added a Sentinel journal entry documenting the vulnerability and prevention guidance.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| app/api/upload/route.ts | Strengthens file deletion authorization by enforcing a strict submissions/{userId}/ prefix check. |
| .jules/sentinel.md | Documents the IDOR issue and the preferred prevention pattern for path-key authorization checks. |
| **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`). |
4de54f3 to
ba37318
Compare
ba37318 to
f459922
Compare
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ❌ Deployment failed View logs |
web | f459922 | May 11 2026, 06:21 PM |
🚨 Severity: CRITICAL
💡 Vulnerability: In
app/api/upload/route.ts, theDELETEendpoint usedfileName.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). This is an Insecure Direct Object Reference (IDOR) and authorization bypass.🎯 Impact: An attacker could delete other users' files by crafting a file name that contains their own ID as a substring.
🔧 Fix: Updated
app/api/upload/route.tsto use.startsWith('submissions/${user.id}/')instead of.includes(). This enforces a strict hierarchical boundary check for file ownership. Added a security comment explaining the fix. Also added a journal entry in.jules/sentinel.md.✅ Verification: Verified using
pnpm lintandpnpm test. Code review passed successfully.PR created automatically by Jules for task 3789603280855221130 started by @xb1g