JakeChampion / JakeChampion/trafficserver
[audit][http2/3] QPACK static-table lookup indexes the array with no bounds check (OOB read)
Nobody has claimed this yet.
- 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
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- 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