security fixes

https://hhvm.com/blog/2021/02/25/security-update.html
facebook · Feb 26, 2021 · 08193b7 · 08193b7
1 parent 98b84ab
commit 08193b7
Show file tree

Hide file tree

Showing 29 changed files with 3,326 additions and 43 deletions.
diff --git a/hphp/runtime/base/mem-file.cpp b/hphp/runtime/base/mem-file.cpp
@@ -109,13 +109,15 @@ bool MemFile::closeImpl() {
 int64_t MemFile::readImpl(char *buffer, int64_t length) {
   assertx(m_len != -1);
   assertx(length > 0);
+  assertx(m_cursor >= 0);
   int64_t remaining = m_len - m_cursor;
   if (remaining < length) length = remaining;
   if (length > 0) {
     memcpy(buffer, (const void *)(m_data + m_cursor), length);
+    m_cursor += length;
+    return length;
   }
-  m_cursor += length;
-  return length;
+  return 0;
 }
 
 int MemFile::getc() {
@@ -126,7 +128,7 @@ int MemFile::getc() {
 bool MemFile::seek(int64_t offset, int whence /* = SEEK_SET */) {
   assertx(m_len != -1);
   if (whence == SEEK_CUR) {
-    if (offset > 0 && offset < bufferedLen()) {
+    if (offset >= 0 && offset < bufferedLen()) {
       setReadPosition(getReadPosition() + offset);
       setPosition(getPosition() + offset);
       return true;
@@ -139,10 +141,13 @@ bool MemFile::seek(int64_t offset, int whence /* = SEEK_SET */) {
   setWritePosition(0);
   setReadPosition(0);
   if (whence == SEEK_SET) {
+    if (offset < 0) return false;
     m_cursor = offset;
-  } else {
-    assertx(whence == SEEK_END);
+  } else if (whence == SEEK_END) {
+    if (m_len + offset < 0) return false;
     m_cursor = m_len + offset;
+  } else {
+    return false;
   }
   setPosition(m_cursor);
   return true;

diff --git a/hphp/runtime/base/preg.cpp b/hphp/runtime/base/preg.cpp
@@ -1925,6 +1925,9 @@ String preg_quote(const String& str,
 
   /* Allocate enough memory so that even if each character
      is quoted, we won't run out of room */
+  static_assert(
+    (StringData::MaxSize * 4 + 1) < std::numeric_limits<int64_t>::max()
+  );
   String ret(4 * str.size() + 1, ReserveString);
   char* out_str = ret.mutableData();
 

diff --git a/hphp/runtime/base/string-data-inl.h b/hphp/runtime/base/string-data-inl.h
@@ -66,7 +66,7 @@ inline void StringData::invalidateHash() {
   assertx(checkSane());
 }
 
-inline void StringData::setSize(int len) {
+inline void StringData::setSize(int64_t len) {
   assertx(!isImmutable() && !hasMultipleRefs());
   assertx(len >= 0 && len <= capacity());
   mutableData()[len] = 0;
@@ -94,7 +94,7 @@ inline char* StringData::mutableData() const {
   return const_cast<char*>(data());
 }
 
-inline int StringData::size() const { return m_len; }
+inline int64_t StringData::size() const { return m_len; }
 inline bool StringData::empty() const { return size() == 0; }
 inline uint32_t StringData::capacity() const {
   assertx(isRefCounted());
@@ -255,4 +255,3 @@ struct string_data_lti {
 //////////////////////////////////////////////////////////////////////
 
 }
-
diff --git a/hphp/runtime/base/string-data.h b/hphp/runtime/base/string-data.h
@@ -294,7 +294,7 @@ struct StringData final : MaybeCountable,
    * Pre: !hasMultipleRefs()
    */
   void invalidateHash();
-  void setSize(int len);
+  void setSize(int64_t len);
 
   /*
    * StringData should not generally be allocated on the stack,
@@ -327,10 +327,9 @@ struct StringData final : MaybeCountable,
   /*
    * Accessor for the length of a string.
    *
-   * Note: size() returns a signed int for historical reasons.  It is
-   * guaranteed to be in the range (0 <= size() <= MaxSize)
+   * Note: size() is guaranteed to be >= 0 and <= MaxSize.
    */
-  int size() const;
+  int64_t size() const;
 
   /*
    * Returns: size() == 0
@@ -700,4 +699,3 @@ template<> class FormatValue<HPHP::StringData*> {
 }
 
 #include "hphp/runtime/base/string-data-inl.h"
-
diff --git a/hphp/runtime/base/type-string.h b/hphp/runtime/base/type-string.h
@@ -195,7 +195,7 @@ struct String {
   }
 
 public:
-  const String& setSize(int len) {
+  const String& setSize(int64_t len) {
     assertx(m_str);
     m_str->setSize(len);
     return *this;
@@ -224,10 +224,10 @@ struct String {
   bool empty() const {
     return m_str ? m_str->empty() : true;
   }
-  int size() const {
+  int64_t size() const {
     return m_str ? m_str->size() : 0;
   }
-  int length() const {
+  int64_t length() const {
     return m_str ? m_str->size() : 0;
   }
   uint32_t capacity() const {
@@ -591,4 +591,3 @@ template<> class FormatValue<HPHP::StaticString> {
   const HPHP::StaticString& m_val;
 };
 }
-
diff --git a/hphp/runtime/ext/gd/ext_gd.cpp b/hphp/runtime/ext/gd/ext_gd.cpp
@@ -7656,6 +7656,7 @@ static int exif_scan_thumbnail(image_info_type *ImageInfo) {
     if (c == 0xFF)
       return 0;
     marker = c;
+    if (ImageInfo->Thumbnail.size - 2 < pos) return 0;
     length = php_jpg_get16(data+pos);
     if (length > ImageInfo->Thumbnail.size || pos >= ImageInfo->Thumbnail.size - length) {
       return 0;

diff --git a/hphp/runtime/ext/hotprofiler/ext_hotprofiler.cpp b/hphp/runtime/ext/hotprofiler/ext_hotprofiler.cpp
@@ -1226,8 +1226,8 @@ struct MemoProfiler final : Profiler {
           if (mme.second.m_return_value != fr) all_same = false;
           count += mme.second.m_count;
           auto ser_len = mme.second.m_return_value.length();
-          min_ser_len = std::min(min_ser_len, ser_len);
-          max_ser_len = std::max(max_ser_len, ser_len);
+          min_ser_len = std::min<int64_t>(min_ser_len, ser_len);
+          max_ser_len = std::max<int64_t>(max_ser_len, ser_len);
           if (mme.second.m_count > 1) any_multiple = true;
         }
         if (!any_multiple && !all_same) continue;

diff --git a/hphp/runtime/ext/mcrypt/ext_mcrypt.cpp b/hphp/runtime/ext/mcrypt/ext_mcrypt.cpp
@@ -656,7 +656,7 @@ Variant HHVM_FUNCTION(mcrypt_generic_init, const Resource& td,
 
   int key_size;
   if (key.size() > max_key_size) {
-    raise_warning("Key size too large; supplied length: %d, max: %d",
+    raise_warning("Key size too large; supplied length: %ld, max: %d",
                     key.size(), max_key_size);
     key_size = max_key_size;
   } else {
@@ -665,10 +665,10 @@ Variant HHVM_FUNCTION(mcrypt_generic_init, const Resource& td,
   memcpy(key_s, key.data(), key.size());
 
   if (iv.size() != iv_size) {
-    raise_warning("Iv size incorrect; supplied length: %d, needed: %d",
+    raise_warning("Iv size incorrect; supplied length: %ld, needed: %d",
                     iv.size(), iv_size);
   }
-  memcpy(iv_s, iv.data(), std::min(iv_size, iv.size()));
+  memcpy(iv_s, iv.data(), std::min<int64_t>(iv_size, iv.size()));
 
   mcrypt_generic_deinit(pm->m_td);
   int result = mcrypt_generic_init(pm->m_td, key_s, key_size, iv_s);

diff --git a/hphp/runtime/ext/openssl/ext_openssl.cpp b/hphp/runtime/ext/openssl/ext_openssl.cpp
@@ -2940,7 +2940,7 @@ static bool php_openssl_validate_iv(
   }
 
   if (piv.size() < iv_required_len) {
-    raise_warning("IV passed is only %d bytes long, cipher "
+    raise_warning("IV passed is only %ld bytes long, cipher "
                   "expects an IV of precisely %d bytes, padding with \\0",
                   piv.size(), iv_required_len);
     memcpy(iv_new, piv.data(), piv.size());
@@ -2949,7 +2949,7 @@ static bool php_openssl_validate_iv(
     return true;
   }
 
-  raise_warning("IV passed is %d bytes long which is longer than the %d "
+  raise_warning("IV passed is %ld bytes long which is longer than the %d "
                 "expected by selected cipher, truncating", piv.size(),
                 iv_required_len);
   memcpy(iv_new, piv.data(), iv_required_len);

diff --git a/hphp/runtime/ext/sockets/ext_sockets.cpp b/hphp/runtime/ext/sockets/ext_sockets.cpp
@@ -255,7 +255,7 @@ static bool set_sockaddr(sockaddr_storage &sa_storage, req::ptr<Socket> sock,
       sa->sun_family = AF_UNIX;
       if (addr.length() > sizeof(sa->sun_path)) {
         raise_warning(
-          "Unix socket path length (%d) is larger than system limit (%lu)",
+          "Unix socket path length (%ld) is larger than system limit (%lu)",
           addr.length(),
           sizeof(sa->sun_path)
         );

diff --git a/hphp/runtime/ext/std/ext_std_file.cpp b/hphp/runtime/ext/std/ext_std_file.cpp
@@ -464,7 +464,7 @@ Variant HHVM_FUNCTION(fwrite,
   CHECK_HANDLE(handle, f);
   int64_t ret = f->write(data, length);
   if (ret < 0) {
-    raise_notice("fwrite(): send of %d bytes failed with errno=%d %s",
+    raise_notice("fwrite(): send of %ld bytes failed with errno=%d %s",
                  data.size(), errno, folly::errnoStr(errno).c_str());
     ret = 0;
   }

diff --git a/hphp/runtime/ext/std/ext_std_variable.cpp b/hphp/runtime/ext/std/ext_std_variable.cpp
@@ -454,7 +454,7 @@ ALWAYS_INLINE String serialize_impl(const Variant& value,
         lazyClassToStringHelper(value.toLazyClassVal());
       auto const size = str->size();
       if (size >= RuntimeOption::MaxSerializedStringSize) {
-        throw Exception("Size of serialized string (%d) exceeds max", size);
+        throw Exception("Size of serialized string (%ld) exceeds max", size);
       }
       StringBuffer sb;
       sb.append("s:");

diff --git a/hphp/runtime/ext/string/ext_string.cpp b/hphp/runtime/ext/string/ext_string.cpp
@@ -1133,15 +1133,23 @@ TypedValue HHVM_FUNCTION(substr_compare,
     return make_tv<KindOfBoolean>(false);
   }
 
-  int cmp_len = s1_len - offset;
-  if (cmp_len < s2_len) cmp_len = s2_len;
-  if (cmp_len > length) cmp_len = length;
+  auto const cmp_len = std::min(s1_len - offset, std::min(s2_len, length));
 
-  const char *s1 = main_str.data();
-  if (case_insensitivity) {
-    return tvReturn(bstrcasecmp(s1 + offset, cmp_len, str.data(), cmp_len));
-  }
-  return tvReturn(string_ncmp(s1 + offset, str.data(), cmp_len));
+  auto const ret = [&] {
+    const char *s1 = main_str.data();
+    if (case_insensitivity) {
+      return bstrcasecmp(s1 + offset, cmp_len, str.data(), cmp_len);
+    }
+    return string_ncmp(s1 + offset, str.data(), cmp_len);
+  }();
+  if (ret == 0) {
+    auto const m1 = std::min(s1_len - offset, length);
+    auto const m2 = std::min(s2_len, length);
+    if (m1 > m2) return tvReturn(1);
+    if (m1 < m2) return tvReturn(-1);
+    return tvReturn(0);
+  }
+  return tvReturn(ret);
 }
 
 TypedValue HHVM_FUNCTION(strstr,

diff --git a/hphp/runtime/ext/strobelight/ext_strobelight.cpp b/hphp/runtime/ext/strobelight/ext_strobelight.cpp
@@ -110,7 +110,10 @@ bool logToUSDT(const Array& bt) {
         assertx(isStringType(type(file_name)));
         strncpy(frame->file_name,
                 val(file_name).pstr->data(),
-                std::min(val(file_name).pstr->size(), strobelight::kFileNameMax));
+                std::min<int64_t>(
+                  val(file_name).pstr->size(),
+                  strobelight::kFileNameMax
+                ));
         frame->file_name[strobelight::kFileNameMax - 1] = '\0';
       }
 
@@ -119,7 +122,10 @@ bool logToUSDT(const Array& bt) {
         assertx(isStringType(type(class_name)));
         strncpy(frame->class_name,
                 val(class_name).pstr->data(),
-                std::min(val(class_name).pstr->size(), strobelight::kClassNameMax));
+                std::min<int64_t>(
+                  val(class_name).pstr->size(),
+                  strobelight::kClassNameMax
+                ));
         frame->class_name[strobelight::kClassNameMax - 1] = '\0';
       }
 
@@ -128,8 +134,10 @@ bool logToUSDT(const Array& bt) {
         assertx(isStringType(type(function_name)));
         strncpy(frame->function,
                 val(function_name).pstr->data(),
-                std::min(val(function_name).pstr->size(),
-                         strobelight::kFunctionMax));
+                std::min<int64_t>(
+                  val(function_name).pstr->size(),
+                  strobelight::kFunctionMax
+                ));
         frame->function[strobelight::kFunctionMax - 1] = '\0';
       }
 

diff --git a/hphp/test/slow/ext_gd/t79224592.php b/hphp/test/slow/ext_gd/t79224592.php
@@ -0,0 +1,7 @@
+<?hh
+
+<<__EntryPoint>>
+function main() {
+  $x = exif_read_data("data://text/plain;base64,SUkqAAQAAAAAAAIAAAAAAAQAaYcAAAIAAAAiAAAAaYcAAAABAAAEAAAA//4CAAEAAgD/////////////////v///////////AAAAAAAAAAAAAAAAAAAAAQAAAAAAAD4AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAACAAAAAAAAAAAAqAAQAAAAAAAIAAAAIzgcAXgAAKgQAAAACAAQnmIIAAAADAAD/AAAAAAAAAACAA///////AQME4gD+////////AAZgVANoaGhoaGhoaGhoaGhoaGhoaGhoaISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhGhoaGhoaGhoaGhoaGhoaGhoaGhoaGhoaGhoaGhoaGhoaGhoaGhoaGhoaGhoaGhoaGhoaGhoaGhoaGhoaGhoaGhoaGhoaGhoaGhoaGhoaGhoaGhoaGjc3Nzcampqampqampqampqampqampqampqampqampqampqampqampqampqampqampqav/YxQAJ2DEk2D3/2MUACdgxxUD////FAGpqampqagb/////////////AQAGAwTiAP4AampqampqampqampqampqampqampqBv////////////8BAAYDBAMFHv8BAAYA3vnY/////////gAGDwAABgAAAAQAAAAyAACjo6Ojo6Oj4v//Dtj/AQAGaGhoaGhoaGhoaGhoaGhoaGhoaGhoaGhoaGhoaGhoaGhoaGhoaGhoaGhoaGhoaGhoaGhoaGhoaGhoaGhoaGhoaGhoaGhoaGhoaGhoaGhoaGhoaGhoaATY/wsABgME4gD+AAbj3vf7Hf/+AAYBAAD//////wEABgME4gD+AAZgVAME//////8BAAYDBOIA/gAGHv8BAAYA3vnY/wEABgME4gD+AAZgVAME2P8LAAYD/////9jhAAgAAAAAAAAAAATiAP4ABh7/AQAGAN752P8BAAYDBOIA/gAGYFQDBNj/CwAGA//////Y4QAIAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAACmpqampqampqampqampqampqampqampqampqYAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA//////8LAAYDBOIA/gAG4973+x3//gAGBQA4////////////////+fn5+fn5+fn5+fn5+fn5+fn5+fn5+fn5+fn5+fn5+fn5+fn5+fn5+fn5+fn5+fn5+fn5+fn5+fn5+fn5AP4ABuPe91sd//4ABgUAAP//////AQAGAwTiAP4ABmCp/Psn///+AAYA3vf7Hf/+AAYBAAD//3oAampqampqampqampqampqampqampqampqampqampqampqampqampqampqampqampqampqampqampqampqav/Y2QAEANBqampqagb/////////////AQAGAwTiAP4ABlQDBNj/AQAGAwUe/wEABgDe+dj////////+AAbi//8O2P8BAAYDBOIDAAAAJQAEAGmHAAACAAAAIgAAAGmHAAACAAAAFAAAAGmHAAAA/gAGAN7/2P8BAAYDBOIA/gAGYFQDBNj/DgAGAwTiAP4ABuPe901NACoAAAAF+yAA////AQME4gD+////////AAZgVNzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3GpqampqampqampqampqampqampqampqampqampqampqampqampqampqampqampqampqamoG/////////////wEABgME4gD+AGpqampqampqampqampqampqampqagb/////////////AQAGAwQDBR7/AQAGAN752P////////4ABg8AAAYAAAAEAAAAMgAAo6Ojo6Ojo+L//w7Y/wEABgME4gD+AAYAQN7/2P8BAAYDBOIA/gCGYFQDBNj/CwAGAwTiAP7//////////////////////////////////////////////////////////////////////////////////////////////zo6Ojr/B2tqampqampqampqampqampqampqampqampqampqampqampqampqampqampqampqampqampqampqBv////////////8BAAYDBOIA/gAGVElJKgAEAAAAAgACAAAAAAAEAGmHAAAFvwAABagAAABphwgAAgAAABAQEBAQBBAQEABw+f+yAP8EAAAAACkGAwTiAP4ABuP/3vf7SUkqAAQAAAADAAAAAAAAAP//////AAIAAAAAAFsAYgADAAIAAAAqAAQAAAADAAAAAAAA9AAAAP8C/wTiAP4ABmBUAwT///////////////////////////////////////////////////////////////////////////////////////////////////////////8BAAYDBOIA/gAGYFQDBNj/AQAGAwTiAP4ABmBUAwTY/wEABgME4gD+AAYA3vcB+x3//gAGAQDyAP//egAG9/sd//4ABgEAAP//egAG//8AAAAAF+IA/gBJKgAEAAAAAP//oQ7/m4L/////////////AQAGAwTiAP4ABlQDBNj/AQAGAx4F/wEABgDe+dj/AQAGAwTiAP4ABmBUAwTY/wsABgP///////////8B/gAG4v//BgME4gD+AAZgVAME//////n5+fn5+fn5+fn5+fn5+fn5+fn5+fn5+fn5+fn5+fn5+fn//////////////62tra2tra3j3vf7Hf/+AAYFAAD//////wED//////8BAwTiBgME4gD+AAbj3vf7Hf/+AAYBAAD//////wEABgMECgoKCgoKCgoK4gD+AAZgVAME//////8BAAYDBOJbAP4ABmBUAwQt/wEABgME4gD+AAZgVAME2P8BAAYDBOIA/gAGAN73+x3//gAGAQAA//////////////////////sd//4ABgEAAP//egAG9/sd//4ABgEAAP////8BAAYDBOIA/gAGYFQDBNj/AQAGAwTiAP4ABgDe9/sd//4ABgEAAP//egAG9/sd//4ABgEAAP//egAG//8AAAAAF+IA/gBJKgAABgME4gB6AAb3+x3//gAGAQAASUkqAAQAAAAIAAAAAAAGAAAAAAAAAAAAAAAAAAAAAAAADP8GAAAAAAAAAHoGAQAAAAAAAAAAAAAAAP////////////////////////////////////////////////////4ABgDe/9j/AQAGAwTiAP4ABmBUAwTY/wsABgME4gD+AAbj3vf7Hf/+AAYFAAD////v/wEABgMM/uIABAAGYKn8+yf///4ABuL//w7Y/wEABgME4gD+AAYA3v/Y/wEABgME4gD+AAZgVAMEl5eXl5eXl5eXl5eXl5eXl5eXl5eXl5eXl5eXl5eXl5eXl5eXl5eXl5eXl5eXl5eXl5eXl5eXl5eXl5eXl5eXl5eXl5eXl5eXl5eXl5eXl5eXl5eXl5eXl5eXl5eXl5eXl5eXl5eXl5eXl5eXl5eXl9j/CwAGAwTiAP4ABuMj3v4AHQb/+/cFAAD//////wEABgME4gD+AAZgVAME2BQAAABphwAAAP4ABgDe/9j/AQAGAwTiAP4ABmBUAwTY/wsABgME4gD+AAbj3vf7Hf/+AAYFAAD//////wEDBOIA/v///////wAGYFQDBNj/CwAGAwTiAP4ABuPe9/sd//4ABgEAAP//////AQAGAwTiAP4ABmBUAwT///////8BAAYDBOIA/gAGYFQDBNj/AQAGAwTiAP4ABmBUAwTY/wEABgME4gD+AAYA3vf7Hf/+AAYBAAD/////////////////////////egAG9/sd//4ABgEAAP//egAG//8BAAYDBOIA/gAGYFQDBNj/CwAGA//////Y4QgA/+EABv////////4ABuL/AAYFAAD///////8BAAYDBOIADtg=");
+  var_dump($x);
+}