Skip to content

Commit 068af36

Browse files
dxbjaviddxbjavid
committed
avoid integer overflow in container size precheck
1 parent 3792592 commit 068af36

7 files changed

Lines changed: 70 additions & 22 deletions

File tree

‎lib/c_glib/src/thrift/c_glib/protocol/thrift_binary_protocol.c‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -577,9 +577,9 @@ thrift_binary_protocol_read_map_begin (ThriftProtocol *protocol,
577577
return -1;
578578
}
579579

580-
if(!ttc->checkReadBytesAvailable (THRIFT_TRANSPORT(tp->transport),
581-
sizei * thrift_binary_protocol_get_min_serialized_size(protocol, k, error) +
582-
sizei * thrift_binary_protocol_get_min_serialized_size(protocol, v, error),
580+
if(!ttc->checkReadBytesAvailable (THRIFT_TRANSPORT(tp->transport),
581+
(glong) sizei * thrift_binary_protocol_get_min_serialized_size(protocol, k, error) +
582+
(glong) sizei * thrift_binary_protocol_get_min_serialized_size(protocol, v, error),
583583
error))
584584
{
585585
return -1;
@@ -642,8 +642,8 @@ thrift_binary_protocol_read_list_begin (ThriftProtocol *protocol,
642642
return -1;
643643
}
644644

645-
if(!ttc->checkReadBytesAvailable (THRIFT_TRANSPORT(tp->transport),
646-
(sizei * thrift_binary_protocol_get_min_serialized_size(protocol, e, error)),
645+
if(!ttc->checkReadBytesAvailable (THRIFT_TRANSPORT(tp->transport),
646+
((glong) sizei * thrift_binary_protocol_get_min_serialized_size(protocol, e, error)),
647647
error))
648648
{
649649
return -1;

‎lib/c_glib/src/thrift/c_glib/protocol/thrift_compact_protocol.c‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1108,9 +1108,9 @@ thrift_compact_protocol_read_map_begin (ThriftProtocol *protocol,
11081108
return -1;
11091109
}
11101110

1111-
if(!ttc->checkReadBytesAvailable (THRIFT_TRANSPORT (tp->transport),
1112-
msize * thrift_protocol_get_min_serialized_size (protocol, *key_type, error) +
1113-
msize * thrift_protocol_get_min_serialized_size (protocol, *value_type, error),
1111+
if(!ttc->checkReadBytesAvailable (THRIFT_TRANSPORT (tp->transport),
1112+
(glong) msize * thrift_protocol_get_min_serialized_size (protocol, *key_type, error) +
1113+
(glong) msize * thrift_protocol_get_min_serialized_size (protocol, *value_type, error),
11141114
error))
11151115
{
11161116
return -1;
@@ -1183,8 +1183,8 @@ thrift_compact_protocol_read_list_begin (ThriftProtocol *protocol,
11831183
*element_type = ret;
11841184
*size = (guint32) lsize;
11851185

1186-
if(!ttc->checkReadBytesAvailable (THRIFT_TRANSPORT (tp->transport),
1187-
(lsize * thrift_protocol_get_min_serialized_size (protocol, *element_type, error)),
1186+
if(!ttc->checkReadBytesAvailable (THRIFT_TRANSPORT (tp->transport),
1187+
((glong) lsize * thrift_protocol_get_min_serialized_size (protocol, *element_type, error)),
11881188
error))
11891189
{
11901190
return -1;

‎lib/cpp/src/thrift/protocol/TBinaryProtocol.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -174,18 +174,18 @@ class TBinaryProtocolT : public TVirtualProtocol<TBinaryProtocolT<Transport_, By
174174

175175
void checkReadBytesAvailable(TSet& set) override
176176
{
177-
trans_->checkReadBytesAvailable(set.size_ * getMinSerializedSize(set.elemType_));
177+
trans_->checkReadBytesAvailable(static_cast<long>(set.size_) * getMinSerializedSize(set.elemType_));
178178
}
179179

180180
void checkReadBytesAvailable(TList& list) override
181181
{
182-
trans_->checkReadBytesAvailable(list.size_ * getMinSerializedSize(list.elemType_));
182+
trans_->checkReadBytesAvailable(static_cast<long>(list.size_) * getMinSerializedSize(list.elemType_));
183183
}
184184

185185
void checkReadBytesAvailable(TMap& map) override
186186
{
187187
int elmSize = getMinSerializedSize(map.keyType_) + getMinSerializedSize(map.valueType_);
188-
trans_->checkReadBytesAvailable(map.size_ * elmSize);
188+
trans_->checkReadBytesAvailable(static_cast<long>(map.size_) * elmSize);
189189
}
190190

191191
protected:

‎lib/cpp/src/thrift/protocol/TCompactProtocol.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -146,18 +146,18 @@ class TCompactProtocolT : public TVirtualProtocol<TCompactProtocolT<Transport_>
146146

147147
void checkReadBytesAvailable(TSet& set) override
148148
{
149-
trans_->checkReadBytesAvailable(set.size_ * getMinSerializedSize(set.elemType_));
149+
trans_->checkReadBytesAvailable(static_cast<long>(set.size_) * getMinSerializedSize(set.elemType_));
150150
}
151151

152152
void checkReadBytesAvailable(TList& list) override
153153
{
154-
trans_->checkReadBytesAvailable(list.size_ * getMinSerializedSize(list.elemType_));
154+
trans_->checkReadBytesAvailable(static_cast<long>(list.size_) * getMinSerializedSize(list.elemType_));
155155
}
156156

157157
void checkReadBytesAvailable(TMap& map) override
158158
{
159159
int elmSize = getMinSerializedSize(map.keyType_) + getMinSerializedSize(map.valueType_);
160-
trans_->checkReadBytesAvailable(map.size_ * elmSize);
160+
trans_->checkReadBytesAvailable(static_cast<long>(map.size_) * elmSize);
161161
}
162162

163163
/**

‎lib/cpp/src/thrift/protocol/TJSONProtocol.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -253,18 +253,18 @@ class TJSONProtocol : public TVirtualProtocol<TJSONProtocol> {
253253

254254
void checkReadBytesAvailable(TSet& set) override
255255
{
256-
trans_->checkReadBytesAvailable(set.size_ * getMinSerializedSize(set.elemType_));
256+
trans_->checkReadBytesAvailable(static_cast<long>(set.size_) * getMinSerializedSize(set.elemType_));
257257
}
258258

259259
void checkReadBytesAvailable(TList& list) override
260260
{
261-
trans_->checkReadBytesAvailable(list.size_ * getMinSerializedSize(list.elemType_));
261+
trans_->checkReadBytesAvailable(static_cast<long>(list.size_) * getMinSerializedSize(list.elemType_));
262262
}
263263

264264
void checkReadBytesAvailable(TMap& map) override
265265
{
266266
int elmSize = getMinSerializedSize(map.keyType_) + getMinSerializedSize(map.valueType_);
267-
trans_->checkReadBytesAvailable(map.size_ * elmSize);
267+
trans_->checkReadBytesAvailable(static_cast<long>(map.size_) * elmSize);
268268
}
269269

270270
class LookaheadReader {

‎lib/cpp/src/thrift/protocol/TProtocol.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -604,18 +604,18 @@ class TProtocol {
604604

605605
virtual void checkReadBytesAvailable(TSet& set)
606606
{
607-
ptrans_->checkReadBytesAvailable(set.size_ * getMinSerializedSize(set.elemType_));
607+
ptrans_->checkReadBytesAvailable(static_cast<long>(set.size_) * getMinSerializedSize(set.elemType_));
608608
}
609609

610610
virtual void checkReadBytesAvailable(TList& list)
611611
{
612-
ptrans_->checkReadBytesAvailable(list.size_ * getMinSerializedSize(list.elemType_));
612+
ptrans_->checkReadBytesAvailable(static_cast<long>(list.size_) * getMinSerializedSize(list.elemType_));
613613
}
614614

615615
virtual void checkReadBytesAvailable(TMap& map)
616616
{
617617
int elmSize = getMinSerializedSize(map.keyType_) + getMinSerializedSize(map.valueType_);
618-
ptrans_->checkReadBytesAvailable(map.size_ * elmSize);
618+
ptrans_->checkReadBytesAvailable(static_cast<long>(map.size_) * elmSize);
619619
}
620620

621621
std::shared_ptr<TTransport> ptrans_;

‎lib/cpp/test/ThrifttReadCheckTests.cpp‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,54 @@ BOOST_AUTO_TEST_CASE(test_tthriftcompactprotocol_read_check_pass) {
242242
BOOST_CHECK_NO_THROW(protocol->readString(eleven));
243243
}
244244

245+
BOOST_AUTO_TEST_CASE(test_tthriftbinaryprotocol_container_size_overflow) {
246+
std::shared_ptr<TConfiguration> config (new TConfiguration(1024));
247+
std::shared_ptr<TMemoryBuffer> transport(new TMemoryBuffer(config));
248+
std::shared_ptr<TBinaryProtocol> protocol(new TBinaryProtocol(transport));
249+
250+
uint32_t val = 0;
251+
TType elemType = apache::thrift::protocol::T_STOP;
252+
// 0x40000000 elements of min size 4 require 4 GiB; the product wraps to 0 in
253+
// 32-bit math and used to slip past the MaxMessageSize check.
254+
TList list(T_I32, 0x40000000);
255+
protocol->writeListBegin(list.elemType_, list.size_);
256+
protocol->writeListEnd();
257+
BOOST_CHECK_THROW(protocol->readListBegin(elemType, val), TTransportException);
258+
protocol->readListEnd();
259+
}
260+
261+
BOOST_AUTO_TEST_CASE(test_tthriftcompactprotocol_container_size_overflow) {
262+
std::shared_ptr<TConfiguration> config (new TConfiguration(1024));
263+
std::shared_ptr<TMemoryBuffer> transport(new TMemoryBuffer(config));
264+
std::shared_ptr<TCompactProtocol> protocol(new TCompactProtocol(transport));
265+
266+
uint32_t val = 0;
267+
TType elemType = apache::thrift::protocol::T_STOP;
268+
// 0x10000000 elements of min size 16 (UUID) require 4 GiB; the product wraps
269+
// to 0 in 32-bit math and used to slip past the MaxMessageSize check.
270+
TList list(T_UUID, 0x10000000);
271+
protocol->writeListBegin(list.elemType_, list.size_);
272+
protocol->writeListEnd();
273+
BOOST_CHECK_THROW(protocol->readListBegin(elemType, val), TTransportException);
274+
protocol->readListEnd();
275+
}
276+
277+
BOOST_AUTO_TEST_CASE(test_tthriftjsonprotocol_container_size_overflow) {
278+
std::shared_ptr<TConfiguration> config (new TConfiguration(1024));
279+
std::shared_ptr<TMemoryBuffer> transport(new TMemoryBuffer(config));
280+
std::shared_ptr<TJSONProtocol> protocol(new TJSONProtocol(transport));
281+
282+
uint32_t val = 0;
283+
TType elemType = apache::thrift::protocol::T_STOP;
284+
// 0x10000000 elements of min size 16 (UUID) require 4 GiB; the product wraps
285+
// to 0 in 32-bit math and used to slip past the MaxMessageSize check.
286+
TList list(T_UUID, 0x10000000);
287+
protocol->writeListBegin(list.elemType_, list.size_);
288+
protocol->writeListEnd();
289+
BOOST_CHECK_THROW(protocol->readListBegin(elemType, val), TTransportException);
290+
protocol->readListEnd();
291+
}
292+
245293
BOOST_AUTO_TEST_CASE(test_tthriftjsonprotocol_read_check_exception) {
246294
std::shared_ptr<TConfiguration> config (new TConfiguration(MAX_MESSAGE_SIZE));
247295
std::shared_ptr<TMemoryBuffer> transport(new TMemoryBuffer(config));

0 commit comments

Comments
 (0)