This is an automated email from the ASF dual-hosted git repository.
chenBright pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/brpc.git
The following commit(s) were added to refs/heads/master by this push:
new f80d6b88d Reject empty HPACK header names (#3694)
f80d6b88d is described below
commit f80d6b88d6c31ae558dacaf30545795bd03fbe19
Author: Weibing Wang <[email protected]>
AuthorDate: Sun Nov 1 13:20:55 2026 +0800
Reject empty HPACK header names (#3694)
Co-authored-by: brpc-oncall <brpc-oncall@localhost>
---
src/brpc/details/hpack.cpp | 9 +++++++++
test/brpc_hpack_unittest.cpp | 23 +++++++++++++++++++++++
2 files changed, 32 insertions(+)
diff --git a/src/brpc/details/hpack.cpp b/src/brpc/details/hpack.cpp
index 76920b957..72eced7fe 100644
--- a/src/brpc/details/hpack.cpp
+++ b/src/brpc/details/hpack.cpp
@@ -775,6 +775,15 @@ inline ssize_t HPacker::DecodeWithKnownPrefix(
LOG(ERROR) << "Fail to decode name";
return -1;
}
+ // RFC 7541 requires a literal header name to be non-empty. A
+ // zero-length name consumes one byte on the wire (the 0x00 length
+ // octet), so name_bytes above is 1 and does not reject it. Letting it
+ // through would trip CHECK(!h.name.empty()) in IndexTable::AddHeader()
+ // when the header is indexed.
+ if (h->name.empty()) {
+ LOG(ERROR) << "Empty header name";
+ return -1;
+ }
tolower(&h->name);
}
ssize_t value_bytes = DecodeString(iter, &h->value);
diff --git a/test/brpc_hpack_unittest.cpp b/test/brpc_hpack_unittest.cpp
index baa485c12..41c14dec5 100644
--- a/test/brpc_hpack_unittest.cpp
+++ b/test/brpc_hpack_unittest.cpp
@@ -682,3 +682,26 @@ TEST_F(HPackTest, zero_size_dynamic_table) {
brpc::HPacker p;
ASSERT_EQ(0, p.Init(0));
}
+
+TEST_F(HPackTest, empty_header_name) {
+ // RFC 7541 requires a literal header name to be non-empty. A zero-length
+ // name consumes one byte on the wire (the 0x00 length octet), so the
+ // "name_bytes <= 0" guard in DecodeWithKnownPrefix() did not reject it.
+ // The empty name then reached IndexTable::AddHeader(), whose
+ // CHECK(!h.name.empty()) aborted the process, so a single small HEADERS
+ // frame from an unauthenticated peer could kill an HTTP/2 server.
+ const uint8_t literals[][3] = {
+ {0x40, 0x00, 0x00}, // with incremental indexing
+ {0x00, 0x00, 0x00}, // without indexing
+ {0x10, 0x00, 0x00}, // never indexed
+ {0x40, 0x80, 0x00}, // with incremental indexing, Huffman-encoded name
+ };
+ for (size_t i = 0; i < sizeof(literals) / sizeof(literals[0]); ++i) {
+ brpc::HPacker p;
+ ASSERT_EQ(0, p.Init(4096));
+ butil::IOBuf buf;
+ buf.append(literals[i], sizeof(literals[i]));
+ brpc::HPacker::Header h;
+ ASSERT_EQ(-1, p.Decode(&buf, &h)) << "case " << i;
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
