Skip to content

fix(volo-build): gate multiservice binary codec behind unsafe-codec - #671

Open
shenyj3 wants to merge 1 commit into
mainfrom
fix/multiservice-unsafe-codec-gating
Open

shenyj3 wants to merge 1 commit into
mainfrom
fix/multiservice-unsafe-codec-gating

Conversation

@shenyj3

@shenyj3 shenyj3 commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • The raw-bytes service impl generated by volo-build for Router-based multi-service servers unconditionally decoded requests with TBinaryUnsafeInputProtocol, whose read primitives use get_unchecked without bounds checks.
  • A truncated/malformed thrift binary frame from any unauthenticated peer could trigger an out-of-bounds heap read (debug: abort; release: silent OOB), bypassing the unsafe-codec feature that guards the single-service codec in codec/default/thrift.rs.
  • Move encode/decode into new volo_thrift::codec::default::multiservice helpers that use the safe TBinaryProtocol by default and the unsafe variants only behind unsafe-codec; the compact path is unchanged.

Test plan

  • cargo check -p volo-thrift with default features and --features unsafe-codec
  • cargo check --workspace --offline
  • New regression test thrift_multi_service_malformed: truncated framed binary frame against a Router server; the default safe codec survives (debug + release) and a subsequent normal request succeeds
  • Same binary built with --features volo-thrift/unsafe-codec aborts at pilota binary_unsafe.rs precondition, confirming the test hits the fix point
  • Existing thrift_multi_service tests pass; fmt + clippy clean

@codecov

codecov Bot commented Sep 14, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.33333% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.59%. Comparing base (90aa52b) to head (cdbd4bc).

Files with missing lines Patch % Lines
volo-thrift/src/codec/default/multiservice.rs 83.33% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #671      +/-   ##
==========================================
+ Coverage   56.14%   61.59%   +5.45%     
==========================================
  Files         166      167       +1     
  Lines       22517    22606      +89     
==========================================
+ Hits        12642    13924    +1282     
+ Misses       9875     8682    -1193     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@shenyj3
shenyj3 force-pushed the fix/multiservice-unsafe-codec-gating branch from 639321d to 9944cda Compare September 15, 2026 04:03
The raw-bytes service impl generated by volo-build for Router-based
multi-service servers unconditionally decoded requests with
TBinaryUnsafeInputProtocol, whose read primitives use get_unchecked
without bounds checks. A truncated binary frame from any peer could
trigger an out-of-bounds heap read (debug: abort; release: silent OOB),
bypassing the unsafe-codec feature that guards the single-service path.

Move encode/decode into volo-thrift helpers that select the safe
TBinaryProtocol by default and the unsafe variants only when the
unsafe-codec feature is enabled, matching the hand-written codec. Add a
regression test that sends a truncated frame and verifies the server
survives and keeps serving.
@shenyj3
shenyj3 force-pushed the fix/multiservice-unsafe-codec-gating branch from 9944cda to cdbd4bc Compare September 15, 2026 11:53
@shenyj3 shenyj3 changed the title fix(volo-thrift): gate multiservice binary codec behind unsafe-codec fix(volo-build): gate multiservice binary codec behind unsafe-codec Sep 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant