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 841aab13 Avoid undefined shift in hpack DecodeInteger on crafted
integer (#3379)
841aab13 is described below
commit 841aab132fea97ab95f796bc30249632ae57d4ee
Author: UB <[email protected]>
AuthorDate: Thu Jul 16 14:45:40 2026 +0530
Avoid undefined shift in hpack DecodeInteger on crafted integer (#3379)
* avoid undefined shift in hpack DecodeInteger on crafted integer
* hpack: rate-limit and specialize over-long integer log message
Signed-off-by: ubeddulla khan <[email protected]>
---------
Signed-off-by: ubeddulla khan <[email protected]>
---
src/brpc/details/hpack.cpp | 11 +++++++++++
test/brpc_hpack_unittest.cpp | 21 +++++++++++++++++++++
2 files changed, 32 insertions(+)
diff --git a/src/brpc/details/hpack.cpp b/src/brpc/details/hpack.cpp
index e4e7890e..e627e81d 100644
--- a/src/brpc/details/hpack.cpp
+++ b/src/brpc/details/hpack.cpp
@@ -547,6 +547,17 @@ inline ssize_t DecodeInteger(butil::IOBufBytesIterator&
iter,
if (!iter) {
return 0;
}
+ // A well-formed integer below MAX_HPACK_INTEGER fits in a few
+ // continuation octets. A run of 0x80 octets (continuation bit set,
+ // payload bits zero) leaves tmp unchanged, so the `tmp <
+ // MAX_HPACK_INTEGER` guard below never trips while m keeps growing;
+ // once m reaches 64 the `<< m` shift is undefined behavior. Refuse the
+ // over-long encoding before that happens.
+ if (m >= 32) {
+ LOG_EVERY_SECOND(ERROR) << "Over-long HPACK integer encoding, "
+ "continuation octets would overflow the
shift";
+ return -1;
+ }
cur_byte = *iter;
in_bytes++;
tmp += static_cast<uint64_t>(cur_byte & 0x7f) << m;
diff --git a/test/brpc_hpack_unittest.cpp b/test/brpc_hpack_unittest.cpp
index d6a0bd02..48d3ae52 100644
--- a/test/brpc_hpack_unittest.cpp
+++ b/test/brpc_hpack_unittest.cpp
@@ -96,6 +96,27 @@ TEST_F(HPackTest, dynamic_table_size_update_before_header) {
ASSERT_EQ("GET", h.value);
}
+TEST_F(HPackTest, integer_with_overlong_continuation) {
+ brpc::HPacker p;
+ ASSERT_EQ(0, p.Init(4096));
+
+ // Indexed header field whose index is encoded with a 7-bit prefix of all
+ // ones (0xFF) followed by a long run of 0x80 continuation octets. Each
+ // 0x80 carries a zero payload, so the decoded value never grows past its
+ // MAX_HPACK_INTEGER guard, but the per-octet shift amount does. Decode
must
+ // reject this as malformed instead of shifting by 64+ bits.
+ butil::IOBuf buf;
+ uint8_t encoded[1 + 20];
+ encoded[0] = 0xFF;
+ for (size_t i = 1; i < sizeof(encoded); ++i) {
+ encoded[i] = 0x80;
+ }
+ buf.append(encoded, sizeof(encoded));
+
+ brpc::HPacker::Header h;
+ ASSERT_LT(p.Decode(&buf, &h), 0);
+}
+
// Copied test cases from example of rfc7541
TEST_F(HPackTest, header_with_indexing) {
brpc::HPacker p1;
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
