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

["Anh Le (JIRA)" <ji...@apache.org>]

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

Anh Le edited comment on THRIFT-3821 at 8/4/17 11:12 AM:
---------------------------------------------------------

[~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):


{code:cpp}
void TFramedTransport::writeSlow(const uint8_t* buf, uint32_t len) {
  // Double buffer size until sufficient.
  uint32_t have = static_cast<uint32_t>(wBase_ - wBuf_.get());
  uint32_t new_size = wBufSize_;
  if (len + have < have /* overflow */ || len + have > 0x7fffffff) {
    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;
  }
  // ...
}

{code}



was (Author: anhldbk):
[~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<uint32_t>(wBase_ - wBuf_.get());
  uint32_t new_size = wBufSize_;
  if (len + have < have /* overflow */ || len + have > 0x7fffffff) {
    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<uint8_t*>(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)