Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions src/brpc/memcache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -503,8 +503,12 @@ bool MemcacheResponse::PopStore(uint8_t command, uint64_t* cas_value) {
- (int)header.key_length;
if (header.status != (uint16_t)STATUS_SUCCESS) {
_buf.pop_front(sizeof(header) + header.extras_length + header.key_length);
_err.clear();
_buf.cutn(&_err, value_size);
if (value_size < 0) {
butil::string_printf(&_err, "value_size=%d is negative", value_size);
} else {
_err.clear();
_buf.cutn(&_err, value_size);
}
return false;
}
LOG_IF(ERROR, value_size != 0) << "STORE response must not have value, actually="
Expand Down
27 changes: 27 additions & 0 deletions test/brpc_memcache_unittest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,33 @@ 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"));
}

static pthread_once_t download_memcached_once = PTHREAD_ONCE_INIT;
static pid_t g_mc_pid = -1;

Expand Down
Loading