JakeChampion / JakeChampion/trafficserver

[audit][http2/3] QPACK static-table lookup indexes the array with no bounds check (OOB read)

Open
#63 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

area:http2-3 audit severity:critical verified
Dominant language
C++
Stars
0
Forks
0
Avg merge
8h 2m
Merged PRs (30d)
21

Description

Severity: critical · Category: memory-bug · ✓ adversarially verified
Location: src/proxy/http3/QPACK.cc:1222

Filed after the initial audit batch (the single critical finding was inadvertently omitted from the high/medium sweep). A fix is already staged on branch claude/codebase-audit-review-9nw7vz.

What's wrong

QPACK::StaticTable::lookup(uint16_t index, ...) dereferences STATIC_HEADER_FIELDS[index] with no range check, but STATIC_HEADER_FIELDS has only 99 entries and index is attacker-controlled (a QPACK integer up to 0xFFFF). It is reached directly from the header-block decode path for any 'Indexed Header Field' / 'Literal Header Field With Name Reference' that sets the static (T) bit, as well as the encoder-stream 'Insert With Name Reference' handler. A single crafted HEADERS field with a static index >= 99 reads past the array; the garbage Header returned supplies a wild name/value pointer and length that are then passed to _attach_header -> field_create/value_set (std::string_view{name, name_len}), causing an out-of-bounds read of attacker-influenced size and near-certain crash or header-memory disclosure. The encoder-stream handler additionally calls StaticTable::lookup(index) unconditionally, ignoring the is_static flag it just parsed, so even dynamic-table name references hit this unchecked static path.

Evidence
1220: QPACK::StaticTable::lookup(uint16_t index, const char **name, ...) {
1222:   const Header &header = STATIC_HEADER_FIELDS[index];   // no check index < 99
...
709:    result = StaticTable::lookup(index, &name, &name_len, &value, &value_len);   // buf[0] & 0x40, static
1156:   StaticTable::lookup(index, &name, &name_len, &dummy, &dummy_len);            // is_static ignored
Suggested fix

Bounds-check index against countof(STATIC_HEADER_FIELDS) in the by-index StaticTable::lookup and return MatchType::NONE (decoding error) when out of range; also honor is_static at the encoder-stream handler by looking up the dynamic table when the static bit is clear, and abort the decode on an invalid index.

Verification

Independently re-traced by a second reviewer instructed to refute it; confirmed. STATIC_HEADER_FIELDS[] has exactly 99 entries (indices 0-98). lookup() indexes STATIC_HEADER_FIELDS[index] with no range check and always returns EXACT, so the callers' if (result.match_type != EXACT) return -1 guards never reject an out-of-range index. The index originates from an xpack-decoded QPACK integer bounded only to 16 bits, directly attacker-reachable via a crafted HEADERS field section over HTTP/3.


Filed from an automated multi-lens codebase audit. Full report: CODEBASE_AUDIT.md / audit-report.html on branch claude/codebase-audit-review-9nw7vz.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in src/proxy/http3/QPACK.cc at the StaticTable::lookup implementation around line 1222 and trace the callers around lines 709 and 1156. Compare the staged fix on branch claude/codebase-audit-review-9nw7vz; done means invalid static indexes are rejected and the encoder-stream path respects the static/dynamic reference flag without out-of-bounds access.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
networking, security
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.