From e703d8a2f7bc1e24f37faa7fb40bc1ddde810a4f Mon Sep 17 00:00:00 2001 From: jwang Date: Fri, 24 Aug 2018 15:50:55 -0700 Subject: [PATCH 1/7] adding int overflow checks to vrefbuffer --- src/vrefbuffer.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/vrefbuffer.c b/src/vrefbuffer.c index 28347476..4734b0ff 100644 --- a/src/vrefbuffer.c +++ b/src/vrefbuffer.c @@ -43,6 +43,10 @@ bool msgpack_vrefbuffer_init(msgpack_vrefbuffer* vbuf, vbuf->end = array + nfirst; vbuf->array = array; + if((sizeof(msgpack_vrefbuffer_chunk) + chunk_size) < chunk_size){ + return false; + } + chunk = (msgpack_vrefbuffer_chunk*)malloc( sizeof(msgpack_vrefbuffer_chunk) + chunk_size); if(chunk == NULL) { @@ -135,6 +139,9 @@ int msgpack_vrefbuffer_append_copy(msgpack_vrefbuffer* vbuf, sz = len; } + if((sizeof(msgpack_vrefbuffer_chunk) + sz) < sz){ + return -1; + } chunk = (msgpack_vrefbuffer_chunk*)malloc( sizeof(msgpack_vrefbuffer_chunk) + sz); if(chunk == NULL) { @@ -165,6 +172,10 @@ int msgpack_vrefbuffer_migrate(msgpack_vrefbuffer* vbuf, msgpack_vrefbuffer* to) { size_t sz = vbuf->chunk_size; + if((sizeof(msgpack_vrefbuffer_chunk) + sz) < sz){ + return -1; + } + msgpack_vrefbuffer_chunk* empty = (msgpack_vrefbuffer_chunk*)malloc( sizeof(msgpack_vrefbuffer_chunk) + sz); if(empty == NULL) { From b3dfe28be4924b912b29479c39d9a19160ff659d Mon Sep 17 00:00:00 2001 From: jwang Date: Mon, 27 Aug 2018 13:28:11 -0700 Subject: [PATCH 2/7] adding int overflow checks to vrefbuffer --- src/vrefbuffer.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/vrefbuffer.c b/src/vrefbuffer.c index 4734b0ff..46bb00fd 100644 --- a/src/vrefbuffer.c +++ b/src/vrefbuffer.c @@ -171,12 +171,13 @@ int msgpack_vrefbuffer_append_copy(msgpack_vrefbuffer* vbuf, int msgpack_vrefbuffer_migrate(msgpack_vrefbuffer* vbuf, msgpack_vrefbuffer* to) { size_t sz = vbuf->chunk_size; + msgpack_vrefbuffer_chunk* empty = NULL; if((sizeof(msgpack_vrefbuffer_chunk) + sz) < sz){ return -1; } - msgpack_vrefbuffer_chunk* empty = (msgpack_vrefbuffer_chunk*)malloc( + empty = (msgpack_vrefbuffer_chunk*)malloc( sizeof(msgpack_vrefbuffer_chunk) + sz); if(empty == NULL) { return -1; From 60930f4b122f499cfbb644c67c93e0112e6c2e4a Mon Sep 17 00:00:00 2001 From: jwang Date: Wed, 29 Aug 2018 16:02:14 -0700 Subject: [PATCH 3/7] adding unit tests and fixing same overflow issue in hpp files --- include/msgpack/v1/vrefbuffer.hpp | 16 +++++++++++++++- test/msgpack_c.cpp | 13 +++++++++++++ test/msgpack_vref.cpp | 9 +++++++++ 3 files changed, 37 insertions(+), 1 deletion(-) diff --git a/include/msgpack/v1/vrefbuffer.hpp b/include/msgpack/v1/vrefbuffer.hpp index 6d52a15e..c0240dbe 100644 --- a/include/msgpack/v1/vrefbuffer.hpp +++ b/include/msgpack/v1/vrefbuffer.hpp @@ -71,6 +71,12 @@ public: m_end = array + nfirst; m_array = array; + + if((sizeof(chunk) + chunk_size) < chunk_size){ + throw std::bad_alloc(); + } + + chunk* c = static_cast(::malloc(sizeof(chunk) + chunk_size)); if(!c) { ::free(array); @@ -141,7 +147,11 @@ public: if(sz < len) { sz = len; } - + + if(sizeof(chunk) + sz < sz){ + throw std::bad_alloc(); + } + chunk* c = static_cast(::malloc(sizeof(chunk) + sz)); if(!c) { throw std::bad_alloc(); @@ -183,6 +193,10 @@ public: { size_t sz = m_chunk_size; + if((sizeof(chunk) + sz) < sz){ + throw std::bad_alloc(); + } + chunk* empty = static_cast(::malloc(sizeof(chunk) + sz)); if(!empty) { throw std::bad_alloc(); diff --git a/test/msgpack_c.cpp b/test/msgpack_c.cpp index c979177a..f77f31ee 100644 --- a/test/msgpack_c.cpp +++ b/test/msgpack_c.cpp @@ -1352,3 +1352,16 @@ TEST(MSGPACKC, unpack_array_uint64) EXPECT_EQ(0xFFF0000000000001LL, obj.via.array.ptr[0].via.u64); msgpack_zone_destroy(&z); } + + +TEST(MSGPACKC, vreff_buffer_overflow) +{ + msgpack_vrefbuffer vbuf; + msgpack_vrefbuffer to; + size_t ref_size = 0; + size_t len = 0x1000; + size_t chunk_size = std::numeric_limits::max(); + char *buf = (char *)malloc(len); + EXPECT_FALSE(msgpack_vrefbuffer_init(&vbuf, ref_size, chunk_size)); + EXPECT_EQ(-1, msgpack_vrefbuffer_migrate(&vbuf, &to)); +} diff --git a/test/msgpack_vref.cpp b/test/msgpack_vref.cpp index 85f5e663..8a85e45b 100644 --- a/test/msgpack_vref.cpp +++ b/test/msgpack_vref.cpp @@ -264,3 +264,12 @@ TEST(MSGPACK, vrefbuffer_small_int64) msgpack::vrefbuffer vbuf(0, 0); GEN_TEST_VREF(int64_t, vbuf); } + +TEST(MSGPACK, vref_buffer_overflow) +{ + size_t chunk_size = std::numeric_limits::max(); + char *buf = (char *)malloc(chunk_size); + ASSERT_THROW(msgpack::vrefbuffer vbuf(0, chunk_size), std::bad_alloc); + msgpack::vrefbuffer vbuf(0,0x1000); + ASSERT_THROW(vbuf.append_copy(buf, chunk_size), std::bad_alloc); +} From 0421dabc1ec7c4dacd33bae66bb697066e66ba74 Mon Sep 17 00:00:00 2001 From: jwang Date: Fri, 31 Aug 2018 10:39:00 -0700 Subject: [PATCH 4/7] removing unused vars --- test/msgpack_c.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/msgpack_c.cpp b/test/msgpack_c.cpp index f77f31ee..a1d21300 100644 --- a/test/msgpack_c.cpp +++ b/test/msgpack_c.cpp @@ -1359,9 +1359,7 @@ TEST(MSGPACKC, vreff_buffer_overflow) msgpack_vrefbuffer vbuf; msgpack_vrefbuffer to; size_t ref_size = 0; - size_t len = 0x1000; size_t chunk_size = std::numeric_limits::max(); - char *buf = (char *)malloc(len); EXPECT_FALSE(msgpack_vrefbuffer_init(&vbuf, ref_size, chunk_size)); EXPECT_EQ(-1, msgpack_vrefbuffer_migrate(&vbuf, &to)); } From c056026dad4a81bdd304612bf09e4701b70d31f5 Mon Sep 17 00:00:00 2001 From: tbeu Date: Mon, 3 Sep 2018 22:22:49 +0200 Subject: [PATCH 5/7] Fix memory leaks --- include/msgpack/v1/vrefbuffer.hpp | 3 +-- src/vrefbuffer.c | 3 ++- test/msgpack_c.cpp | 2 +- test/msgpack_vref.cpp | 10 ++++++---- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/include/msgpack/v1/vrefbuffer.hpp b/include/msgpack/v1/vrefbuffer.hpp index c0240dbe..b80b7a6a 100644 --- a/include/msgpack/v1/vrefbuffer.hpp +++ b/include/msgpack/v1/vrefbuffer.hpp @@ -71,11 +71,10 @@ public: m_end = array + nfirst; m_array = array; - if((sizeof(chunk) + chunk_size) < chunk_size){ + ::free(array); throw std::bad_alloc(); } - chunk* c = static_cast(::malloc(sizeof(chunk) + chunk_size)); if(!c) { diff --git a/src/vrefbuffer.c b/src/vrefbuffer.c index 46bb00fd..ea493793 100644 --- a/src/vrefbuffer.c +++ b/src/vrefbuffer.c @@ -44,6 +44,7 @@ bool msgpack_vrefbuffer_init(msgpack_vrefbuffer* vbuf, vbuf->array = array; if((sizeof(msgpack_vrefbuffer_chunk) + chunk_size) < chunk_size){ + free(array); return false; } @@ -171,7 +172,7 @@ int msgpack_vrefbuffer_append_copy(msgpack_vrefbuffer* vbuf, int msgpack_vrefbuffer_migrate(msgpack_vrefbuffer* vbuf, msgpack_vrefbuffer* to) { size_t sz = vbuf->chunk_size; - msgpack_vrefbuffer_chunk* empty = NULL; + msgpack_vrefbuffer_chunk* empty; if((sizeof(msgpack_vrefbuffer_chunk) + sz) < sz){ return -1; diff --git a/test/msgpack_c.cpp b/test/msgpack_c.cpp index a1d21300..cfcfed0c 100644 --- a/test/msgpack_c.cpp +++ b/test/msgpack_c.cpp @@ -1354,7 +1354,7 @@ TEST(MSGPACKC, unpack_array_uint64) } -TEST(MSGPACKC, vreff_buffer_overflow) +TEST(MSGPACKC, vref_buffer_overflow) { msgpack_vrefbuffer vbuf; msgpack_vrefbuffer to; diff --git a/test/msgpack_vref.cpp b/test/msgpack_vref.cpp index 8a85e45b..9a7f188c 100644 --- a/test/msgpack_vref.cpp +++ b/test/msgpack_vref.cpp @@ -266,10 +266,12 @@ TEST(MSGPACK, vrefbuffer_small_int64) } TEST(MSGPACK, vref_buffer_overflow) -{ +{ + size_t ref_size = 0; size_t chunk_size = std::numeric_limits::max(); char *buf = (char *)malloc(chunk_size); - ASSERT_THROW(msgpack::vrefbuffer vbuf(0, chunk_size), std::bad_alloc); - msgpack::vrefbuffer vbuf(0,0x1000); - ASSERT_THROW(vbuf.append_copy(buf, chunk_size), std::bad_alloc); + ASSERT_THROW(msgpack::vrefbuffer vbuf(ref_size, chunk_size), std::bad_alloc); +// msgpack::vrefbuffer vbuf2(0, 0x1000); +// ASSERT_THROW(vbuf2.append_copy(buf, chunk_size), std::bad_alloc); + free(buf); } From 349c1331713bbf16cd2fb3cdefea3977021d4ed3 Mon Sep 17 00:00:00 2001 From: jwang Date: Wed, 5 Sep 2018 09:40:54 +0200 Subject: [PATCH 6/7] Fix malloc size --- test/msgpack_vref.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/msgpack_vref.cpp b/test/msgpack_vref.cpp index 9a7f188c..dfd00e01 100644 --- a/test/msgpack_vref.cpp +++ b/test/msgpack_vref.cpp @@ -267,11 +267,11 @@ TEST(MSGPACK, vrefbuffer_small_int64) TEST(MSGPACK, vref_buffer_overflow) { - size_t ref_size = 0; + size_t ref_size = 0; size_t chunk_size = std::numeric_limits::max(); - char *buf = (char *)malloc(chunk_size); + char *buf = (char *)malloc(0x1000); ASSERT_THROW(msgpack::vrefbuffer vbuf(ref_size, chunk_size), std::bad_alloc); -// msgpack::vrefbuffer vbuf2(0, 0x1000); -// ASSERT_THROW(vbuf2.append_copy(buf, chunk_size), std::bad_alloc); + msgpack::vrefbuffer vbuf2(0, 0x1000); + ASSERT_THROW(vbuf2.append_copy(buf, chunk_size), std::bad_alloc); free(buf); } From d72765870a601e5deb401eab1fb1e1796e797d3e Mon Sep 17 00:00:00 2001 From: tbeu Date: Thu, 6 Sep 2018 15:09:52 +0200 Subject: [PATCH 7/7] Move overflow check up --- include/msgpack/v1/vrefbuffer.hpp | 9 ++++----- src/vrefbuffer.c | 9 ++++----- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/include/msgpack/v1/vrefbuffer.hpp b/include/msgpack/v1/vrefbuffer.hpp index b80b7a6a..39bcd69c 100644 --- a/include/msgpack/v1/vrefbuffer.hpp +++ b/include/msgpack/v1/vrefbuffer.hpp @@ -58,6 +58,10 @@ public: :m_ref_size(std::max(ref_size, detail::packer_max_buffer_size + 1)), m_chunk_size(chunk_size) { + if((sizeof(chunk) + chunk_size) < chunk_size) { + throw std::bad_alloc(); + } + size_t nfirst = (sizeof(iovec) < 72/2) ? 72 / sizeof(iovec) : 8; @@ -71,11 +75,6 @@ public: m_end = array + nfirst; m_array = array; - if((sizeof(chunk) + chunk_size) < chunk_size){ - ::free(array); - throw std::bad_alloc(); - } - chunk* c = static_cast(::malloc(sizeof(chunk) + chunk_size)); if(!c) { ::free(array); diff --git a/src/vrefbuffer.c b/src/vrefbuffer.c index ea493793..e03d832b 100644 --- a/src/vrefbuffer.c +++ b/src/vrefbuffer.c @@ -30,6 +30,10 @@ bool msgpack_vrefbuffer_init(msgpack_vrefbuffer* vbuf, ref_size > MSGPACK_PACKER_MAX_BUFFER_SIZE + 1 ? ref_size : MSGPACK_PACKER_MAX_BUFFER_SIZE + 1 ; + if((sizeof(msgpack_vrefbuffer_chunk) + chunk_size) < chunk_size) { + return false; + } + nfirst = (sizeof(struct iovec) < 72/2) ? 72 / sizeof(struct iovec) : 8; @@ -43,11 +47,6 @@ bool msgpack_vrefbuffer_init(msgpack_vrefbuffer* vbuf, vbuf->end = array + nfirst; vbuf->array = array; - if((sizeof(msgpack_vrefbuffer_chunk) + chunk_size) < chunk_size){ - free(array); - return false; - } - chunk = (msgpack_vrefbuffer_chunk*)malloc( sizeof(msgpack_vrefbuffer_chunk) + chunk_size); if(chunk == NULL) {