[THRIFT-3821] TMemoryBuffer buffer may overflow when resizing

• Type: Bug
• Status: Closed
• Priority: Major
• Resolution: Fixed
• Affects Version/s: 0.9.3
• Fix Version/s: 0.11.0
• Component/s: C++ - Library
• Assignee: James E. King III
• Reporter: Huaisi Xu
• Created: 09/May/16 20:47
• Updated: 17/Jun/23 07:54
• Resolved: 13/Aug/17 03:05

Description

In ensurecanwrite():

  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;

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,

avail = available_write() + (new_size - bufferSize_) 

overflows again, so we will end up with an shrinked buffer.

What is worse is that after

  wBase_ = new_buffer + (wBase_ - buffer_);
  wBound_ = new_buffer + new_size;

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.

Issue Links

• is related to: THRIFT-5716 TMemoryBuffer resizing might shrink the buffer size due to uint32_t overflow
• relates to: THRIFT-3166 TBufferBase can wrap the pointer in read, readAll, and write
• links to: GitHub Pull Request #1325
• links to: GitHub Pull Request #1326
• links to: GitHub Pull Request #1369