diff --git a/src/brpc/policy/crc32c_checksum.cpp b/src/brpc/policy/crc32c_checksum.cpp index 8af7fbe2ff..b99de725e2 100644 --- a/src/brpc/policy/crc32c_checksum.cpp +++ b/src/brpc/policy/crc32c_checksum.cpp @@ -17,6 +17,8 @@ #include "brpc/policy/crc32c_checksum.h" +#include + #include "brpc/details/controller_private_accessor.h" #include "brpc/log.h" #include "butil/crc32c.h" @@ -61,12 +63,15 @@ void Crc32cCompute(const ChecksumIn& in) { } bool Crc32cVerify(const ChecksumIn& in) { - uint32_t crc = ComputeCrc32c(in); auto& val = ControllerPrivateAccessor(const_cast(in.cntl)) .checksum_value(); - CHECK_EQ(val.size(), sizeof(crc)); - auto expected = *reinterpret_cast(val.data()); + uint32_t expected; + if (val.size() != sizeof(expected)) { + return false; + } + memcpy(&expected, val.data(), sizeof(expected)); expected = butil::crc32c::Unmask(butil::NetToHost32(expected)); + const uint32_t crc = ComputeCrc32c(in); RPC_VLOG << "Crc32cVerify crc=" << crc << " expected=" << expected; return crc == expected; } diff --git a/test/brpc_checksum_unittest.cpp b/test/brpc_checksum_unittest.cpp index 7811cd6ce0..0bf0cf11fc 100644 --- a/test/brpc_checksum_unittest.cpp +++ b/test/brpc_checksum_unittest.cpp @@ -27,6 +27,7 @@ #include "brpc/details/controller_private_accessor.h" #include "brpc/policy/crc32c_checksum.h" #include "butil/iobuf.h" +#include "butil/memory/scope_guard.h" #include "echo.pb.h" namespace { @@ -50,6 +51,34 @@ TEST_F(ChecksumAttachmentTest, verify_succeeds_when_body_only) { EXPECT_TRUE(brpc::policy::Crc32cVerify(verify_in)); } +TEST_F(ChecksumAttachmentTest, verify_rejects_invalid_checksum_lengths) { + brpc::Controller cntl; + butil::IOBuf body; + body.append("request body"); + brpc::ChecksumIn in{&body, &cntl, nullptr}; + brpc::policy::Crc32cCompute(in); + brpc::ControllerPrivateAccessor accessor(&cntl); + const std::string valid = accessor.checksum_value(); + ASSERT_EQ(sizeof(uint32_t), valid.size()); + + logging::StringSink logs; + { + logging::LogSink* previous = logging::SetLogSink(&logs); + BUTIL_SCOPE_EXIT { logging::SetLogSink(previous); }; + for (size_t size = 0; size <= 8; ++size) { + if (size == valid.size()) { + continue; + } + const std::string invalid(size, '\0'); + accessor.set_checksum_value(invalid.data(), invalid.size()); + EXPECT_FALSE(brpc::policy::Crc32cVerify(in)) << "size=" << size; + } + accessor.set_checksum_value(valid.data(), valid.size()); + EXPECT_TRUE(brpc::policy::Crc32cVerify(in)); + } + EXPECT_EQ(std::string::npos, logs.find("Check failed: val.size()")); +} + TEST_F(ChecksumAttachmentTest, verify_succeeds_when_attachment_included) { brpc::Controller cntl; butil::IOBuf body;