Skip to content

fix(codec): eliminate uninitialized memory in encode_item on unwind - #2847

Open
nathanielford wants to merge 1 commit into
grpc:masterfrom
nathanielford:fix/issue-2720-encode-item-exception-safety
Open

fix(codec): eliminate uninitialized memory in encode_item on unwind#2847
nathanielford wants to merge 1 commit into
grpc:masterfrom
nathanielford:fix/issue-2720-encode-item-exception-safety

Conversation

@nathanielford

@nathanielford nathanielford commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Motivation

Fixes #2720.

In tonic/src/codec/encode.rs, encode_item previously advanced the buffer's logical length by 5 bytes using unsafe { buf.advance_mut(HEADER_SIZE); } before invoking the user-provided Encoder::encode. If the encoder panicked or unwound, buf was left with 5 uninitialized bytes exposed to callers catching unwinds.

Solution

  • Replace unsafe { buf.advance_mut(HEADER_SIZE); } with safe zero-filled initialization via buf.put_slice(&[0u8; HEADER_SIZE]), eliminating the unsafe block entirely.
  • Writing 5 zero bytes into the cache line that is immediately overwritten by finish_encoding incurs minimal performance impact (a single store instruction).
  • The alternative RAII drop guard approach was deliberately avoided because it preserves unsafe code and could resurface the soundness vulnerability if std::mem::forget (or a similar leak) were ever introduced.
  • Add unit test encode_item_exception_safety_on_panic verifying that unwinding panics leave only initialized zero bytes in the buffer.

Test Plan

  • Ran cargo test -p tonic codec::encode.
  • Ran cargo fmt --all --check.
  • Ran cargo clippy.

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.

Tonic's encode_item function is not exception safe, leading to use of uninitialized memory

2 participants