Skip to content

Add UAF regression test for Response body with transferred ArrayBuffer#6579

Merged
jasnell merged 2 commits intomainfrom
erikcorry/body-uaf-regression-test
Apr 15, 2026
Merged

Add UAF regression test for Response body with transferred ArrayBuffer#6579
jasnell merged 2 commits intomainfrom
erikcorry/body-uaf-regression-test

Conversation

@erikcorry
Copy link
Copy Markdown
Contributor

The previous commit fixed a use-after-free where constructing a Response from an ArrayBuffer, then transferring the buffer via structuredClone and triggering GC, caused the Response body read to access freed backing store memory. This adds a regression test that reproduces the original crash (heap-use-after-free READ of size 1024 under ASAN) and verifies the data remains intact after transfer + GC.

@erikcorry erikcorry requested review from a team as code owners April 14, 2026 07:53
The previous commit fixed a use-after-free where constructing a Response
from an ArrayBuffer, then transferring the buffer via structuredClone
and triggering GC, caused the Response body read to access freed backing
store memory. This adds a regression test that reproduces the original
crash (heap-use-after-free READ of size 1024 under ASAN) and verifies
the data remains intact after transfer + GC.
@erikcorry erikcorry force-pushed the erikcorry/body-uaf-regression-test branch from 2460bb1 to e62debb Compare April 14, 2026 07:54
Copy link
Copy Markdown
Contributor

@ask-bonk ask-bonk bot left a comment

Choose a reason for hiding this comment

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

I'm Bonk, and I've done a quick review of your PR.

This adds a regression test for a UAF when constructing a Response from an ArrayBuffer that is subsequently transferred and GC'd, plus an updated comment in http.c++ explaining the fix.

Issues (ranked by severity):

  1. Missing copyright header on the new test file response-uaf-test.js. Every other .js test file in this directory includes the standard Apache 2.0 header.
  2. Extra blank line in BUILD.bazel after the new wd_test entry (double blank line vs single blank line convention used by every other entry).

Comment thread src/workerd/api/tests/response-uaf-test.js
Comment thread src/workerd/api/tests/BUILD.bazel
@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk bot commented Apr 14, 2026

Review posted successfully on PR #6579. Here's a summary of my findings:

Two issues found:

  1. Missing copyright header on response-uaf-test.js — all other test files in the directory include the standard Apache 2.0 header. Posted a suggestion with the header added.

  2. Extra blank line in BUILD.bazel after the new wd_test entry — the rest of the file uses single blank lines between entries.

Both have concrete suggestion comments attached. The C++ comment update in http.c++ accurately describes the UAF fix, and the test logic is well-structured for reproducing the original crash scenario.

github run

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Apr 14, 2026

Merging this PR will improve performance by 39.29%

⚡ 1 improved benchmark
✅ 71 untouched benchmarks
⏩ 129 skipped benchmarks1

Performance Changes

Benchmark BASE HEAD Efficiency
simpleStringBody[Response] 27.2 µs 19.5 µs +39.29%

Comparing erikcorry/body-uaf-regression-test (5d7fe84) with main (e7cd397)

Open in CodSpeed

Footnotes

  1. 129 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@jasnell jasnell merged commit 5d2c819 into main Apr 15, 2026
24 checks passed
@jasnell jasnell deleted the erikcorry/body-uaf-regression-test branch April 15, 2026 08:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants