[ https://issues.apache.org/jira/browse/THRIFT-3821?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jens Geyer updated THRIFT-3821: ------------------------------- Description: 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. was: 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. > 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<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.3.4#6332)