Repository navigation
fix(id-web): safe blob content types — no caller-chosen text/html (#908) - #919
Conversation
blob_handler derived its Content-Type from the caller-supplied ?filename= parameter and sent no protective headers, so any stored blob could be served as text/html from the id origin (same-origin XSS by link). - safe_blob_content_type: only passive types (images incl. SVG, video, audio, PDF, plain/markdown text) are served inline with their type; everything else is application/octet-stream + Content-Disposition: attachment. - Every blob response now carries X-Content-Type-Options: nosniff and Content-Security-Policy: default-src 'none'; sandbox, so an SVG cannot execute on top-level navigation while <img> embedding keeps working. - upload_handler reconciles a supplied filename's extension with the validated content type (a PNG named evil.html is stored as evil.png) and percent-encodes the filename in the returned URL; images_list_handler encodes its blob URLs the same way. - Regression tests in upload_router_tests drive the real router: ?filename=evil.html is not served as HTML, PNG stays inline image/png with protective headers, SVG carries the sandbox CSP, and a masquerading upload name is reconciled.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 55 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 97718382e6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .header(header::X_CONTENT_TYPE_OPTIONS, "nosniff") | ||
| .header( | ||
| header::CONTENT_SECURITY_POLICY, | ||
| "default-src 'none'; sandbox", |
There was a problem hiding this comment.
For uploaded or synced SVGs that use <style> blocks or style attributes, this policy blocks their styling when the blob is opened directly, changing colors, hiding strokes, or making diagrams unreadable. With no explicit style-src, it falls back to default-src 'none' under the CSP specification. SVG remains an explicitly supported inline type, so preserve its non-executable styling, for example with style-src 'unsafe-inline', while retaining the sandbox and script restrictions. The new <svg/> response-header test cannot detect this rendering regression.
Useful? React with 👍 / 👎.
Fixes #908.
Problem
GET /blob/{hash}derived itsContent-Typefrom the caller-supplied?filename=parameter and sent no protective headers, so any stored blob (including content synced from peers) could be served astext/htmlfrom the id web origin — same-origin XSS by link, persisted by the immutable year-long cache header.Fix (pkgs/id/src/web/routes.rs only)
safe_blob_content_typekeeps the filename-implied type only for passive types (images incl. SVG, video, audio, PDF, plain/markdown text) served inline; everything else (html/css/js/json/xml/unknown) becomesapplication/octet-stream+Content-Disposition: attachment.X-Content-Type-Options: nosniffandContent-Security-Policy: default-src 'none'; sandbox— an SVG cannot execute on top-level navigation, while<img>embedding of the upload URL keeps working.upload_handlerreconciles a supplied filename's extension with the validated content type (evil.htmldeclaredimage/pngis stored asevil.png; jpeg accepts jpg/jpeg) and percent-encodes the filename in the returned URL;images_list_handlerencodes its blob URLs the same way.Tests
New tests in
upload_router_testsdrive the real router end-to-end (upload → GET /blob):?filename=evil.htmlon a PNG blob →application/octet-stream+ attachment + nosniff + sandbox CSP, bytes unchangedimage/png, no disposition, protective headers presentimage/svg+xmlwithdefault-src 'none'; sandboxCSP + nosniffevil.html/image/png→ stored/returned asevil.pngsafe_blob_content_typeandreconcile_upload_filenameLocal verification: rustfmt (edition 2024) clean. Per the sandbox build rule no local cargo compile was run (id lib-test link OOMs in the sandbox); CI is the test executor — merge after the flake check leg executes these tests green.
Need help on this PR? Tag
@codesmith-botwith what you need.