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 f818ebc0 Guard negative value_size in memcache PopStore (#3392)
f818ebc0 is described below
commit f818ebc05bd8f1b9a260faff2623aad615d511a4
Author: UB <[email protected]>
AuthorDate: Wed Jul 22 08:45:03 2026 +0530
Guard negative value_size in memcache PopStore (#3392)
* guard negative value_size in memcache PopStore
Signed-off-by: ubeddulla khan <[email protected]>
* memcache: drop only the declared message when value_size is negative
Move the value_size < 0 check ahead of the pop_front so a malformed
reply no longer pops sizeof(header) + extras_length + key_length past
the declared message boundary. Discard exactly
sizeof(header) + total_body_length instead, keeping the following
pipelined responses aligned.
Signed-off-by: ubeddulla khan <[email protected]>
---------
Signed-off-by: ubeddulla khan <[email protected]>
---
src/brpc/memcache.cpp | 7 +++++++
test/brpc_memcache_unittest.cpp | 31 +++++++++++++++++++++++++++++++
2 files changed, 38 insertions(+)
diff --git a/src/brpc/memcache.cpp b/src/brpc/memcache.cpp
index c7d6f836..8d01e7bf 100644
--- a/src/brpc/memcache.cpp
+++ b/src/brpc/memcache.cpp
@@ -501,6 +501,13 @@ bool MemcacheResponse::PopStore(uint8_t command, uint64_t*
cas_value) {
LOG_IF(ERROR, header.key_length != 0) << "STORE response must not have
key";
int value_size = (int)header.total_body_length - (int)header.extras_length
- (int)header.key_length;
+ if (value_size < 0) {
+ // extras_length + key_length overrun the declared body. Drop exactly
the
+ // declared message so that following pipelined responses stay aligned.
+ _buf.pop_front(sizeof(header) + header.total_body_length);
+ butil::string_printf(&_err, "value_size=%d is negative", value_size);
+ return false;
+ }
if (header.status != (uint16_t)STATUS_SUCCESS) {
_buf.pop_front(sizeof(header) + header.extras_length +
header.key_length);
_err.clear();
diff --git a/test/brpc_memcache_unittest.cpp b/test/brpc_memcache_unittest.cpp
index b1da9584..3b7e676b 100644
--- a/test/brpc_memcache_unittest.cpp
+++ b/test/brpc_memcache_unittest.cpp
@@ -62,6 +62,37 @@ TEST(MemcacheParserTest,
RejectOversizedResponseBeforeBufferingBody) {
}
+TEST(MemcacheParserTest, PopStoreRejectsNegativeValueSize) {
+ // A STORE error response whose extras_length + key_length exceeds
+ // total_body_length makes value_size negative. PopStore used to pass that
+ // straight to cutn(&_err, value_size); the negative value became a huge
+ // size_t and drained the rest of the (pipelined) buffer into the error
+ // string. The sibling parsers PopGet/PopCounter/PopVersion already guard
+ // value_size < 0; PopStore must do the same.
+ brpc::policy::MemcacheResponseHeader header = {};
+ header.magic = brpc::policy::MC_MAGIC_RESPONSE;
+ header.command = brpc::policy::MC_BINARY_SET;
+ header.status = 1; // non-zero: error response
+ header.extras_length = 1; // claims extras...
+ header.key_length = 0;
+ header.total_body_length = 0; // ...but body carries none ->
value_size = -1
+
+ brpc::MemcacheResponse response;
+ response.raw_buffer().append(&header, sizeof(header));
+ // Bytes of a following pipelined response that must not leak into _err.
+ const char next_response[] = "SECRET-NEXT-RESPONSE";
+ response.raw_buffer().append(next_response, sizeof(next_response) - 1);
+
+ uint64_t cas_value = 0;
+ ASSERT_FALSE(response.PopSet(&cas_value));
+ ASSERT_EQ("value_size=-1 is negative", response.LastError());
+ ASSERT_EQ(std::string::npos, response.LastError().find("RESPONSE"));
+ // Only the declared message (header + total_body_length) is dropped, so
the
+ // following pipelined response is still intact at the head of the buffer.
+ ASSERT_EQ(sizeof(next_response) - 1, response.raw_buffer().size());
+ ASSERT_EQ(next_response, response.raw_buffer().to_string());
+}
+
static pthread_once_t download_memcached_once = PTHREAD_ONCE_INIT;
static pid_t g_mc_pid = -1;
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
