CVE-2021-24036: Integer Overflow or Wraparound (CWE-190)
value: Passing an attacker controlled size when creating an IOBuf could cause integer overflow, leading to an out of bounds write on the heap with the possibility of remote code execution. This issue affects versions of folly prior to v2021.07.22.00. This issue affects HHVM versions prior to 4.80.5, all versions between 4.81.0 and 4.102.1, all versions between 4.103.0 and 4.113.0, and versions 4.114.0, 4.115.0, 4.116.0, 4.117.0, 4.118.0 and 4.118.1.
CWE description: The product performs a calculation that can
         produce an integer overflow or wraparound when the logic
         assumes that the resulting value will always be larger than
         the original value. This occurs when an integer value is
         incremented to a value that is too large to store in the
         associated representation. When this occurs, the value may
         become a very small or negative number.
CVSS v3 base score: 9.8
Published: 2021-07-23T01:15Z
Repository: https://github.com/facebook/folly
Language: C++
Files changed: folly/io/IOBuf.cpp
Commit date: 2021-07-20 09:06:10 -0700
Commit message: [folly] Add additional overflow checks to IOBuf - CVE-2021-24036

Summary:
As per title

CVE-2021-24036

Reviewed By: jan

Differential Revision: D27938605

fbshipit-source-id: 7481c54ae6fbb7b67b15b3631d5357c2f7043f9c

Diff: diff --git a/folly/io/IOBuf.cpp b/folly/io/IOBuf.cpp
index 1b3db0ee3..2e0cb31c4 100644
--- a/folly/io/IOBuf.cpp
+++ b/folly/io/IOBuf.cpp
@@ -21,19 +21,21 @@
 #include <folly/io/IOBuf.h>
 
 #include <cassert>
 #include <cstdint>
 #include <cstdlib>
+#include <limits>
 #include <stdexcept>
 
 #include <folly/Conv.h>
 #include <folly/Likely.h>
 #include <folly/Memory.h>
 #include <folly/ScopeGuard.h>
 #include <folly/hash/SpookyHashV2.h>
 #include <folly/io/Cursor.h>
 #include <folly/lang/Align.h>
+#include <folly/lang/CheckedMath.h>
 #include <folly/lang/Exception.h>
 #include <folly/memory/Malloc.h>
 #include <folly/memory/SanitizeAddress.h>
 
 /*
@@ -82,11 +84,12 @@ enum : std::size_t {
   //
   // (The size of 1k is largely just a guess here.  We could could probably do
   // benchmarks of real applications to see if adjusting this number makes a
   // difference.  Callers that know their exact use case can also explicitly
   // call createCombined() or createSeparate().)
-  kDefaultCombinedBufSize = 1024
+  kDefaultCombinedBufSize = 1024,
+  kMaxIOBufSize = std::numeric_limits<size_t>::max() >> 1,
 };
 
 // Helper function for IOBuf::takeOwnership()
 // The user's free function is not allowed to throw.
 // (We are already in the middle of throwing an exception, so
@@ -185,10 +188,13 @@ void IOBuf::SharedInfo::releaseStorage(SharedInfo* info) noexcept {
     IOBuf::releaseStorage(&storage->hs, kSharedInfoInUse);
   }
 }
 
 void* IOBuf::operator new(size_t size) {
+  if (size > kMaxIOBufSize) {
+    throw_exception<std::bad_alloc>();
+  }
   size_t fullSize = offsetof(HeapStorage, buf) + size;
   auto storage = static_cast<HeapStorage*>(checkedMalloc(fullSize));
 
   new (&storage->prefix) HeapPrefix(kIOBufInUse, fullSize);
 
@@ -295,10 +301,14 @@ IOBuf::IOBuf(
     std::size_t headroom,
     std::size_t minTailroom)
     : IOBuf(op, br.data(), br.size(), headroom, minTailroom) {}
 
 unique_ptr<IOBuf> IOBuf::create(std::size_t capacity) {
+  if (capacity > kMaxIOBufSize) {
+    throw_exception<std::bad_alloc>();
+  }
+
   // For smaller-sized buffers, allocate the IOBuf, SharedInfo, and the buffer
   // all with a single allocation.
   //
   // We don't do this for larger buffers since it can be wasteful if the user
   // needs to reallocate the buffer but keeps using the same IOBuf object.
@@ -326,10 +336,14 @@ unique_ptr<IOBuf> IOBuf::create(std::size_t capacity) {
 
   return createSeparate(capacity);
 }
 
 unique_ptr<IOBuf> IOBuf::createCombined(std::size_t capacity) {
+  if (capacity > kMaxIOBufSize) {
+    throw_exception<std::bad_alloc>();
+  }
+
   // To save a memory allocation, allocate space for the IOBuf object, the
   // SharedInfo struct, and the data itself all with a single call to malloc().
   size_t requiredStorage = offsetof(HeapFullStorage, align) + capacity;
   size_t mallocSize = goodMallocSize(requiredStorage);
   auto storage = static_cast<HeapFullStorage*>(checkedMalloc(mallocSize));
@@ -454,10 +468,14 @@ unique_ptr<IOBuf> IOBuf::takeOwnership(
     std::size_t length,
     FreeFunction freeFn,
     void* userData,
     bool freeOnError,
     TakeOwnershipOption option) {
+  if (capacity > kMaxIOBufSize) {
+    throw_exception<std::bad_alloc>();
+  }
+
   // do not allow only user data without a freeFn
   // since we use that for folly::sizedFree
 
   DCHECK(
       !userData || (userData && freeFn) ||
@@ -1004,12 +1022,17 @@ void IOBuf::decrementRefcount() noexcept {
     }
   }
 }
 
 void IOBuf::reserveSlow(std::size_t minHeadroom, std::size_t minTailroom) {
-  size_t newCapacity = (size_t)length_ + minHeadroom + minTailroom;
-  DCHECK_LT(newCapacity, UINT32_MAX);
+  size_t newCapacity = length_;
+  if (!checked_add(&newCapacity, newCapacity, minHeadroom) ||
+      !checked_add(&newCapacity, newCapacity, minTailroom) ||
+      newCapacity > kMaxIOBufSize) {
+    // overflow
+    throw_exception<std::bad_alloc>();
+  }
 
   // reserveSlow() is dangerous if anyone else is sharing the buffer, as we may
   // reallocate and free the original buffer.  It should only ever be called if
   // we are the only user of the buffer.
   DCHECK(!isSharedOne());
@@ -1156,10 +1179,14 @@ void IOBuf::freeExtBuffer() noexcept {
 void IOBuf::allocExtBuffer(
     std::size_t minCapacity,
     uint8_t** bufReturn,
     SharedInfo** infoReturn,
     std::size_t* capacityReturn) {
+  if (minCapacity > kMaxIOBufSize) {
+    throw_exception<std::bad_alloc>();
+  }
+
   size_t mallocSize = goodExtBufferSize(minCapacity);
   auto buf = static_cast<uint8_t*>(checkedMalloc(mallocSize));
   initExtBuffer(buf, mallocSize, infoReturn, capacityReturn);
 
   // the userData and the freeFn are nullptr here
@@ -1171,10 +1198,14 @@ void IOBuf::allocExtBuffer(
 
   *bufReturn = buf;
 }
 
 size_t IOBuf::goodExtBufferSize(std::size_t minCapacity) {
+  if (minCapacity > kMaxIOBufSize) {
+    throw_exception<std::bad_alloc>();
+  }
+
   // Determine how much space we should allocate.  We'll store the SharedInfo
   // for the external buffer just after the buffer itself.  (We store it just
   // after the buffer rather than just before so that the code can still just
   // use free(buf_) to free the buffer.)
   size_t minSize = static_cast<size_t>(minCapacity) + sizeof(SharedInfo);

Vulnerable code:   kDefaultCombinedBufSize = 1024
  size_t newCapacity = (size_t)length_ + minHeadroom + minTailroom;
  DCHECK_LT(newCapacity, UINT32_MAX);
Fixed code: #include <limits>
#include <folly/lang/CheckedMath.h>
  kDefaultCombinedBufSize = 1024,
  kMaxIOBufSize = std::numeric_limits<size_t>::max() >> 1,
  if (size > kMaxIOBufSize) {
    throw_exception<std::bad_alloc>();
  }
  if (capacity > kMaxIOBufSize) {
    throw_exception<std::bad_alloc>();
  }

  if (capacity > kMaxIOBufSize) {
    throw_exception<std::bad_alloc>();
  }

  if (capacity > kMaxIOBufSize) {
    throw_exception<std::bad_alloc>();
  }

  size_t newCapacity = length_;
  if (!checked_add(&newCapacity, newCapacity, minHeadroom) ||
      !checked_add(&newCapacity, newCapacity, minTailroom) ||
      newCapacity > kMaxIOBufSize) {
    // overflow
    throw_exception<std::bad_alloc>();
  }
  if (minCapacity > kMaxIOBufSize) {
    throw_exception<std::bad_alloc>();
  }

  if (minCapacity > kMaxIOBufSize) {
    throw_exception<std::bad_alloc>();
  }