Skip to content

Clamp HTTP3 frame type buf size to reader bytes - #13242

Merged
JosiahWI merged 4 commits into
apache:masterfrom
JosiahWI:refactor/harden-http3-type-parsing
Jun 8, 2026
Merged

JosiahWI merged 4 commits into
apache:masterfrom
JosiahWI:refactor/harden-http3-type-parsing

Conversation

@JosiahWI

@JosiahWI JosiahWI commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

The length of the source buffer for HTTP3 type parsing was always taken
to be the maximum length of the type field. This seemed to work without
UB when I tested it through Http3FrameDispatcher, but Kit Chan pointed
out that it is risky (#11720).

This patch refactors the type parsing to guarantee that the number of
bytes passed to the parser will not be greater than the number of
initialized bytes in the buffer.

Fixes #11720

The length of the source buffer for HTTP3 type parsing was always taken
to be the maximum length of the type field. This seemed to work without
UB when I tested it through `Http3FrameDispatcher`, but Kit Chan pointed
out that it is risky (apache#11720).

This patch refactors the type parsing to guarantee that the number of
bytes passed to the parser will not be greater than the number of
initialized bytes in the buffer.
@JosiahWI JosiahWI added this to the 10.1.3 milestone Jun 5, 2026
@JosiahWI
JosiahWI requested a review from shukitchan June 5, 2026 12:22
@JosiahWI JosiahWI self-assigned this Jun 5, 2026
Copilot AI review requested due to automatic review settings June 5, 2026 12:22

Copilot AI 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.

Pull request overview

This PR addresses a fuzzing-reported risk in HTTP/3 frame type parsing by ensuring the type parser never receives a buffer length larger than the number of bytes actually copied from the IOBufferReader, avoiding potential reads of uninitialized stack data.

Changes:

  • Clamp the number of bytes used for HTTP/3 frame type parsing to min(reader.read_avail(), FRAME_TYPE_MAX_BYTES).
  • Apply the clamping consistently in both Http3FrameFactory::create() and Http3FrameFactory::fast_create().
  • Add <algorithm> include to support std::min.

Comment thread src/proxy/http3/Http3Frame.cc
Comment thread src/proxy/http3/Http3Frame.cc
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 5, 2026 12:42

Copilot AI 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.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment thread src/proxy/http3/Http3Frame.cc
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@zwoop

zwoop commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

[approve ci]

@maskit

maskit commented Jun 5, 2026

Copy link
Copy Markdown
Member

I'm not sure if it's worth back porting this fix for 10.x. I'd leave it on master for 11.0.

@JosiahWI JosiahWI modified the milestones: 10.1.3, 11.0.0 Jun 6, 2026
@JosiahWI
JosiahWI merged commit 3189423 into apache:master Jun 8, 2026
15 checks passed
@JosiahWI
JosiahWI deleted the refactor/harden-http3-type-parsing branch June 8, 2026 10:49
cmcfarlen pushed a commit to cmcfarlen/trafficserver that referenced this pull request Jul 29, 2026
* Clamp HTTP3 frame type buf size to reader bytes

The length of the source buffer for HTTP3 type parsing was always taken
to be the maximum length of the type field. This seemed to work without
UB when I tested it through `Http3FrameDispatcher`, but Kit Chan pointed
out that it is risky (apache#11720).

This patch refactors the type parsing to guarantee that the number of
bytes passed to the parser will not be greater than the number of
initialized bytes in the buffer.

* Fix incorrect identifier name

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* Fix incorrect identifier

* Fix incorrect identifier name

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
cmcfarlen pushed a commit to cmcfarlen/trafficserver that referenced this pull request Jul 29, 2026
* Clamp HTTP3 frame type buf size to reader bytes

The length of the source buffer for HTTP3 type parsing was always taken
to be the maximum length of the type field. This seemed to work without
UB when I tested it through `Http3FrameDispatcher`, but Kit Chan pointed
out that it is risky (apache#11720).

This patch refactors the type parsing to guarantee that the number of
bytes passed to the parser will not be greater than the number of
initialized bytes in the buffer.

* Fix incorrect identifier name

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* Fix incorrect identifier

* Fix incorrect identifier name

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
(cherry picked from commit 3189423)
@cmcfarlen cmcfarlen moved this to Picked v10.2.0 in ATS v10.2.x Jul 29, 2026
@cmcfarlen cmcfarlen modified the milestones: 11.0.0, 10.2.0 Jul 29, 2026
@github-project-automation github-project-automation Bot moved this from Picked v10.2.0 to For v10.2.0 in ATS v10.2.x Jul 29, 2026
@cmcfarlen cmcfarlen moved this from For v10.2.0 to Picked v10.2.0 in ATS v10.2.x Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Picked v10.2.0

Development

Successfully merging this pull request may close these issues.

use-of-uninitialized value problem found by fuzzing: Http3Frame

5 participants