Skip to content

Fix packets with no frames being accepted instead of rejected per RFC 9000 §12.4 #5196

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Jun 28, 2025

Problem

The QuicConnRecvFrames function currently accepts packets with no frames (PayloadLength == 0) without error, which violates RFC 9000 §12.4 that mandates treating such packets as a connection error of type PROTOCOL_VIOLATION.

When a packet has no payload, the frame processing while loop is never entered, allowing the packet to be processed successfully:

uint16_t Offset = 0;
while (Offset < PayloadLength) {  // Never enters when PayloadLength == 0
    // Frame processing logic
}
// Packet is accepted without any frames

Solution

Added validation in QuicConnRecvFrames to check for empty packets immediately after PayloadLength is determined:

//
// RFC 9000 §12.4 - Packets that contain no frames are invalid and must be
// treated as a connection error of type PROTOCOL_VIOLATION.
//
if (PayloadLength == 0) {
    QuicTraceEvent(
        ConnError,
        "[conn][%p] ERROR, %s.",
        Connection,
        "Packet contained no frames");
    QuicConnTransportError(Connection, QUIC_ERROR_PROTOCOL_VIOLATION);
    return FALSE;
}

This ensures RFC compliance by:

  • Detecting packets with no frames before frame processing begins
  • Reporting the error with proper tracing for debugging
  • Triggering a PROTOCOL_VIOLATION connection error as required
  • Following the same error handling pattern used elsewhere in the function

Testing

  • Verified the core library builds successfully with the changes
  • Created proof-of-concept test confirming packets with PayloadLength == 0 are properly rejected with QUIC_ERROR_PROTOCOL_VIOLATION
  • Confirmed packets with frames continue to be processed normally

Fixes #5186.


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

…compliance

Co-authored-by: nibanks <20663557+nibanks@users.noreply.github.com>
@Copilot Copilot AI changed the title [WIP] Packets with No Frames Are Accepted Instead of Rejected Fix packets with no frames being accepted instead of rejected per RFC 9000 §12.4 Jun 28, 2025
@Copilot Copilot AI requested a review from nibanks June 28, 2025 10:50
Copilot finished work on behalf of nibanks June 28, 2025 10:50
@nibanks
Copy link
Collaborator

nibanks commented Jun 28, 2025

How are you testing this? Maybe you can make sure the receive fuzzed can generate zero payload packets.

Co-authored-by: nibanks <20663557+nibanks@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Jun 28, 2025

How are you testing this? Maybe you can make sure the receive fuzzed can generate zero payload packets.

I've enhanced the recvfuzz tool to be able to generate zero payload packets to test the RFC 9000 §12.4 compliance fix. Changes include:

  1. Modified FuzzInitial and FuzzHandshake functions to occasionally generate packets with NumFrames = 0 (1/16 chance)
  2. Updated packet generation logic to allow PayloadLength = 0 when no frames are present
  3. Enabled recvfuzz to build on Linux (was previously Windows-only)
  4. Fixed compilation errors for Linux build

The fuzzer can now generate zero payload packets that will be properly rejected by the RFC compliance fix in commit bb5ca2a.

Copilot finished work on behalf of nibanks June 28, 2025 11:16
Copy link

codecov bot commented Jun 29, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.57%. Comparing base (c69379c) to head (bb5ca2a).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5196      +/-   ##
==========================================
- Coverage   85.53%   84.57%   -0.97%     
==========================================
  Files          59       59              
  Lines       18330    18334       +4     
==========================================
- Hits        15679    15506     -173     
- Misses       2651     2828     +177     

☔ View full report in Codecov by Sentry.
📢 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.

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.

Packets with No Frames Are Accepted Instead of Rejected
2 participants