[jira] [Commented] (THRIFT-3821) TMemoryBuffer buffer may overflow when resizing

2017-09-22 Thread James E. King, III (JIRA) 

[ 
https://issues.apache.org/jira/browse/THRIFT-3821?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16177257#comment-16177257
 ] 

James E. King, III commented on THRIFT-3821:


I checked in a small change to make the maximum buffer size configurable so 
that the unit test would not need 2GB of memory - it was getting killed in one 
of my build environments.

> TMemoryBuffer buffer may overflow when resizing
> ---
>
> Key: THRIFT-3821
> URL: https://issues.apache.org/jira/browse/THRIFT-3821
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Library
>Affects Versions: 0.9.3
>Reporter: Huaisi Xu
>Assignee: James E. King, III
> Fix For: 0.11.0
>
>
> In ensurecanwrite():
> {code}
>   uint32_t new_size = bufferSize_;
>   while (len > avail) {
> new_size = new_size > 0 ? new_size * 2 : 1;
> avail = available_write() + (new_size - bufferSize_);
>   }
>   // Allocate into a new pointer so we don't bork ours if it fails.
>   uint8_t* new_buffer = static_cast(std::realloc(buffer_, 
> new_size));
>   if (new_buffer == NULL) {
> throw std::bad_alloc();
>   }
>   rBase_ = new_buffer + (rBase_ - buffer_);
>   rBound_ = new_buffer + (rBound_ - buffer_);
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
>   buffer_ = new_buffer;
>   bufferSize_ = new_size;
> {code}
> If old bufferSize_ is lager than 2gb, then calculating new size will overflow.
> i.e. if bufferSize_ = 3355443200, then new buffer size will be 2415919104, 
> which is less than old size.
> However, 
> {code}
> avail = available_write() + (new_size - bufferSize_) 
> {code}
> overflows again, so we will end up with an shrinked buffer.
> What is worse is that after
> {code}
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
> {code}
> wBase_ stays the same, but wBound_ becomes lower than it. What happens next 
> is that   uint32_t avail = available_write() may overflow every time 
> subsequently. and thrift writes to unknown memory.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (THRIFT-3821) TMemoryBuffer buffer may overflow when resizing

2017-09-22 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/THRIFT-3821?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16177255#comment-16177255
 ] 

ASF GitHub Bot commented on THRIFT-3821:


Github user asfgit closed the pull request at:

https://github.com/apache/thrift/pull/1369


> TMemoryBuffer buffer may overflow when resizing
> ---
>
> Key: THRIFT-3821
> URL: https://issues.apache.org/jira/browse/THRIFT-3821
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Library
>Affects Versions: 0.9.3
>Reporter: Huaisi Xu
>Assignee: James E. King, III
> Fix For: 0.11.0
>
>
> In ensurecanwrite():
> {code}
>   uint32_t new_size = bufferSize_;
>   while (len > avail) {
> new_size = new_size > 0 ? new_size * 2 : 1;
> avail = available_write() + (new_size - bufferSize_);
>   }
>   // Allocate into a new pointer so we don't bork ours if it fails.
>   uint8_t* new_buffer = static_cast(std::realloc(buffer_, 
> new_size));
>   if (new_buffer == NULL) {
> throw std::bad_alloc();
>   }
>   rBase_ = new_buffer + (rBase_ - buffer_);
>   rBound_ = new_buffer + (rBound_ - buffer_);
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
>   buffer_ = new_buffer;
>   bufferSize_ = new_size;
> {code}
> If old bufferSize_ is lager than 2gb, then calculating new size will overflow.
> i.e. if bufferSize_ = 3355443200, then new buffer size will be 2415919104, 
> which is less than old size.
> However, 
> {code}
> avail = available_write() + (new_size - bufferSize_) 
> {code}
> overflows again, so we will end up with an shrinked buffer.
> What is worse is that after
> {code}
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
> {code}
> wBase_ stays the same, but wBound_ becomes lower than it. What happens next 
> is that   uint32_t avail = available_write() may overflow every time 
> subsequently. and thrift writes to unknown memory.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (THRIFT-3821) TMemoryBuffer buffer may overflow when resizing

2017-09-22 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/THRIFT-3821?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16176872#comment-16176872
 ] 

ASF GitHub Bot commented on THRIFT-3821:


GitHub user jeking3 opened a pull request:

https://github.com/apache/thrift/pull/1369

THRIFT-3821: make memory buffer size configurable

 so unit test doesn't need 2GB to run; 
also added a unit test to prove THRIFT-3480 alternative

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/jeking3/thrift memorybuffer

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/thrift/pull/1369.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1369


commit 22f7af0e75768680fea79b656d6c32df5a430d43
Author: James E. King, III 
Date:   2017-09-22T18:20:15Z

THRIFT-3821: make memory buffer size configurable so unit test doesn't need 
2GB to run; prove THRIFT-3480




> TMemoryBuffer buffer may overflow when resizing
> ---
>
> Key: THRIFT-3821
> URL: https://issues.apache.org/jira/browse/THRIFT-3821
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Library
>Affects Versions: 0.9.3
>Reporter: Huaisi Xu
>Assignee: James E. King, III
> Fix For: 0.11.0
>
>
> In ensurecanwrite():
> {code}
>   uint32_t new_size = bufferSize_;
>   while (len > avail) {
> new_size = new_size > 0 ? new_size * 2 : 1;
> avail = available_write() + (new_size - bufferSize_);
>   }
>   // Allocate into a new pointer so we don't bork ours if it fails.
>   uint8_t* new_buffer = static_cast(std::realloc(buffer_, 
> new_size));
>   if (new_buffer == NULL) {
> throw std::bad_alloc();
>   }
>   rBase_ = new_buffer + (rBase_ - buffer_);
>   rBound_ = new_buffer + (rBound_ - buffer_);
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
>   buffer_ = new_buffer;
>   bufferSize_ = new_size;
> {code}
> If old bufferSize_ is lager than 2gb, then calculating new size will overflow.
> i.e. if bufferSize_ = 3355443200, then new buffer size will be 2415919104, 
> which is less than old size.
> However, 
> {code}
> avail = available_write() + (new_size - bufferSize_) 
> {code}
> overflows again, so we will end up with an shrinked buffer.
> What is worse is that after
> {code}
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
> {code}
> wBase_ stays the same, but wBound_ becomes lower than it. What happens next 
> is that   uint32_t avail = available_write() may overflow every time 
> subsequently. and thrift writes to unknown memory.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (THRIFT-3821) TMemoryBuffer buffer may overflow when resizing

2017-08-12 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/THRIFT-3821?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16124780#comment-16124780
 ] 

ASF GitHub Bot commented on THRIFT-3821:


Github user asfgit closed the pull request at:

https://github.com/apache/thrift/pull/1326


> TMemoryBuffer buffer may overflow when resizing
> ---
>
> Key: THRIFT-3821
> URL: https://issues.apache.org/jira/browse/THRIFT-3821
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Library
>Affects Versions: 0.9.3
>Reporter: Huaisi Xu
>Assignee: James E. King, III
> Fix For: 0.11.0
>
>
> In ensurecanwrite():
> {code}
>   uint32_t new_size = bufferSize_;
>   while (len > avail) {
> new_size = new_size > 0 ? new_size * 2 : 1;
> avail = available_write() + (new_size - bufferSize_);
>   }
>   // Allocate into a new pointer so we don't bork ours if it fails.
>   uint8_t* new_buffer = static_cast(std::realloc(buffer_, 
> new_size));
>   if (new_buffer == NULL) {
> throw std::bad_alloc();
>   }
>   rBase_ = new_buffer + (rBase_ - buffer_);
>   rBound_ = new_buffer + (rBound_ - buffer_);
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
>   buffer_ = new_buffer;
>   bufferSize_ = new_size;
> {code}
> If old bufferSize_ is lager than 2gb, then calculating new size will overflow.
> i.e. if bufferSize_ = 3355443200, then new buffer size will be 2415919104, 
> which is less than old size.
> However, 
> {code}
> avail = available_write() + (new_size - bufferSize_) 
> {code}
> overflows again, so we will end up with an shrinked buffer.
> What is worse is that after
> {code}
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
> {code}
> wBase_ stays the same, but wBound_ becomes lower than it. What happens next 
> is that   uint32_t avail = available_write() may overflow every time 
> subsequently. and thrift writes to unknown memory.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (THRIFT-3821) TMemoryBuffer buffer may overflow when resizing

2017-08-12 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/THRIFT-3821?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16124779#comment-16124779
 ] 

ASF GitHub Bot commented on THRIFT-3821:


Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1326
  
I'd prefer if we refactored it to make the limit configurable with a 
default matching what it is now; that way tests won't need multiple gigabytes 
of memory to function properly.  I'll approve and merge this for now as it is 
an improvement.


> TMemoryBuffer buffer may overflow when resizing
> ---
>
> Key: THRIFT-3821
> URL: https://issues.apache.org/jira/browse/THRIFT-3821
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Library
>Affects Versions: 0.9.3
>Reporter: Huaisi Xu
>
> In ensurecanwrite():
> {code}
>   uint32_t new_size = bufferSize_;
>   while (len > avail) {
> new_size = new_size > 0 ? new_size * 2 : 1;
> avail = available_write() + (new_size - bufferSize_);
>   }
>   // Allocate into a new pointer so we don't bork ours if it fails.
>   uint8_t* new_buffer = static_cast(std::realloc(buffer_, 
> new_size));
>   if (new_buffer == NULL) {
> throw std::bad_alloc();
>   }
>   rBase_ = new_buffer + (rBase_ - buffer_);
>   rBound_ = new_buffer + (rBound_ - buffer_);
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
>   buffer_ = new_buffer;
>   bufferSize_ = new_size;
> {code}
> If old bufferSize_ is lager than 2gb, then calculating new size will overflow.
> i.e. if bufferSize_ = 3355443200, then new buffer size will be 2415919104, 
> which is less than old size.
> However, 
> {code}
> avail = available_write() + (new_size - bufferSize_) 
> {code}
> overflows again, so we will end up with an shrinked buffer.
> What is worse is that after
> {code}
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
> {code}
> wBase_ stays the same, but wBound_ becomes lower than it. What happens next 
> is that   uint32_t avail = available_write() may overflow every time 
> subsequently. and thrift writes to unknown memory.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (THRIFT-3821) TMemoryBuffer buffer may overflow when resizing

2017-08-12 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/THRIFT-3821?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16124775#comment-16124775
 ] 

ASF GitHub Bot commented on THRIFT-3821:


Github user asuhan commented on the issue:

https://github.com/apache/thrift/pull/1326
  
Are there any additional changes I should make to this pull request? As 
I've said in the inline comment, I believe comparing to 
`std::numeric_limits::max()` is actually consistent with what the 
error message says.


> TMemoryBuffer buffer may overflow when resizing
> ---
>
> Key: THRIFT-3821
> URL: https://issues.apache.org/jira/browse/THRIFT-3821
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Library
>Affects Versions: 0.9.3
>Reporter: Huaisi Xu
>
> In ensurecanwrite():
> {code}
>   uint32_t new_size = bufferSize_;
>   while (len > avail) {
> new_size = new_size > 0 ? new_size * 2 : 1;
> avail = available_write() + (new_size - bufferSize_);
>   }
>   // Allocate into a new pointer so we don't bork ours if it fails.
>   uint8_t* new_buffer = static_cast(std::realloc(buffer_, 
> new_size));
>   if (new_buffer == NULL) {
> throw std::bad_alloc();
>   }
>   rBase_ = new_buffer + (rBase_ - buffer_);
>   rBound_ = new_buffer + (rBound_ - buffer_);
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
>   buffer_ = new_buffer;
>   bufferSize_ = new_size;
> {code}
> If old bufferSize_ is lager than 2gb, then calculating new size will overflow.
> i.e. if bufferSize_ = 3355443200, then new buffer size will be 2415919104, 
> which is less than old size.
> However, 
> {code}
> avail = available_write() + (new_size - bufferSize_) 
> {code}
> overflows again, so we will end up with an shrinked buffer.
> What is worse is that after
> {code}
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
> {code}
> wBase_ stays the same, but wBound_ becomes lower than it. What happens next 
> is that   uint32_t avail = available_write() may overflow every time 
> subsequently. and thrift writes to unknown memory.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (THRIFT-3821) TMemoryBuffer buffer may overflow when resizing

2017-08-10 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/THRIFT-3821?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16122799#comment-16122799
 ] 

ASF GitHub Bot commented on THRIFT-3821:


Github user asuhan commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1326#discussion_r132613779
  
--- Diff: lib/cpp/src/thrift/transport/TBufferTransports.cpp ---
@@ -361,9 +361,13 @@ void TMemoryBuffer::ensureCanWrite(uint32_t len) {
   }
 
   // Grow the buffer as necessary.
-  uint32_t new_size = bufferSize_;
+  uint64_t new_size = bufferSize_;
   while (len > avail) {
 new_size = new_size > 0 ? new_size * 2 : 1;
+if (new_size > std::numeric_limits::max()) {
--- End diff --

Basically, if we only go through powers of 2 when resizing (which looks to 
be the case), the buffer can only grow to 2GB since the maximum value of 
`uint32_t` is one short of 4GB.  


> TMemoryBuffer buffer may overflow when resizing
> ---
>
> Key: THRIFT-3821
> URL: https://issues.apache.org/jira/browse/THRIFT-3821
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Library
>Affects Versions: 0.9.3
>Reporter: Huaisi Xu
>
> In ensurecanwrite():
> {code}
>   uint32_t new_size = bufferSize_;
>   while (len > avail) {
> new_size = new_size > 0 ? new_size * 2 : 1;
> avail = available_write() + (new_size - bufferSize_);
>   }
>   // Allocate into a new pointer so we don't bork ours if it fails.
>   uint8_t* new_buffer = static_cast(std::realloc(buffer_, 
> new_size));
>   if (new_buffer == NULL) {
> throw std::bad_alloc();
>   }
>   rBase_ = new_buffer + (rBase_ - buffer_);
>   rBound_ = new_buffer + (rBound_ - buffer_);
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
>   buffer_ = new_buffer;
>   bufferSize_ = new_size;
> {code}
> If old bufferSize_ is lager than 2gb, then calculating new size will overflow.
> i.e. if bufferSize_ = 3355443200, then new buffer size will be 2415919104, 
> which is less than old size.
> However, 
> {code}
> avail = available_write() + (new_size - bufferSize_) 
> {code}
> overflows again, so we will end up with an shrinked buffer.
> What is worse is that after
> {code}
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
> {code}
> wBase_ stays the same, but wBound_ becomes lower than it. What happens next 
> is that   uint32_t avail = available_write() may overflow every time 
> subsequently. and thrift writes to unknown memory.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (THRIFT-3821) TMemoryBuffer buffer may overflow when resizing

2017-08-10 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/THRIFT-3821?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16122776#comment-16122776
 ] 

ASF GitHub Bot commented on THRIFT-3821:


Github user asuhan commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1326#discussion_r132612265
  
--- Diff: lib/cpp/src/thrift/transport/TBufferTransports.cpp ---
@@ -361,9 +361,13 @@ void TMemoryBuffer::ensureCanWrite(uint32_t len) {
   }
 
   // Grow the buffer as necessary.
-  uint32_t new_size = bufferSize_;
+  uint64_t new_size = bufferSize_;
   while (len > avail) {
 new_size = new_size > 0 ? new_size * 2 : 1;
+if (new_size > std::numeric_limits::max()) {
--- End diff --

We check it post-resize and it fails if we go past 4 GB, therefore 
pre-resize it must be at most 2GB. Checking against `int32_t` would be overly 
conservative; we mainly care about avoiding the arithmetic overflow. Maybe the 
error message could be improved? `"Internal buffer size was already past 2GB 
when we attempted to resize"` would be a more precise description, but I didn't 
want to make it overly verbose / obscure.


> TMemoryBuffer buffer may overflow when resizing
> ---
>
> Key: THRIFT-3821
> URL: https://issues.apache.org/jira/browse/THRIFT-3821
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Library
>Affects Versions: 0.9.3
>Reporter: Huaisi Xu
>
> In ensurecanwrite():
> {code}
>   uint32_t new_size = bufferSize_;
>   while (len > avail) {
> new_size = new_size > 0 ? new_size * 2 : 1;
> avail = available_write() + (new_size - bufferSize_);
>   }
>   // Allocate into a new pointer so we don't bork ours if it fails.
>   uint8_t* new_buffer = static_cast(std::realloc(buffer_, 
> new_size));
>   if (new_buffer == NULL) {
> throw std::bad_alloc();
>   }
>   rBase_ = new_buffer + (rBase_ - buffer_);
>   rBound_ = new_buffer + (rBound_ - buffer_);
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
>   buffer_ = new_buffer;
>   bufferSize_ = new_size;
> {code}
> If old bufferSize_ is lager than 2gb, then calculating new size will overflow.
> i.e. if bufferSize_ = 3355443200, then new buffer size will be 2415919104, 
> which is less than old size.
> However, 
> {code}
> avail = available_write() + (new_size - bufferSize_) 
> {code}
> overflows again, so we will end up with an shrinked buffer.
> What is worse is that after
> {code}
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
> {code}
> wBase_ stays the same, but wBound_ becomes lower than it. What happens next 
> is that   uint32_t avail = available_write() may overflow every time 
> subsequently. and thrift writes to unknown memory.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (THRIFT-3821) TMemoryBuffer buffer may overflow when resizing

2017-08-10 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/THRIFT-3821?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16122757#comment-16122757
 ] 

ASF GitHub Bot commented on THRIFT-3821:


Github user jeking3 commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1326#discussion_r132611000
  
--- Diff: lib/cpp/src/thrift/transport/TBufferTransports.cpp ---
@@ -361,9 +361,13 @@ void TMemoryBuffer::ensureCanWrite(uint32_t len) {
   }
 
   // Grow the buffer as necessary.
-  uint32_t new_size = bufferSize_;
+  uint64_t new_size = bufferSize_;
   while (len > avail) {
 new_size = new_size > 0 ? new_size * 2 : 1;
+if (new_size > std::numeric_limits::max()) {
--- End diff --

Shouldn't this be int32_t instead of uint32_t, if we want to test against 
2GB?  Sorry I didn't catch this before.


> TMemoryBuffer buffer may overflow when resizing
> ---
>
> Key: THRIFT-3821
> URL: https://issues.apache.org/jira/browse/THRIFT-3821
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Library
>Affects Versions: 0.9.3
>Reporter: Huaisi Xu
>
> In ensurecanwrite():
> {code}
>   uint32_t new_size = bufferSize_;
>   while (len > avail) {
> new_size = new_size > 0 ? new_size * 2 : 1;
> avail = available_write() + (new_size - bufferSize_);
>   }
>   // Allocate into a new pointer so we don't bork ours if it fails.
>   uint8_t* new_buffer = static_cast(std::realloc(buffer_, 
> new_size));
>   if (new_buffer == NULL) {
> throw std::bad_alloc();
>   }
>   rBase_ = new_buffer + (rBase_ - buffer_);
>   rBound_ = new_buffer + (rBound_ - buffer_);
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
>   buffer_ = new_buffer;
>   bufferSize_ = new_size;
> {code}
> If old bufferSize_ is lager than 2gb, then calculating new size will overflow.
> i.e. if bufferSize_ = 3355443200, then new buffer size will be 2415919104, 
> which is less than old size.
> However, 
> {code}
> avail = available_write() + (new_size - bufferSize_) 
> {code}
> overflows again, so we will end up with an shrinked buffer.
> What is worse is that after
> {code}
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
> {code}
> wBase_ stays the same, but wBound_ becomes lower than it. What happens next 
> is that   uint32_t avail = available_write() may overflow every time 
> subsequently. and thrift writes to unknown memory.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (THRIFT-3821) TMemoryBuffer buffer may overflow when resizing

2017-08-10 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/THRIFT-3821?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16122756#comment-16122756
 ] 

ASF GitHub Bot commented on THRIFT-3821:


Github user jeking3 commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1326#discussion_r132611133
  
--- Diff: lib/cpp/test/TMemoryBufferTest.cpp ---
@@ -117,4 +117,17 @@ BOOST_AUTO_TEST_CASE(test_exceptions) {
   BOOST_CHECK_NO_THROW(buf2.write((const uint8_t*)"bar", 3));
 }
 
+#ifndef _WIN32
+// We can't allocate 1 GB of memory in 32-bit environments.
--- End diff --

This makes me wonder if the TBufferTransport should have a size limit that 
is configurable, with a default of INT32_MAX, and then the test can make a 
smaller one like 4KB, and write 4KB and then one byte more, instead of using up 
2GB of memory.


> TMemoryBuffer buffer may overflow when resizing
> ---
>
> Key: THRIFT-3821
> URL: https://issues.apache.org/jira/browse/THRIFT-3821
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Library
>Affects Versions: 0.9.3
>Reporter: Huaisi Xu
>
> In ensurecanwrite():
> {code}
>   uint32_t new_size = bufferSize_;
>   while (len > avail) {
> new_size = new_size > 0 ? new_size * 2 : 1;
> avail = available_write() + (new_size - bufferSize_);
>   }
>   // Allocate into a new pointer so we don't bork ours if it fails.
>   uint8_t* new_buffer = static_cast(std::realloc(buffer_, 
> new_size));
>   if (new_buffer == NULL) {
> throw std::bad_alloc();
>   }
>   rBase_ = new_buffer + (rBase_ - buffer_);
>   rBound_ = new_buffer + (rBound_ - buffer_);
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
>   buffer_ = new_buffer;
>   bufferSize_ = new_size;
> {code}
> If old bufferSize_ is lager than 2gb, then calculating new size will overflow.
> i.e. if bufferSize_ = 3355443200, then new buffer size will be 2415919104, 
> which is less than old size.
> However, 
> {code}
> avail = available_write() + (new_size - bufferSize_) 
> {code}
> overflows again, so we will end up with an shrinked buffer.
> What is worse is that after
> {code}
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
> {code}
> wBase_ stays the same, but wBound_ becomes lower than it. What happens next 
> is that   uint32_t avail = available_write() may overflow every time 
> subsequently. and thrift writes to unknown memory.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (THRIFT-3821) TMemoryBuffer buffer may overflow when resizing

2017-08-10 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/THRIFT-3821?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16122696#comment-16122696
 ] 

ASF GitHub Bot commented on THRIFT-3821:


Github user asuhan commented on the issue:

https://github.com/apache/thrift/pull/1326
  
I've disabled the test for Windows since allocating 1GB on 32-bit is likely 
to fail with `bad_alloc`.

Test failure looks unrelated, also got a clean run before: 
https://travis-ci.org/apache/thrift/jobs/263298365.


> TMemoryBuffer buffer may overflow when resizing
> ---
>
> Key: THRIFT-3821
> URL: https://issues.apache.org/jira/browse/THRIFT-3821
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Library
>Affects Versions: 0.9.3
>Reporter: Huaisi Xu
>
> In ensurecanwrite():
> {code}
>   uint32_t new_size = bufferSize_;
>   while (len > avail) {
> new_size = new_size > 0 ? new_size * 2 : 1;
> avail = available_write() + (new_size - bufferSize_);
>   }
>   // Allocate into a new pointer so we don't bork ours if it fails.
>   uint8_t* new_buffer = static_cast(std::realloc(buffer_, 
> new_size));
>   if (new_buffer == NULL) {
> throw std::bad_alloc();
>   }
>   rBase_ = new_buffer + (rBase_ - buffer_);
>   rBound_ = new_buffer + (rBound_ - buffer_);
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
>   buffer_ = new_buffer;
>   bufferSize_ = new_size;
> {code}
> If old bufferSize_ is lager than 2gb, then calculating new size will overflow.
> i.e. if bufferSize_ = 3355443200, then new buffer size will be 2415919104, 
> which is less than old size.
> However, 
> {code}
> avail = available_write() + (new_size - bufferSize_) 
> {code}
> overflows again, so we will end up with an shrinked buffer.
> What is worse is that after
> {code}
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
> {code}
> wBase_ stays the same, but wBound_ becomes lower than it. What happens next 
> is that   uint32_t avail = available_write() may overflow every time 
> subsequently. and thrift writes to unknown memory.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (THRIFT-3821) TMemoryBuffer buffer may overflow when resizing

2017-08-10 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/THRIFT-3821?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16122386#comment-16122386
 ] 

ASF GitHub Bot commented on THRIFT-3821:


Github user asuhan commented on the issue:

https://github.com/apache/thrift/pull/1326
  
Thanks for the review, I'll address both. It's my preference to squash the 
commits as well, sounds like I've misinterpreted 
https://thrift.apache.org/docs/HowToContribute (it says "When bugfixing: add 
test that will isolate bug before applying change that fixes it").


> TMemoryBuffer buffer may overflow when resizing
> ---
>
> Key: THRIFT-3821
> URL: https://issues.apache.org/jira/browse/THRIFT-3821
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Library
>Affects Versions: 0.9.3
>Reporter: Huaisi Xu
>
> In ensurecanwrite():
> {code}
>   uint32_t new_size = bufferSize_;
>   while (len > avail) {
> new_size = new_size > 0 ? new_size * 2 : 1;
> avail = available_write() + (new_size - bufferSize_);
>   }
>   // Allocate into a new pointer so we don't bork ours if it fails.
>   uint8_t* new_buffer = static_cast(std::realloc(buffer_, 
> new_size));
>   if (new_buffer == NULL) {
> throw std::bad_alloc();
>   }
>   rBase_ = new_buffer + (rBase_ - buffer_);
>   rBound_ = new_buffer + (rBound_ - buffer_);
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
>   buffer_ = new_buffer;
>   bufferSize_ = new_size;
> {code}
> If old bufferSize_ is lager than 2gb, then calculating new size will overflow.
> i.e. if bufferSize_ = 3355443200, then new buffer size will be 2415919104, 
> which is less than old size.
> However, 
> {code}
> avail = available_write() + (new_size - bufferSize_) 
> {code}
> overflows again, so we will end up with an shrinked buffer.
> What is worse is that after
> {code}
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
> {code}
> wBase_ stays the same, but wBound_ becomes lower than it. What happens next 
> is that   uint32_t avail = available_write() may overflow every time 
> subsequently. and thrift writes to unknown memory.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (THRIFT-3821) TMemoryBuffer buffer may overflow when resizing

2017-08-10 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/THRIFT-3821?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16122170#comment-16122170
 ] 

ASF GitHub Bot commented on THRIFT-3821:


Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1326
  
Please rebase (and squash to a single commit, if you can) and push here, so 
the build system can build the change and we can verify it before merging it.  
Thanks.


> TMemoryBuffer buffer may overflow when resizing
> ---
>
> Key: THRIFT-3821
> URL: https://issues.apache.org/jira/browse/THRIFT-3821
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Library
>Affects Versions: 0.9.3
>Reporter: Huaisi Xu
>
> In ensurecanwrite():
> {code}
>   uint32_t new_size = bufferSize_;
>   while (len > avail) {
> new_size = new_size > 0 ? new_size * 2 : 1;
> avail = available_write() + (new_size - bufferSize_);
>   }
>   // Allocate into a new pointer so we don't bork ours if it fails.
>   uint8_t* new_buffer = static_cast(std::realloc(buffer_, 
> new_size));
>   if (new_buffer == NULL) {
> throw std::bad_alloc();
>   }
>   rBase_ = new_buffer + (rBase_ - buffer_);
>   rBound_ = new_buffer + (rBound_ - buffer_);
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
>   buffer_ = new_buffer;
>   bufferSize_ = new_size;
> {code}
> If old bufferSize_ is lager than 2gb, then calculating new size will overflow.
> i.e. if bufferSize_ = 3355443200, then new buffer size will be 2415919104, 
> which is less than old size.
> However, 
> {code}
> avail = available_write() + (new_size - bufferSize_) 
> {code}
> overflows again, so we will end up with an shrinked buffer.
> What is worse is that after
> {code}
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
> {code}
> wBase_ stays the same, but wBound_ becomes lower than it. What happens next 
> is that   uint32_t avail = available_write() may overflow every time 
> subsequently. and thrift writes to unknown memory.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (THRIFT-3821) TMemoryBuffer buffer may overflow when resizing

2017-08-10 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/THRIFT-3821?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16122168#comment-16122168
 ] 

ASF GitHub Bot commented on THRIFT-3821:


Github user jeking3 commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1326#discussion_r132545680
  
--- Diff: lib/cpp/src/thrift/transport/TBufferTransports.cpp ---
@@ -361,9 +361,12 @@ void TMemoryBuffer::ensureCanWrite(uint32_t len) {
   }
 
   // Grow the buffer as necessary.
-  uint32_t new_size = bufferSize_;
+  uint64_t new_size = bufferSize_;
   while (len > avail) {
 new_size = new_size > 0 ? new_size * 2 : 1;
+if (new_size > std::numeric_limits::max()) {
+  throw TTransportException("Internal buffer size exceeded 2GB");
--- End diff --

I would prefer to see a TTransportException with an error type BAD_ARGS 
here, however it looks like error types are not being used elsewhere in this 
module.


> TMemoryBuffer buffer may overflow when resizing
> ---
>
> Key: THRIFT-3821
> URL: https://issues.apache.org/jira/browse/THRIFT-3821
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Library
>Affects Versions: 0.9.3
>Reporter: Huaisi Xu
>
> In ensurecanwrite():
> {code}
>   uint32_t new_size = bufferSize_;
>   while (len > avail) {
> new_size = new_size > 0 ? new_size * 2 : 1;
> avail = available_write() + (new_size - bufferSize_);
>   }
>   // Allocate into a new pointer so we don't bork ours if it fails.
>   uint8_t* new_buffer = static_cast(std::realloc(buffer_, 
> new_size));
>   if (new_buffer == NULL) {
> throw std::bad_alloc();
>   }
>   rBase_ = new_buffer + (rBase_ - buffer_);
>   rBound_ = new_buffer + (rBound_ - buffer_);
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
>   buffer_ = new_buffer;
>   bufferSize_ = new_size;
> {code}
> If old bufferSize_ is lager than 2gb, then calculating new size will overflow.
> i.e. if bufferSize_ = 3355443200, then new buffer size will be 2415919104, 
> which is less than old size.
> However, 
> {code}
> avail = available_write() + (new_size - bufferSize_) 
> {code}
> overflows again, so we will end up with an shrinked buffer.
> What is worse is that after
> {code}
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
> {code}
> wBase_ stays the same, but wBound_ becomes lower than it. What happens next 
> is that   uint32_t avail = available_write() may overflow every time 
> subsequently. and thrift writes to unknown memory.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (THRIFT-3821) TMemoryBuffer buffer may overflow when resizing

2017-08-04 Thread Anh Le (JIRA) 

[ 
https://issues.apache.org/jira/browse/THRIFT-3821?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16114252#comment-16114252
 ] 

Anh Le commented on THRIFT-3821:


[~HuaisiXu] Regarding this issue, I think we may have the same thing with the 
following 
[code](https://github.com/apache/thrift/blob/master/lib/cpp/src/thrift/transport/TBufferTransports.cpp#L227):

{{void TFramedTransport::writeSlow(const uint8_t* buf, uint32_t len) {
  // Double buffer size until sufficient.
  uint32_t have = static_cast(wBase_ - wBuf_.get());
  uint32_t new_size = wBufSize_;
  if (len + have < have /* overflow */ || len + have > 0x7fff) {
throw TTransportException(TTransportException::BAD_ARGS,
  "Attempted to write over 2 GB to 
TFramedTransport.");
  }
  while (new_size < len + have) {
new_size = new_size > 0 ? new_size * 2 : 1;
  }
  // ...
}
}}


> TMemoryBuffer buffer may overflow when resizing
> ---
>
> Key: THRIFT-3821
> URL: https://issues.apache.org/jira/browse/THRIFT-3821
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Library
>Affects Versions: 0.9.3
>Reporter: Huaisi Xu
>
> In ensurecanwrite():
> {code}
>   uint32_t new_size = bufferSize_;
>   while (len > avail) {
> new_size = new_size > 0 ? new_size * 2 : 1;
> avail = available_write() + (new_size - bufferSize_);
>   }
>   // Allocate into a new pointer so we don't bork ours if it fails.
>   uint8_t* new_buffer = static_cast(std::realloc(buffer_, 
> new_size));
>   if (new_buffer == NULL) {
> throw std::bad_alloc();
>   }
>   rBase_ = new_buffer + (rBase_ - buffer_);
>   rBound_ = new_buffer + (rBound_ - buffer_);
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
>   buffer_ = new_buffer;
>   bufferSize_ = new_size;
> {code}
> If old bufferSize_ is lager than 2gb, then calculating new size will overflow.
> i.e. if bufferSize_ = 3355443200, then new buffer size will be 2415919104, 
> which is less than old size.
> However, 
> {code}
> avail = available_write() + (new_size - bufferSize_) 
> {code}
> overflows again, so we will end up with an shrinked buffer.
> What is worse is that after
> {code}
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
> {code}
> wBase_ stays the same, but wBound_ becomes lower than it. What happens next 
> is that   uint32_t avail = available_write() may overflow every time 
> subsequently. and thrift writes to unknown memory.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (THRIFT-3821) TMemoryBuffer buffer may overflow when resizing

2017-08-03 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/THRIFT-3821?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16113826#comment-16113826
 ] 

ASF GitHub Bot commented on THRIFT-3821:


GitHub user asuhan opened a pull request:

https://github.com/apache/thrift/pull/1326

THRIFT-3821

Going over 2 GB is quite possible when using Thrift for serialization. We 
can detect the condition and throw an exception.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/asuhan/thrift THRIFT-3821

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/thrift/pull/1326.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1326


commit 314a0b30c59b0e8b314dc6c82061eecabeb10e14
Author: Alex Şuhan 
Date:   2017-08-03T18:45:31Z

THRIFT-3821 Test for overflow on buffer resize in TMemoryBuffer

commit 0fe1997d53020980abdb6f43843206a289a8e24d
Author: Alex Şuhan 
Date:   2017-08-03T19:28:17Z

THRIFT-3821 Check for overflow on buffer resize in TMemoryBuffer




> TMemoryBuffer buffer may overflow when resizing
> ---
>
> Key: THRIFT-3821
> URL: https://issues.apache.org/jira/browse/THRIFT-3821
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Library
>Affects Versions: 0.9.3
>Reporter: Huaisi Xu
>
> In ensurecanwrite():
> {code}
>   uint32_t new_size = bufferSize_;
>   while (len > avail) {
> new_size = new_size > 0 ? new_size * 2 : 1;
> avail = available_write() + (new_size - bufferSize_);
>   }
>   // Allocate into a new pointer so we don't bork ours if it fails.
>   uint8_t* new_buffer = static_cast(std::realloc(buffer_, 
> new_size));
>   if (new_buffer == NULL) {
> throw std::bad_alloc();
>   }
>   rBase_ = new_buffer + (rBase_ - buffer_);
>   rBound_ = new_buffer + (rBound_ - buffer_);
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
>   buffer_ = new_buffer;
>   bufferSize_ = new_size;
> {code}
> If old bufferSize_ is lager than 2gb, then calculating new size will overflow.
> i.e. if bufferSize_ = 3355443200, then new buffer size will be 2415919104, 
> which is less than old size.
> However, 
> {code}
> avail = available_write() + (new_size - bufferSize_) 
> {code}
> overflows again, so we will end up with an shrinked buffer.
> What is worse is that after
> {code}
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
> {code}
> wBase_ stays the same, but wBound_ becomes lower than it. What happens next 
> is that   uint32_t avail = available_write() may overflow every time 
> subsequently. and thrift writes to unknown memory.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (THRIFT-3821) TMemoryBuffer buffer may overflow when resizing

2017-08-03 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/THRIFT-3821?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16113824#comment-16113824
 ] 

ASF GitHub Bot commented on THRIFT-3821:


Github user asuhan closed the pull request at:

https://github.com/apache/thrift/pull/1325


> TMemoryBuffer buffer may overflow when resizing
> ---
>
> Key: THRIFT-3821
> URL: https://issues.apache.org/jira/browse/THRIFT-3821
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Library
>Affects Versions: 0.9.3
>Reporter: Huaisi Xu
>
> In ensurecanwrite():
> {code}
>   uint32_t new_size = bufferSize_;
>   while (len > avail) {
> new_size = new_size > 0 ? new_size * 2 : 1;
> avail = available_write() + (new_size - bufferSize_);
>   }
>   // Allocate into a new pointer so we don't bork ours if it fails.
>   uint8_t* new_buffer = static_cast(std::realloc(buffer_, 
> new_size));
>   if (new_buffer == NULL) {
> throw std::bad_alloc();
>   }
>   rBase_ = new_buffer + (rBase_ - buffer_);
>   rBound_ = new_buffer + (rBound_ - buffer_);
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
>   buffer_ = new_buffer;
>   bufferSize_ = new_size;
> {code}
> If old bufferSize_ is lager than 2gb, then calculating new size will overflow.
> i.e. if bufferSize_ = 3355443200, then new buffer size will be 2415919104, 
> which is less than old size.
> However, 
> {code}
> avail = available_write() + (new_size - bufferSize_) 
> {code}
> overflows again, so we will end up with an shrinked buffer.
> What is worse is that after
> {code}
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
> {code}
> wBase_ stays the same, but wBound_ becomes lower than it. What happens next 
> is that   uint32_t avail = available_write() may overflow every time 
> subsequently. and thrift writes to unknown memory.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (THRIFT-3821) TMemoryBuffer buffer may overflow when resizing

2017-08-03 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/THRIFT-3821?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16113381#comment-16113381
 ] 

ASF GitHub Bot commented on THRIFT-3821:


GitHub user asuhan opened a pull request:

https://github.com/apache/thrift/pull/1325

THRIFT-3821: Check for overflow on buffer resize in TMemoryBuffer

Going over 2 GB is quite possible when using Thrift for serialization. We 
can detect the condition and throw an exception.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/asuhan/thrift THRIFT-3821

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/thrift/pull/1325.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1325


commit 457b7f5be06c57e8a0f0adf33c3c43d031e21991
Author: Alex Şuhan 
Date:   2017-08-03T18:45:31Z

THRIFT-3821 Test for overflow on buffer resize in TMemoryBuffer

commit af851cb570c70cef833d764dbfacfc5961031f86
Author: Alex Şuhan 
Date:   2017-08-03T19:28:17Z

THRIFT-3821 Check for overflow on buffer resize in TMemoryBuffer




> TMemoryBuffer buffer may overflow when resizing
> ---
>
> Key: THRIFT-3821
> URL: https://issues.apache.org/jira/browse/THRIFT-3821
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Library
>Affects Versions: 0.9.3
>Reporter: Huaisi Xu
>
> In ensurecanwrite():
> {code}
>   uint32_t new_size = bufferSize_;
>   while (len > avail) {
> new_size = new_size > 0 ? new_size * 2 : 1;
> avail = available_write() + (new_size - bufferSize_);
>   }
>   // Allocate into a new pointer so we don't bork ours if it fails.
>   uint8_t* new_buffer = static_cast(std::realloc(buffer_, 
> new_size));
>   if (new_buffer == NULL) {
> throw std::bad_alloc();
>   }
>   rBase_ = new_buffer + (rBase_ - buffer_);
>   rBound_ = new_buffer + (rBound_ - buffer_);
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
>   buffer_ = new_buffer;
>   bufferSize_ = new_size;
> {code}
> If old bufferSize_ is lager than 2gb, then calculating new size will overflow.
> i.e. if bufferSize_ = 3355443200, then new buffer size will be 2415919104, 
> which is less than old size.
> However, 
> {code}
> avail = available_write() + (new_size - bufferSize_) 
> {code}
> overflows again, so we will end up with an shrinked buffer.
> What is worse is that after
> {code}
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
> {code}
> wBase_ stays the same, but wBound_ becomes lower than it. What happens next 
> is that   uint32_t avail = available_write() may overflow every time 
> subsequently. and thrift writes to unknown memory.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (THRIFT-3821) TMemoryBuffer buffer may overflow when resizing

2016-06-21 Thread James E. King, III (JIRA) 

[ 
https://issues.apache.org/jira/browse/THRIFT-3821?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15343218#comment-15343218
 ] 

James E. King, III commented on THRIFT-3821:


Transferring 2GB over thrift seems like an unlikely use case... I am going to 
lower the priority of this one down from critical.

> TMemoryBuffer buffer may overflow when resizing
> ---
>
> Key: THRIFT-3821
> URL: https://issues.apache.org/jira/browse/THRIFT-3821
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Library
>Affects Versions: 0.9.3
>Reporter: Huaisi Xu
>Priority: Critical
>
> In ensurecanwrite():
> {code}
>   uint32_t new_size = bufferSize_;
>   while (len > avail) {
> new_size = new_size > 0 ? new_size * 2 : 1;
> avail = available_write() + (new_size - bufferSize_);
>   }
>   // Allocate into a new pointer so we don't bork ours if it fails.
>   uint8_t* new_buffer = static_cast(std::realloc(buffer_, 
> new_size));
>   if (new_buffer == NULL) {
> throw std::bad_alloc();
>   }
>   rBase_ = new_buffer + (rBase_ - buffer_);
>   rBound_ = new_buffer + (rBound_ - buffer_);
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
>   buffer_ = new_buffer;
>   bufferSize_ = new_size;
> {code}
> If old bufferSize_ is lager than 2gb, then calculating new size will overflow.
> i.e. if bufferSize_ = 3355443200, then new buffer size will be 2415919104, 
> which is less than old size.
> However, 
> {code}
> avail = available_write() + (new_size - bufferSize_) 
> {code}
> overflows again, so we will end up with an shrinked buffer.
> What is worse is that after
> {code}
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
> {code}
> wBase_ stays the same, but wBound_ becomes lower than it. What happens next 
> is that   uint32_t avail = available_write() may overflow every time 
> subsequently. and thrift writes to unknown memory.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (THRIFT-3821) TMemoryBuffer buffer may overflow when resizing

2016-06-14 Thread James E. King, III (JIRA) 

[ 
https://issues.apache.org/jira/browse/THRIFT-3821?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15330589#comment-15330589
 ] 

James E. King, III commented on THRIFT-3821:


How likely is this to occur in the field?  It requires allocating 2GB of memory 
with the class; if this is not a normal use case it may not be considered 
critical; I wouldn't want to block 0.10.0 for this if it is not likely to occur.

> TMemoryBuffer buffer may overflow when resizing
> ---
>
> Key: THRIFT-3821
> URL: https://issues.apache.org/jira/browse/THRIFT-3821
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Library
>Affects Versions: 0.9.3
>Reporter: Huaisi Xu
>Priority: Critical
>
> In ensurecanwrite():
> {code}
>   uint32_t new_size = bufferSize_;
>   while (len > avail) {
> new_size = new_size > 0 ? new_size * 2 : 1;
> avail = available_write() + (new_size - bufferSize_);
>   }
>   // Allocate into a new pointer so we don't bork ours if it fails.
>   uint8_t* new_buffer = static_cast(std::realloc(buffer_, 
> new_size));
>   if (new_buffer == NULL) {
> throw std::bad_alloc();
>   }
>   rBase_ = new_buffer + (rBase_ - buffer_);
>   rBound_ = new_buffer + (rBound_ - buffer_);
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
>   buffer_ = new_buffer;
>   bufferSize_ = new_size;
> {code}
> If old bufferSize_ is lager than 2gb, then calculating new size will overflow.
> i.e. if bufferSize_ = 3355443200, then new buffer size will be 2415919104, 
> which is less than old size.
> However, 
> {code}
> avail = available_write() + (new_size - bufferSize_) 
> {code}
> overflows again, so we will end up with an shrinked buffer.
> What is worse is that after
> {code}
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
> {code}
> wBase_ stays the same, but wBound_ becomes lower than it. What happens next 
> is that   uint32_t avail = available_write() may overflow every time 
> subsequently. and thrift writes to unknown memory.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (THRIFT-3821) TMemoryBuffer buffer may overflow when resizing

2016-05-10 Thread Huaisi Xu (JIRA) 

[ 
https://issues.apache.org/jira/browse/THRIFT-3821?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15279305#comment-15279305
 ] 

Huaisi Xu commented on THRIFT-3821:
---

Thanks for reformatting this for me.

May I ask how you plan to address this? check overflow before resizing?

> TMemoryBuffer buffer may overflow when resizing
> ---
>
> Key: THRIFT-3821
> URL: https://issues.apache.org/jira/browse/THRIFT-3821
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Library
>Affects Versions: 0.9.3
>Reporter: Huaisi Xu
>Priority: Critical
>
> In ensurecanwrite():
> {code}
>   uint32_t new_size = bufferSize_;
>   while (len > avail) {
> new_size = new_size > 0 ? new_size * 2 : 1;
> avail = available_write() + (new_size - bufferSize_);
>   }
>   // Allocate into a new pointer so we don't bork ours if it fails.
>   uint8_t* new_buffer = static_cast(std::realloc(buffer_, 
> new_size));
>   if (new_buffer == NULL) {
> throw std::bad_alloc();
>   }
>   rBase_ = new_buffer + (rBase_ - buffer_);
>   rBound_ = new_buffer + (rBound_ - buffer_);
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
>   buffer_ = new_buffer;
>   bufferSize_ = new_size;
> {code}
> If old bufferSize_ is lager than 2gb, then calculating new size will overflow.
> i.e. if bufferSize_ = 3355443200, then new buffer size will be 2415919104, 
> which is less than old size.
> However, 
> {code}
> avail = available_write() + (new_size - bufferSize_) 
> {code}
> overflows again, so we will end up with an shrinked buffer.
> What is worse is that after
> {code}
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
> {code}
> wBase_ stays the same, but wBound_ becomes lower than it. What happens next 
> is that   uint32_t avail = available_write() may overflow every time 
> subsequently. and thrift writes to unknown memory.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)