Skip to content

Commit 983c813

Browse files
dxbjavidJens-G
dxbjavid
authored and
Jens-G
committed
Widen multiplication operands in container size prechecks to prevent arithmetic overflow on LLP64 platforms
Client: cpp,c_glib Patch: Javid Khan This closes #3590
1 parent 35c1a53 commit 983c813

10 files changed

Lines changed: 74 additions & 25 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+
(gint64) sizei * thrift_binary_protocol_get_min_serialized_size(protocol, k, error) +
582+
(gint64) 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+
((gint64) 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+
(gint64) msize * thrift_protocol_get_min_serialized_size (protocol, *key_type, error) +
1113+
(gint64) 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+
((gint64) lsize * thrift_protocol_get_min_serialized_size (protocol, *element_type, error)),
11881188
error))
11891189
{
11901190
return -1;

‎lib/c_glib/src/thrift/c_glib/transport/thrift_transport.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ thrift_transport_updateKnownMessageSize(ThriftTransport *transport, glong size,
160160
}
161161

162162
gboolean
163-
thrift_transport_checkReadBytesAvailable(ThriftTransport *transport, glong numBytes, GError **error)
163+
thrift_transport_checkReadBytesAvailable(ThriftTransport *transport, gint64 numBytes, GError **error)
164164
{
165165
gboolean boolean = TRUE;
166166
ThriftTransport *tt = THRIFT_TRANSPORT (transport);

‎lib/c_glib/src/thrift/c_glib/transport/thrift_transport.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ struct _ThriftTransportClass
8383
gint32 (*read_all) (ThriftTransport *transport, gpointer buf,
8484
guint32 len, GError **error);
8585
gboolean (*updateKnownMessageSize) (ThriftTransport *transport, glong size, GError **error);
86-
gboolean (*checkReadBytesAvailable) (ThriftTransport *transport, glong numBytes, GError **error);
86+
gboolean (*checkReadBytesAvailable) (ThriftTransport *transport, gint64 numBytes, GError **error);
8787
gboolean (*resetConsumedMessageSize) (ThriftTransport *transport, glong newSize, GError **error);
8888
gboolean (*countConsumedMessageBytes) (ThriftTransport *transport, glong numBytes, GError **error);
8989
};

‎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<int64_t>(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<int64_t>(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<int64_t>(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<int64_t>(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<int64_t>(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<int64_t>(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<int64_t>(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<int64_t>(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<int64_t>(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<int64_t>(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<int64_t>(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<int64_t>(map.size_) * elmSize);
619619
}
620620

621621
std::shared_ptr<TTransport> ptrans_;

‎lib/cpp/src/thrift/transport/TTransport.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include <thrift/Thrift.h>
2424
#include <thrift/TConfiguration.h>
2525
#include <thrift/transport/TTransportException.h>
26+
#include <cstdint>
2627
#include <memory>
2728
#include <string>
2829

@@ -282,7 +283,7 @@ class TTransport {
282283
*
283284
* @param numBytes numBytes bytes of data
284285
*/
285-
void checkReadBytesAvailable(long int numBytes)
286+
void checkReadBytesAvailable(int64_t numBytes)
286287
{
287288
if (remainingMessageSize_ < numBytes || numBytes < 0)
288289
throw TTransportException(TTransportException::END_OF_FILE, "MaxMessageSize reached");

‎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)