From 068af367eea9bc63878eae19f2a4d2ef76daefcb Mon Sep 17 00:00:00 2001 From: dxbjavid Date: Tue, 9 Jun 2026 16:35:58 +0530 Subject: [PATCH] avoid integer overflow in container size precheck --- .../c_glib/protocol/thrift_binary_protocol.c | 10 ++-- .../c_glib/protocol/thrift_compact_protocol.c | 10 ++-- lib/cpp/src/thrift/protocol/TBinaryProtocol.h | 6 +-- .../src/thrift/protocol/TCompactProtocol.h | 6 +-- lib/cpp/src/thrift/protocol/TJSONProtocol.h | 6 +-- lib/cpp/src/thrift/protocol/TProtocol.h | 6 +-- lib/cpp/test/ThrifttReadCheckTests.cpp | 48 +++++++++++++++++++ 7 files changed, 70 insertions(+), 22 deletions(-) diff --git a/lib/c_glib/src/thrift/c_glib/protocol/thrift_binary_protocol.c b/lib/c_glib/src/thrift/c_glib/protocol/thrift_binary_protocol.c index 0df2fb0c326..464ffb443bb 100644 --- a/lib/c_glib/src/thrift/c_glib/protocol/thrift_binary_protocol.c +++ b/lib/c_glib/src/thrift/c_glib/protocol/thrift_binary_protocol.c @@ -577,9 +577,9 @@ thrift_binary_protocol_read_map_begin (ThriftProtocol *protocol, return -1; } - if(!ttc->checkReadBytesAvailable (THRIFT_TRANSPORT(tp->transport), - sizei * thrift_binary_protocol_get_min_serialized_size(protocol, k, error) + - sizei * thrift_binary_protocol_get_min_serialized_size(protocol, v, error), + if(!ttc->checkReadBytesAvailable (THRIFT_TRANSPORT(tp->transport), + (glong) sizei * thrift_binary_protocol_get_min_serialized_size(protocol, k, error) + + (glong) sizei * thrift_binary_protocol_get_min_serialized_size(protocol, v, error), error)) { return -1; @@ -642,8 +642,8 @@ thrift_binary_protocol_read_list_begin (ThriftProtocol *protocol, return -1; } - if(!ttc->checkReadBytesAvailable (THRIFT_TRANSPORT(tp->transport), - (sizei * thrift_binary_protocol_get_min_serialized_size(protocol, e, error)), + if(!ttc->checkReadBytesAvailable (THRIFT_TRANSPORT(tp->transport), + ((glong) sizei * thrift_binary_protocol_get_min_serialized_size(protocol, e, error)), error)) { return -1; diff --git a/lib/c_glib/src/thrift/c_glib/protocol/thrift_compact_protocol.c b/lib/c_glib/src/thrift/c_glib/protocol/thrift_compact_protocol.c index 73be027c5dd..5d980cb5677 100644 --- a/lib/c_glib/src/thrift/c_glib/protocol/thrift_compact_protocol.c +++ b/lib/c_glib/src/thrift/c_glib/protocol/thrift_compact_protocol.c @@ -1108,9 +1108,9 @@ thrift_compact_protocol_read_map_begin (ThriftProtocol *protocol, return -1; } - if(!ttc->checkReadBytesAvailable (THRIFT_TRANSPORT (tp->transport), - msize * thrift_protocol_get_min_serialized_size (protocol, *key_type, error) + - msize * thrift_protocol_get_min_serialized_size (protocol, *value_type, error), + if(!ttc->checkReadBytesAvailable (THRIFT_TRANSPORT (tp->transport), + (glong) msize * thrift_protocol_get_min_serialized_size (protocol, *key_type, error) + + (glong) msize * thrift_protocol_get_min_serialized_size (protocol, *value_type, error), error)) { return -1; @@ -1183,8 +1183,8 @@ thrift_compact_protocol_read_list_begin (ThriftProtocol *protocol, *element_type = ret; *size = (guint32) lsize; - if(!ttc->checkReadBytesAvailable (THRIFT_TRANSPORT (tp->transport), - (lsize * thrift_protocol_get_min_serialized_size (protocol, *element_type, error)), + if(!ttc->checkReadBytesAvailable (THRIFT_TRANSPORT (tp->transport), + ((glong) lsize * thrift_protocol_get_min_serialized_size (protocol, *element_type, error)), error)) { return -1; diff --git a/lib/cpp/src/thrift/protocol/TBinaryProtocol.h b/lib/cpp/src/thrift/protocol/TBinaryProtocol.h index cba6e69485d..cc0f34a9096 100644 --- a/lib/cpp/src/thrift/protocol/TBinaryProtocol.h +++ b/lib/cpp/src/thrift/protocol/TBinaryProtocol.h @@ -174,18 +174,18 @@ class TBinaryProtocolT : public TVirtualProtocolcheckReadBytesAvailable(set.size_ * getMinSerializedSize(set.elemType_)); + trans_->checkReadBytesAvailable(static_cast(set.size_) * getMinSerializedSize(set.elemType_)); } void checkReadBytesAvailable(TList& list) override { - trans_->checkReadBytesAvailable(list.size_ * getMinSerializedSize(list.elemType_)); + trans_->checkReadBytesAvailable(static_cast(list.size_) * getMinSerializedSize(list.elemType_)); } void checkReadBytesAvailable(TMap& map) override { int elmSize = getMinSerializedSize(map.keyType_) + getMinSerializedSize(map.valueType_); - trans_->checkReadBytesAvailable(map.size_ * elmSize); + trans_->checkReadBytesAvailable(static_cast(map.size_) * elmSize); } protected: diff --git a/lib/cpp/src/thrift/protocol/TCompactProtocol.h b/lib/cpp/src/thrift/protocol/TCompactProtocol.h index c7d81eea6e5..16b6927e1bd 100644 --- a/lib/cpp/src/thrift/protocol/TCompactProtocol.h +++ b/lib/cpp/src/thrift/protocol/TCompactProtocol.h @@ -146,18 +146,18 @@ class TCompactProtocolT : public TVirtualProtocol void checkReadBytesAvailable(TSet& set) override { - trans_->checkReadBytesAvailable(set.size_ * getMinSerializedSize(set.elemType_)); + trans_->checkReadBytesAvailable(static_cast(set.size_) * getMinSerializedSize(set.elemType_)); } void checkReadBytesAvailable(TList& list) override { - trans_->checkReadBytesAvailable(list.size_ * getMinSerializedSize(list.elemType_)); + trans_->checkReadBytesAvailable(static_cast(list.size_) * getMinSerializedSize(list.elemType_)); } void checkReadBytesAvailable(TMap& map) override { int elmSize = getMinSerializedSize(map.keyType_) + getMinSerializedSize(map.valueType_); - trans_->checkReadBytesAvailable(map.size_ * elmSize); + trans_->checkReadBytesAvailable(static_cast(map.size_) * elmSize); } /** diff --git a/lib/cpp/src/thrift/protocol/TJSONProtocol.h b/lib/cpp/src/thrift/protocol/TJSONProtocol.h index 09eb6eab1bf..766bcf7f188 100644 --- a/lib/cpp/src/thrift/protocol/TJSONProtocol.h +++ b/lib/cpp/src/thrift/protocol/TJSONProtocol.h @@ -253,18 +253,18 @@ class TJSONProtocol : public TVirtualProtocol { void checkReadBytesAvailable(TSet& set) override { - trans_->checkReadBytesAvailable(set.size_ * getMinSerializedSize(set.elemType_)); + trans_->checkReadBytesAvailable(static_cast(set.size_) * getMinSerializedSize(set.elemType_)); } void checkReadBytesAvailable(TList& list) override { - trans_->checkReadBytesAvailable(list.size_ * getMinSerializedSize(list.elemType_)); + trans_->checkReadBytesAvailable(static_cast(list.size_) * getMinSerializedSize(list.elemType_)); } void checkReadBytesAvailable(TMap& map) override { int elmSize = getMinSerializedSize(map.keyType_) + getMinSerializedSize(map.valueType_); - trans_->checkReadBytesAvailable(map.size_ * elmSize); + trans_->checkReadBytesAvailable(static_cast(map.size_) * elmSize); } class LookaheadReader { diff --git a/lib/cpp/src/thrift/protocol/TProtocol.h b/lib/cpp/src/thrift/protocol/TProtocol.h index 565a8ecba85..c42ccf308e1 100644 --- a/lib/cpp/src/thrift/protocol/TProtocol.h +++ b/lib/cpp/src/thrift/protocol/TProtocol.h @@ -604,18 +604,18 @@ class TProtocol { virtual void checkReadBytesAvailable(TSet& set) { - ptrans_->checkReadBytesAvailable(set.size_ * getMinSerializedSize(set.elemType_)); + ptrans_->checkReadBytesAvailable(static_cast(set.size_) * getMinSerializedSize(set.elemType_)); } virtual void checkReadBytesAvailable(TList& list) { - ptrans_->checkReadBytesAvailable(list.size_ * getMinSerializedSize(list.elemType_)); + ptrans_->checkReadBytesAvailable(static_cast(list.size_) * getMinSerializedSize(list.elemType_)); } virtual void checkReadBytesAvailable(TMap& map) { int elmSize = getMinSerializedSize(map.keyType_) + getMinSerializedSize(map.valueType_); - ptrans_->checkReadBytesAvailable(map.size_ * elmSize); + ptrans_->checkReadBytesAvailable(static_cast(map.size_) * elmSize); } std::shared_ptr ptrans_; diff --git a/lib/cpp/test/ThrifttReadCheckTests.cpp b/lib/cpp/test/ThrifttReadCheckTests.cpp index 09dffe228d3..d62f66330ad 100644 --- a/lib/cpp/test/ThrifttReadCheckTests.cpp +++ b/lib/cpp/test/ThrifttReadCheckTests.cpp @@ -242,6 +242,54 @@ BOOST_AUTO_TEST_CASE(test_tthriftcompactprotocol_read_check_pass) { BOOST_CHECK_NO_THROW(protocol->readString(eleven)); } +BOOST_AUTO_TEST_CASE(test_tthriftbinaryprotocol_container_size_overflow) { + std::shared_ptr config (new TConfiguration(1024)); + std::shared_ptr transport(new TMemoryBuffer(config)); + std::shared_ptr protocol(new TBinaryProtocol(transport)); + + uint32_t val = 0; + TType elemType = apache::thrift::protocol::T_STOP; + // 0x40000000 elements of min size 4 require 4 GiB; the product wraps to 0 in + // 32-bit math and used to slip past the MaxMessageSize check. + TList list(T_I32, 0x40000000); + protocol->writeListBegin(list.elemType_, list.size_); + protocol->writeListEnd(); + BOOST_CHECK_THROW(protocol->readListBegin(elemType, val), TTransportException); + protocol->readListEnd(); +} + +BOOST_AUTO_TEST_CASE(test_tthriftcompactprotocol_container_size_overflow) { + std::shared_ptr config (new TConfiguration(1024)); + std::shared_ptr transport(new TMemoryBuffer(config)); + std::shared_ptr protocol(new TCompactProtocol(transport)); + + uint32_t val = 0; + TType elemType = apache::thrift::protocol::T_STOP; + // 0x10000000 elements of min size 16 (UUID) require 4 GiB; the product wraps + // to 0 in 32-bit math and used to slip past the MaxMessageSize check. + TList list(T_UUID, 0x10000000); + protocol->writeListBegin(list.elemType_, list.size_); + protocol->writeListEnd(); + BOOST_CHECK_THROW(protocol->readListBegin(elemType, val), TTransportException); + protocol->readListEnd(); +} + +BOOST_AUTO_TEST_CASE(test_tthriftjsonprotocol_container_size_overflow) { + std::shared_ptr config (new TConfiguration(1024)); + std::shared_ptr transport(new TMemoryBuffer(config)); + std::shared_ptr protocol(new TJSONProtocol(transport)); + + uint32_t val = 0; + TType elemType = apache::thrift::protocol::T_STOP; + // 0x10000000 elements of min size 16 (UUID) require 4 GiB; the product wraps + // to 0 in 32-bit math and used to slip past the MaxMessageSize check. + TList list(T_UUID, 0x10000000); + protocol->writeListBegin(list.elemType_, list.size_); + protocol->writeListEnd(); + BOOST_CHECK_THROW(protocol->readListBegin(elemType, val), TTransportException); + protocol->readListEnd(); +} + BOOST_AUTO_TEST_CASE(test_tthriftjsonprotocol_read_check_exception) { std::shared_ptr config (new TConfiguration(MAX_MESSAGE_SIZE)); std::shared_ptr transport(new TMemoryBuffer(config));