Skip to content

httpv2: the frame guard tests the declared length, not the buffer, so a 4-octet frame can report length=16777215 #799

Description

@JarryShaw

Describe the bug

pcapkit/protocols/application/httpv2.py:223 guards on if schema.length < 9: — the declared length read off the wire, never len(buffer). So a truncated frame whose declared length happens to be ≥ 9 parses, and the parsed object reports a length the capture does not contain. Measured on 0419c1c97, a 4-octet buffer sweeping the declared length:

declared=0         -> ProtocolError
declared=5         -> ProtocolError
declared=8         -> ProtocolError
declared=9         -> PARSED   version=2 length=9
declared=10        -> ProtocolError
declared=15        -> PARSED   version=2 length=15
declared=100       -> ProtocolError
declared=65535     -> PARSED   version=2 length=65535
declared=16777215  -> PARSED   version=2 length=16777215

A 4-octet buffer yields length=16777215. The value is attacker-controlled, and anything downstream that trusts length for framing or offsets is handed a number the capture cannot support.

Note the result is non-monotone — 9 parses, 10 refuses, 15 parses. A guard testing the intended quantity could not produce that shape; it is the signature of an accident rather than a design.

Expected behavior

The sub-9 class should be uniformly refused. Adding a buffer-length condition alongside the declared-length one — so both must hold — makes every truncated frame a ProtocolError.

Why this is worth more than it looks

It is the root cause of the only compromise #789 had to accept. That PR made _guess_version's HTTP/2 arm reachable and had to widen the last arm's suppression to (ProtocolError, struct.error), admitting in a code comment that a genuine httpv2 schema defect on well-formed HTTP/2 bytes would now surface as unknown HTTP version rather than crashing loudly.

That residual exists only because this guard tests the wrong quantity. Make the sub-9 class uniformly ProtocolError and _guess_version can drop the struct.error suppression entirely — so this is a follow-up that shrinks #789's surface rather than merely noting it.

It also became reachable from ordinary extraction at #789's head: on base the guess path died in arm 1, so via UDP/80 a truncated HTTP/2 frame became Raw; now it parses as a confident HTTP/2.

Additional context

Found by the cross-review on #789, which ranked it above the related residual that HTTPv2 constructed directly still raises a bare struct.error under 9 octets — the same defect from a less consequential angle. Both routes measured: HTTPv2 direct and HTTP(..., version=2) parse a 4-octet buffer declaring 15; HTTP() refused it on base and parses it at 11bb7693a.

Also related, and worth checking in the same pass: httpv2.py:292's make() emits _make_http_length(...) + 9, treating Length as the total frame size where RFC 9113 §4.1 says it is the payload size, header excluded. That is tracked separately as the round-trip asymmetry noted on #682's investigation; whoever fixes the guard should confirm the two are consistent afterwards.

Related: #789, #787, #682.

Activity

  1. added
    bugIssues reporting a defect (set by the bug report template; a default, not an assessment)
    fixPull requests that fix a defect (fix: subject prefix)
    on Sep 25, 2026
  2. JarryShaw commented on Sep 25, 2026

    @JarryShaw
    OwnerAuthor

    Labelling blocked so it does not read as unheld work.

    Checkable blocker: #789 merged. This issue's fix lets _guess_version drop the struct.error suppression that #789 introduced — so the two changes touch the same reasoning in pcapkit/protocols/application/http.py, and #789 (11bb7693a, review: good-to-go, awaiting the maintainer) holds that file now. Doing this first would mean writing the guard against a _guess_version that is about to change, then re-deriving it.

    gh pr view 789 -R JarryShaw/PyPCAPKit --json state,mergedAt
    

    Sequence once clear, because the order is the point: fix the guard here so the whole sub-9 class becomes a uniform ProtocolError, then narrow suppress(ProtocolError, struct.error) back to suppress(ProtocolError) on the last arm and delete the residual paragraph #789's comment now carries. That paragraph is an honest admission of a cost this issue removes — so the follow-up should retire the comment, not leave it describing a compromise that no longer exists.

    Note the line numbers above are pinned to 0419c1c97 and httpv2.py is untouched by #789, so :223 and :292 should still be right when this starts — but re-derive rather than trust, since that has bitten three times today.

  3. added
    blockedDeferred pending another issue or decision; see the last comment for what unblocks it
    wipWork in flight - a covering PR is open or an agent is actively on it
    and removed
    blockedDeferred pending another issue or decision; see the last comment for what unblocks it
    on Sep 25, 2026
  4. removed
    wipWork in flight - a covering PR is open or an agent is actively on it
    on Sep 25, 2026
  5. added this to the 1.5 milestone on Oct 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugIssues reporting a defect (set by the bug report template; a default, not an assessment)fixPull requests that fix a defect (fix: subject prefix)

    Projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions