Skip to content

Fix use of uninitialized value issue from fuzzing#13140

Open
shukitchan wants to merge 1 commit into
apache:masterfrom
shukitchan:fix/oss-fuzz-4669620266270720-uninit-type-buf
Open

Fix use of uninitialized value issue from fuzzing#13140
shukitchan wants to merge 1 commit into
apache:masterfrom
shukitchan:fix/oss-fuzz-4669620266270720-uninit-type-buf

Conversation

@shukitchan
Copy link
Copy Markdown
Contributor

This pull request refactors the FNV-1a hash implementation to simplify initialization and improve code clarity. The main changes involve moving the FNV initial constants into the class definitions as constexpr static members, initializing member variables directly, and simplifying constructors.

Hash algorithm improvements:

  • Moved FNV initial constants (FNV_INIT_32, FNV_INIT_64) into the respective classes (ATSHash32FNV1a, ATSHash64FNV1a) as static constexpr members and used them to initialize hval directly in the class definition (include/tscore/HashFNV.h). [1] [2]
  • Updated the clear() methods to use the new fnv_init static member instead of external constants (src/tscore/HashFNV.cc). [1] [2]
  • Simplified constructors for ATSHash32FNV1a and ATSHash64FNV1a by using default constructors instead of explicitly calling clear() (src/tscore/HashFNV.cc).

Code style and safety:

  • Zero-initialized the type_buf buffer in Http3FrameFactory::create for improved safety (src/proxy/http3/Http3Frame.cc).

This fixes the reported fuzzing issues
https://oss-fuzz.com/testcase-detail/4669620266270720
https://oss-fuzz.com/testcase-detail/4793610426449920

@shukitchan shukitchan marked this pull request as draft May 6, 2026 21:51
@shukitchan shukitchan self-assigned this May 6, 2026
@shukitchan shukitchan added the Fuzz label May 6, 2026
@shukitchan shukitchan added this to the 11.0.0 milestone May 6, 2026
@ezelkow1
Copy link
Copy Markdown
Member

ezelkow1 commented May 7, 2026

[approve ci freebsd clang-analyzer autest]

@shukitchan shukitchan marked this pull request as ready for review May 7, 2026 16:27
@shukitchan shukitchan requested a review from Copilot May 11, 2026 16:26
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Refactors the FNV-1a hash implementations to ensure hash state is initialized deterministically (addressing fuzzing-reported uninitialized reads) and hardens HTTP/3 frame parsing against uninitialized stack data.

Changes:

  • Move FNV init constants into the hash classes as static constexpr members and use in-class member initialization.
  • Simplify FNV-1a constructors to default constructors; update clear() to use class init constants.
  • Zero-initialize the HTTP/3 frame type buffer before copying from the reader.

Reviewed changes

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

File Description
src/tscore/HashFNV.cc Removes file-scope init constants, defaults constructors, and updates clear() to use class init constants.
include/tscore/HashFNV.h Adds static constexpr fnv_init and in-class initialization of hval for 32/64-bit FNV-1a.
src/proxy/http3/Http3Frame.cc Zero-initializes type_buf to avoid uninitialized bytes influencing frame type parsing.

Comment on lines 509 to 510
reader.memcpy(type_buf, sizeof(type_buf));
Http3FrameType type = Http3Frame::type(type_buf, sizeof(type_buf));
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I will need some help to see if this comment makes sense or not.
Otherwise I think we can ignore it for now.

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.

I think safe to ignore

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.

4 participants