Skip to content

webcore/Blob: validate content-type in the structured-clone deserializer - #33946

Open
robobun wants to merge 2 commits into
mainfrom
farm/df01b129/blob-deserialize-content-type-validation
Open

webcore/Blob: validate content-type in the structured-clone deserializer#33946
robobun wants to merge 2 commits into
mainfrom
farm/df01b129/blob-deserialize-content-type-validation

Conversation

@robobun

@robobun robobun commented Jul 11, 2026

Copy link
Copy Markdown
Collaborator

Problem

The Blob structured-clone deserializer stored the wire record's content_type field verbatim, with no validation or lowercasing. A crafted serialized image can therefore produce a Blob whose .type holds bytes the Blob constructor would reject (control characters, CR/LF, bytes above 0x7E, or simply un-lowercased ASCII), and that string then propagates unchanged into new Response(blob)'s Content-Type header.

import * as v8 from "node:v8";
const T = "TEXT/HT" + String.fromCharCode(1) + "L";
const img = Buffer.concat([
  Buffer.from([14,0,0,0, 254, 4]), Buffer.alloc(8),
  Buffer.from([T.length,0,0,0]), Buffer.from(T, "binary"),
  Buffer.from([1, 1, 2,0,0,0]), Buffer.from("hi"),
  Buffer.from([0,0,0,0,0]), Buffer.alloc(8),
]);
const blob = v8.deserialize(img);
blob.type                                       // "TEXT/HT\^AL"
new Blob(["hi"], { type: T }).type              // ""  (the constructor rejects it)
new Response(blob).headers.get("content-type")  // "TEXT/HT\^AL"

An object state unreachable from JS materialises from bytes and flows into an HTTP header; anything that serves deserialized Blobs is exposed to header-value bytes the application never produced.

Cause

_on_structured_clone_deserialize in src/runtime/webcore/Blob.rs reads the length-prefixed content_type field and assigns it directly:

if !content_type.is_empty() {
    blob.content_type
        .set(BlobContentType::Owned(std::sync::Arc::from(content_type)));
    blob.content_type_was_set.set(content_type_was_set);
}

Every JS-facing entry point (the Blob/File constructors, Blob.prototype.slice, Bun.file().write, the S3 file path) runs the string through is_valid_blob_type (WHATWG File API 3.1: every byte in U+0020..U+007E) and then lowercases it. The wire reader skipped both.

Fix

Apply the same guard and normalization on the deserialize path: reject to the empty string when any byte is outside U+0020..U+007E, otherwise store the lowercased value via the same mime_type lookup / BlobContentType::from_lowercased step every other entry point uses. Honest payloads Bun itself wrote are already normalized, so only crafted images are affected.

This is the third wire-field validator at this boundary, alongside the existing fd-range and NUL-in-path checks in the same function.

Verification

New test in test/js/web/structured-clone-blob-file.test.ts under deserialize of crafted payloads. It serialises a Blob with a sentinel content-type, locates the field in the output, and patches it with control bytes, CR/LF, DEL, a byte above 0x7F, and all-uppercase input, exercising both bun:jsc and node:v8 deserialize entry points. Each case also checks new Blob(parts, {type}) with the same string so the two paths are asserted to agree.

USE_SYSTEM_BUN=1 bun test test/js/web/structured-clone-blob-file.test.ts -t "crafted content-type"
  0 pass, 1 fail  (type: "TEXT/HTM\^A;A=BCD", response: "TEXT/HTM\^A;A=BCD")

bun bd test test/js/web/structured-clone-blob-file.test.ts
  42 pass, 0 fail

Also green: test/js/web/fetch/blob.test.ts (42 pass) and test/js/web/workers/structured-clone.test.ts (226 pass).

Note: #33605 (not yet landed) removes the vm.mime_type() lookup at the user-type call sites in favour of plain from_lowercased. Whichever lands second needs a one-line rebase at this site; the validation and the test here are unaffected.


no test proof · iteration 0 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/web/structured-clone-blob-file.test.ts

The Blob structured-clone deserializer stored the wire record's
content-type verbatim, so a crafted image could produce a Blob whose
.type contained control bytes or un-lowercased text, a state no
new Blob() can reach, and that string then propagated into
new Response(blob)'s Content-Type header.

Apply the same is_valid_blob_type + lowercase step the Blob
constructor uses. This is the third validator at this boundary,
alongside the existing fd-range and NUL-in-path checks.
@coderabbitai

coderabbitai Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 57 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: e5d9ca00-7f70-4dd9-9fd3-2aa314cae288

📥 Commits

Reviewing files that changed from the base of the PR and between ab84aa2 and 7d96175.

📒 Files selected for processing (2)
  • src/runtime/webcore/Blob.rs
  • test/js/web/structured-clone-blob-file.test.ts

Comment @coderabbitai help to get the list of available commands.

@robobun

robobun commented Jul 11, 2026

Copy link
Copy Markdown
Collaborator Author
Updated 7:55 PM PT - Jul 10th, 2026

@autofix-ci[bot], your commit 7d96175 has 3 failures in Build #71706 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 33946

That installs a local version of the PR into your bun-33946 executable, so you can run:

bun-33946 --bun

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — third wire-field validator in this function, mechanically applies the same is_valid_blob_type + mime_type/from_lowercased pattern used at every other content-type entry point in Blob.rs.

What was reviewed:

  • Confirmed the new guard matches the 6 existing call sites (constructors, slice, write) byte-for-byte.
  • Checked content_type is a local Vec<u8> from read_slice, so the rejected branch just drops it — no leak vs. the old Arc::from path.
  • Test covers control bytes, CR/LF, DEL, ≥0x80, uppercase normalization, and unmodified round-trip on both bun:jsc and node:v8 deserializers, and cross-checks each against new Blob({type}).
Extended reasoning...

Overview

This PR adds input validation to the Blob structured-clone deserializer's content_type field in src/runtime/webcore/Blob.rs. Previously the wire bytes were stored verbatim via BlobContentType::Owned(Arc::from(content_type)); now they pass through is_valid_blob_type (WHATWG File API §3.1: every byte in U+0020..U+007E) and the same vm.mime_type() / BlobContentType::from_lowercased normalization used by every other user-facing entry point in the file. A comprehensive test is added to test/js/web/structured-clone-blob-file.test.ts.

Security risks

The change is a hardening: it adds validation on untrusted deserialized bytes and fails closed to the empty string, matching the constructor. There is no new attack surface — the risk direction is strictly narrowing. The concern being addressed (control bytes / CR-LF flowing into a Content-Type response header) is real but requires the application to deserialize attacker-controlled v8/jsc-serialized bytes and then serve the resulting Blob, which is already a hostile-input scenario. No auth, crypto, or permission logic is touched.

Level of scrutiny

Low-to-medium. The Rust change is 7 lines and is a verbatim copy of the pattern at lines ~1372, ~1792, ~2101, ~2456, ~5699, and ~5789 of the same file. This is the third PR in a row applying wire-field validation to _on_structured_clone_deserialize#33920 (negative fd) and #33921 (NUL in path) landed the same week with identical structure, so the pattern and review bar are well established. Honest payloads Bun wrote itself are already lowercased and in-range, so the only observable behaviour change is on crafted images.

Other factors

  • The test is thorough: it locates the field by sentinel rather than hard-coding offsets (robust to framing changes), exercises both deserialize entry points, asserts parity with new Blob({type}) for each input, and verifies the header-injection vector via new Response(blob).headers.get('content-type').
  • The content_type local is a Vec<u8> returned by read_slice; in the new rejected branch it simply drops at end of scope, so no allocation lifetime changes vs. the old code.
  • The PR description flags a trivial one-line rebase interaction with #33605 (which drops the mime_type() lookup) — whichever lands second adjusts; the validation and test are unaffected.
  • No prior human or bot review comments to address; bug-hunting system found nothing.

@robobun

robobun commented Jul 11, 2026

Copy link
Copy Markdown
Collaborator Author

CI status: the diff itself is green. test/js/web/structured-clone-blob-file.test.ts (the file this PR changes) passed on every lane.

The failures in build 71706 are unrelated to this change:

Ready for review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant