[
https://issues.apache.org/jira/browse/THRIFT-5716?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Quanlong Huang resolved THRIFT-5716.
------------------------------------
Fix Version/s: 0.19.0
Resolution: Fixed
> TMemoryBuffer resizing might shrink the buffer size due to uint32_t overflow
> ----------------------------------------------------------------------------
>
> Key: THRIFT-5716
> URL: https://issues.apache.org/jira/browse/THRIFT-5716
> Project: Thrift
> Issue Type: Bug
> Components: C++ - Library
> Affects Versions: 0.14.0
> Reporter: Quanlong Huang
> Assignee: Quanlong Huang
> Priority: Critical
> Fix For: 0.19.0
>
> Time Spent: 1h
> Remaining Estimate: 0h
>
> In TMemoryBuffer::ensureCanWrite(), the calculation of 'required_buffer_size'
> might overflow since it's the sum of two uint32_t values. The type of
> 'required_buffer_size' should be uint64_t.
> {code:cpp}
> void TMemoryBuffer::ensureCanWrite(uint32_t len) {
> // Check available space
> uint32_t avail = available_write();
> if (len <= avail) {
> return;
> }
> if (!owner_) {
> throw TTransportException("Insufficient space in external MemoryBuffer");
> }
> // Grow the buffer as necessary.
> const uint32_t current_used = bufferSize_ - avail;
> const uint32_t required_buffer_size = len + current_used; // <-- This could
> overflow
> if (required_buffer_size > maxBufferSize_) {
> throw TTransportException(TTransportException::BAD_ARGS,
> "Internal buffer size overflow when requesting
> a buffer of size " + std::to_string(required_buffer_size));
> }
> // Always grow to the next bigger power of two:
> const double suggested_buffer_size =
> std::exp2(std::ceil(std::log2(required_buffer_size)));
> // Unless the power of two exceeds maxBufferSize_:
> const uint64_t new_size =
> static_cast<uint64_t>((std::min)(suggested_buffer_size,
> static_cast<double>(maxBufferSize_)));
> // Allocate into a new pointer so we don't bork ours if it fails.
> auto* new_buffer = static_cast<uint8_t*>(std::realloc(buffer_,
> static_cast<std::size_t>(new_size)));
> if (new_buffer == nullptr) {
> throw std::bad_alloc();
> }
> rBase_ = new_buffer + (rBase_ - buffer_);
> rBound_ = new_buffer + (rBound_ - buffer_);
> wBase_ = new_buffer + (wBase_ - buffer_);
> wBound_ = new_buffer + new_size;
> // Note: with realloc() we do not need to free the previous buffer:
> buffer_ = new_buffer;
> bufferSize_ = static_cast<uint32_t>(new_size);
> } {code}
> Overflow example:
> {noformat}
> len=1066587646, current_used=4167372953,
> required_buffer_size=938993303{noformat}
> The buffer is not expanded if 'required_buffer_size' overflow.
> TMemoryBuffer::writeSlow() would finally writes to invalid address and crash
> the process:
> {code:cpp}
> void TMemoryBuffer::writeSlow(const uint8_t* buf, uint32_t len) {
> ensureCanWrite(len);
> // Copy into the buffer and increment wBase_.
> memcpy(wBase_, buf, len); // <-- This could crash
> wBase_ += len;
> } {code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
