CVE-2020-1916: Out-of-bounds Write (CWE-787)
value: An incorrect size calculation in ldap_escape may lead to an integer overflow when overly long input is passed in, resulting in an out-of-bounds write. This issue affects HHVM prior to 4.56.2, all versions between 4.57.0 and 4.78.0, 4.79.0, 4.80.0, 4.81.0, 4.82.0, 4.83.0.
CWE description: The product writes data past the end, or before the beginning, of the intended buffer.
CVSS v3 base score: 9.8
Published: 2021-03-10T16:15Z
Repository: https://github.com/facebook/hhvm
Language: C++
Files changed: hphp/runtime/base/preg.cpp, hphp/runtime/base/preg.h, hphp/runtime/base/runtime-option.cpp, hphp/runtime/base/runtime-option.h, hphp/runtime/base/string-util.cpp, hphp/runtime/ext/domdocument/ext_domdocument.h, hphp/runtime/ext/gd/libgd/gd.cpp, hphp/runtime/ext/ldap/ext_ldap.cpp, hphp/runtime/ext/xmlreader/ext_xmlreader.cpp, hphp/runtime/server/admin-request-handler.cpp, hphp/runtime/server/admin-request-handler.h, hphp/test/slow/ext_ldap/t78806688.php, hphp/test/slow/ext_ldap/t78806688.php.expectf, hphp/test/slow/ext_string/t76103217.php, hphp/test/slow/ext_string/t76103217.php.expect, hphp/test/slow/ext_xml/xmlreader_empty_expand.php, hphp/test/slow/ext_xml/xmlreader_empty_expand.php.expectf, hphp/util/light-process.cpp, hphp/util/light-process.h, hphp/zend/crypt-blowfish.cpp, hphp/zend/zend-string.cpp
Commit date: 2020-11-12 15:57:35 -0800
Commit message: security fixes

https://hhvm.com/blog/2020/11/12/security-update.html

Diff: diff --git a/hphp/runtime/base/preg.cpp b/hphp/runtime/base/preg.cpp
index 7e55c5873c80..b766a8bc20ee 100644
--- a/hphp/runtime/base/preg.cpp
+++ b/hphp/runtime/base/preg.cpp
@@ -47,10 +47,11 @@
 #include "hphp/runtime/vm/jit/vtune-jit.h"
 
 #include "hphp/util/logger.h"
 #include "hphp/util/concurrent-scalable-cache.h"
 
+#include <folly/FileUtil.h>
 #include <folly/json.h>
 
 /* Only defined in pcre >= 8.32 */
 #ifndef PCRE_STUDY_JIT_COMPILE
 # define PCRE_STUDY_JIT_COMPILE 0
@@ -215,11 +216,11 @@ public:
   void reinit(CacheKind kind);
   bool find(Accessor& accessor, const StringData* key,
             TempKeyCache& keyCache);
   void insert(Accessor& accessor, const StringData* regex,
               TempKeyCache& keyCache, const pcre_cache_entry* ent);
-  void dump(const std::string& filename);
+  void dump(folly::File& file);
   size_t size() const;
 
 private:
   void clearStatic();
 
@@ -510,16 +511,16 @@ void PCRECache::insert(
       }
       break;
   }
 }
 
-void PCRECache::dump(const std::string& filename) {
-  std::ofstream out(filename.c_str());
+void PCRECache::dump(folly::File& file) {
   switch (m_kind) {
     case CacheKind::Static:
       for (auto& it : *m_staticCache) {
-        out << it.first->data() << "\n";
+        folly::writeFull(file.fd(), it.first->data(), it.first->size());
+        folly::writeFull(file.fd(), "\n", 1);
       }
       break;
     case CacheKind::Lru:
     case CacheKind::Scalable:
       {
@@ -528,16 +529,16 @@ void PCRECache::dump(const std::string& filename) {
           m_lruCache->snapshotKeys(keys);
         } else {
           m_scalableCache->snapshotKeys(keys);
         }
         for (auto& key: keys) {
-          out << key.c_str() << "\n";
+          folly::writeFull(file.fd(), key.data(), key.size());
+          folly::writeFull(file.fd(), "\n", 1);
         }
       }
       break;
   }
-  out.close();
 }
 
 size_t PCRECache::size() const {
   switch (m_kind) {
     case CacheKind::Static:
@@ -570,12 +571,12 @@ void pcre_reinit() {
 }
 
 void pcre_init() {
 }
 
-void pcre_dump_cache(const std::string& filename) {
-  s_pcreCache.dump(filename);
+void pcre_dump_cache(folly::File& file) {
+  s_pcreCache.dump(file);
 }
 
 static pcre_jit_stack* alloc_jit_stack(void* /*data*/) {
   return tl_pcre_globals->jit_stack;
 }
diff --git a/hphp/runtime/base/preg.h b/hphp/runtime/base/preg.h
index b2818790554a..b3b603567885 100644
--- a/hphp/runtime/base/preg.h
+++ b/hphp/runtime/base/preg.h
@@ -16,10 +16,11 @@
 
 #pragma once
 
 #include "hphp/runtime/base/type-string.h"
 
+#include <folly/File.h>
 #include <folly/Optional.h>
 
 #include <cstdint>
 #include <cstddef>
 #include <memory>
@@ -109,13 +110,13 @@ void pcre_reinit();
  * Clean up thread-local PCREs.
  */
 void pcre_session_exit();
 
 /*
- * Dump the contents of the PCRE cache to filename.
+ * Dump the contents of the PCRE cache to the given file.
  */
-void pcre_dump_cache(const std::string& filename);
+void pcre_dump_cache(folly::File& file);
 
 ///////////////////////////////////////////////////////////////////////////////
 // PHP API
 
 Variant preg_grep(const String& pattern, const Array& input, int flags = 0);
diff --git a/hphp/runtime/base/runtime-option.cpp b/hphp/runtime/base/runtime-option.cpp
index 8e249627065f..529061e236f6 100644
--- a/hphp/runtime/base/runtime-option.cpp
+++ b/hphp/runtime/base/runtime-option.cpp
@@ -52,10 +52,11 @@
 #include "hphp/util/current-executable.h" // @donotremove
 #include "hphp/util/file-cache.h"
 #include "hphp/util/gzip.h"
 #include "hphp/util/hardware-counter.h"
 #include "hphp/util/hdf.h"
+#include "hphp/util/light-process.h"
 #include "hphp/util/log-file-flusher.h"
 #include "hphp/util/logger.h"
 #include "hphp/util/network.h"
 #include "hphp/util/numa.h"
 #include "hphp/util/process.h"
@@ -752,10 +753,11 @@ int RuntimeOption::AdminThreadCount = 1;
 bool RuntimeOption::AdminServerEnableSSLWithPlainText = false;
 bool RuntimeOption::AdminServerStatsNeedPassword = true;
 std::string RuntimeOption::AdminPassword;
 std::set<std::string> RuntimeOption::AdminPasswords;
 std::set<std::string> RuntimeOption::HashedAdminPasswords;
+std::string RuntimeOption::AdminDumpPath;
 
 std::string RuntimeOption::ProxyOriginRaw;
 int RuntimeOption::ProxyPercentageRaw = 0;
 int RuntimeOption::ProxyRetry = 3;
 bool RuntimeOption::UseServeURLs;
@@ -2396,10 +2398,12 @@ void RuntimeOption::Load(
     }
     Config::Bind(LightProcessFilePrefix, ini, config,
                  "Server.LightProcessFilePrefix", "./lightprocess");
     Config::Bind(LightProcessCount, ini, config,
                  "Server.LightProcessCount", 0);
+    Config::Bind(LightProcess::g_strictUser, ini, config,
+                 "Server.LightProcessStrictUser", false);
     Config::Bind(ForceServerNameToHeader, ini, config,
                  "Server.ForceServerNameToHeader");
     Config::Bind(AllowDuplicateCookies, ini, config,
                  "Server.AllowDuplicateCookies", false);
     Config::Bind(PathDebug, ini, config, "Server.PathDebug", false);
@@ -2511,10 +2515,12 @@ void RuntimeOption::Load(
                  "AdminServer.StatsNeedPassword", AdminServerStatsNeedPassword);
     AdminPassword = Config::GetString(ini, config, "AdminServer.Password");
     AdminPasswords = Config::GetSet(ini, config, "AdminServer.Passwords");
     HashedAdminPasswords =
       Config::GetSet(ini, config, "AdminServer.HashedPasswords");
+    Config::Bind(AdminDumpPath, ini, config,
+                 "AdminServer.DumpPath", "/tmp/hhvm_admin_dump");
   }
   {
     // Proxy
     Config::Bind(ProxyOriginRaw, ini, config, "Proxy.Origin");
     Config::Bind(ProxyPercentageRaw, ini, config, "Proxy.Percentage", 0);
diff --git a/hphp/runtime/base/runtime-option.h b/hphp/runtime/base/runtime-option.h
index 8b76768c9422..80845c6065e2 100644
--- a/hphp/runtime/base/runtime-option.h
+++ b/hphp/runtime/base/runtime-option.h
@@ -494,10 +494,12 @@ struct RuntimeOption {
   static bool AdminServerStatsNeedPassword;
   static std::string AdminPassword;
   static std::set<std::string> AdminPasswords;
   static std::set<std::string> HashedAdminPasswords;
 
+  static std::string AdminDumpPath;
+
   /*
    * Options related to reverse proxying. ProxyOriginRaw and ProxyPercentageRaw
    * may be mutated by background threads and should only be read or written
    * using the helper functions defined with HttpRequestHandler.
    */
diff --git a/hphp/runtime/base/string-util.cpp b/hphp/runtime/base/string-util.cpp
index 55130bbc916b..3a346487c2c2 100644
--- a/hphp/runtime/base/string-util.cpp
+++ b/hphp/runtime/base/string-util.cpp
@@ -433,11 +433,12 @@ String StringUtil::ROT13(const String& input) {
   return String(string_rot13(input.data(), input.size()),
                 input.size(), AttachString);
 }
 
 String StringUtil::Crypt(const String& input, const char *salt /* = "" */) {
-  if (salt && salt[0] == '\0') {
+  assertx(salt);
+  if (salt[0] == '\0') {
     raise_notice("crypt(): No salt parameter was specified."
       " You must use a randomly generated salt and a strong"
       " hash function to produce a secure hash.");
   }
   return String(string_crypt(input.c_str(), salt), AttachString);
diff --git a/hphp/runtime/ext/domdocument/ext_domdocument.h b/hphp/runtime/ext/domdocument/ext_domdocument.h
index 8709719bc1b5..6b5580f18b1c 100644
--- a/hphp/runtime/ext/domdocument/ext_domdocument.h
+++ b/hphp/runtime/ext/domdocument/ext_domdocument.h
@@ -49,11 +49,13 @@ struct DOMNode {
   }
 
   // for __clone
   DOMNode& operator=(const DOMNode& copy);
 
-  req::ptr<XMLDocumentData> doc() const { return m_node->doc(); }
+  req::ptr<XMLDocumentData> doc() const {
+    return m_node ? m_node->doc() : nullptr;
+  }
   XMLNode node() const { return m_node; }
   xmlNodePtr nodep() const {
     return m_node ? m_node->nodep() : nullptr;
   }
 
diff --git a/hphp/runtime/ext/gd/libgd/gd.cpp b/hphp/runtime/ext/gd/libgd/gd.cpp
index 3b7a1786d2b9..2a1f20a65091 100644
--- a/hphp/runtime/ext/gd/libgd/gd.cpp
+++ b/hphp/runtime/ext/gd/libgd/gd.cpp
@@ -183,12 +183,12 @@ gdImagePtr gdImageCreateTrueColor (int sx, int sy)
     return NULL;
   }
 
   // Check for OOM before doing a potentially large allocation.
   auto allocsz = sizeof(gdImage)
-    + sy * (sizeof(int *) + sizeof(unsigned char *))
-    + sx * sy * (sizeof(int) + sizeof(unsigned char));
+    + (sizeof(int *) + sizeof(unsigned char *)) * sy
+    + (sizeof(int) + sizeof(unsigned char)) * sx * sy;
   if (UNLIKELY(precheckOOM(allocsz))) {
     // Don't throw here because GD might need to do its own cleanup.
     return NULL;
   }
 
diff --git a/hphp/runtime/ext/ldap/ext_ldap.cpp b/hphp/runtime/ext/ldap/ext_ldap.cpp
index f323bb898e78..54d128b5d62b 100644
--- a/hphp/runtime/ext/ldap/ext_ldap.cpp
+++ b/hphp/runtime/ext/ldap/ext_ldap.cpp
@@ -2137,11 +2137,11 @@ String HHVM_FUNCTION(ldap_escape,
     esc[(unsigned char)ignores[i]] = 0;
   }
 
   char hex[] = "0123456789abcdef";
 
-  String result(3 * value.size(), ReserveString);
+  String result(3UL * value.size(), ReserveString);
   char *rdata = result.get()->mutableData(), *r = rdata;
 
   for (int i = 0; i < value.size(); i++) {
     auto c = (unsigned char)value[i];
     if (esc[c]) {
diff --git a/hphp/runtime/ext/xmlreader/ext_xmlreader.cpp b/hphp/runtime/ext/xmlreader/ext_xmlreader.cpp
index e23e7242cdb3..09a6d3cf3bc5 100644
--- a/hphp/runtime/ext/xmlreader/ext_xmlreader.cpp
+++ b/hphp/runtime/ext/xmlreader/ext_xmlreader.cpp
@@ -596,15 +596,15 @@ Variant HHVM_METHOD(XMLReader, expand,
   SYNC_VM_REGS_SCOPED();
 
   if (!basenode.isNull()) {
     auto dombasenode = Native::data<DOMNode>(basenode.toObject());
     doc = dombasenode->doc();
-    docp = doc->docp();
-    if (docp == nullptr) {
+    if (doc == nullptr || doc->docp() == nullptr) {
       raise_warning("Invalid State Error");
       return false;
     }
+    docp = doc->docp();
   }
 
   if (data->m_ptr) {
     xmlNodePtr node = xmlTextReaderExpand(data->m_ptr);
     if (node == nullptr) {
diff --git a/hphp/runtime/server/admin-request-handler.cpp b/hphp/runtime/server/admin-request-handler.cpp
index 4aa13497ca23..1c0a00c0c2b7 100644
--- a/hphp/runtime/server/admin-request-handler.cpp
+++ b/hphp/runtime/server/admin-request-handler.cpp
@@ -74,10 +74,13 @@
 #ifdef ENABLE_EXTENSION_MYSQL
 #include "hphp/runtime/ext/mysql/mysql_stats.h"
 #endif
 
 #include <folly/Conv.h>
+#include <folly/File.h>
+#include <folly/FileUtil.h>
+#include <folly/Optional.h>
 #include <folly/Random.h>
 #include <folly/portability/Unistd.h>
 
 #include <cstdio>
 #include <fstream>
@@ -252,10 +255,38 @@ void AdminRequestHandler::teardownRequest(Transport* transport) noexcept {
   };
   GetAccessLog().log(transport, nullptr);
   WarnIfNotOK(transport);
 }
 
+namespace {
+
+// When this struct is destroyed, it will close the file.
+struct DumpFile {
+  std::string path;
+  folly::File file;
+};
+
+folly::Optional<DumpFile> dump_file(const char* name) {
+  auto const path = folly::sformat("{}/{}", RO::AdminDumpPath, name);
+
+  // mkdir -p the directory prefix of `path`
+  if (FileUtil::mkdir(path) != 0) return folly::none;
+
+  // If remove fails because of a permissions issue, then we won't be
+  // able to open the file for exclusive write below.
+  remove(path.c_str());
+
+  // Create the file, failing if it already exists. Doing so ensures
+  // that we have write access to the file and that no other user does.
+  auto const fd = open(path.c_str(), O_CREAT|O_EXCL|O_RDWR, 0666);
+  if (fd < 0) return folly::none;
+
+  return DumpFile{path, folly::File(fd, /*owns=*/true)};
+}
+
+}
+
 void AdminRequestHandler::handleRequest(Transport *transport) {
   transport->addHeader("Content-Type", "text/plain");
   std::string cmd = transport->getCommand();
 
   do {
@@ -712,14 +743,16 @@ void AdminRequestHandler::handleRequest(Transport *transport) {
     if (strncmp(cmd.c_str(), "static-strings", 14) == 0 &&
         handleStaticStringsRequest(cmd, transport)) {
       break;
     }
     if (strncmp(cmd.c_str(), "dump-static-strings", 19) == 0) {
-      auto filename = transport->getParam("file");
-      if (filename == "") filename = "/tmp/static_strings";
-      handleDumpStaticStringsRequest(cmd, filename);
-      transport->sendString("OK\n");
+      if (auto file = dump_file("static_strings")) {
+        handleDumpStaticStringsRequest(file->file);
+        transport->sendString(folly::sformat("dumped to {}\n", file->path));
+      } else {
+        transport->sendString("Unable to mkdir or file already exists.\n");
+      }
       break;
     }
     if (strncmp(cmd.c_str(), "random-static-strings", 21) == 0) {
       handleRandomStaticStringsRequest(cmd, transport);
       break;
@@ -744,14 +777,16 @@ void AdminRequestHandler::handleRequest(Transport *transport) {
       transport->sendString(size.str());
       break;
     }
 
     if (cmd == "dump-pcre-cache") {
-      auto filename = transport->getParam("file");
-      if (filename == "") filename = "/tmp/pcre_cache";
-      pcre_dump_cache(filename);
-      transport->sendString("OK\n");
+      if (auto file = dump_file("pcre_cache")) {
+        pcre_dump_cache(file->file);
+        transport->sendString(folly::sformat("dumped to {}\n", file->path));
+      } else {
+        transport->sendString("Unable to mkdir or file already exists.\n");
+      }
       break;
     }
 
     if (cmd == "start-stacktrace-profiler") {
       enable_stacktrace_profiler = true;
@@ -1366,17 +1401,15 @@ bool AdminRequestHandler::handleStaticStringsRequest(const std::string& cmd,
 std::string formatStaticString(StringData* str) {
   return folly::sformat(
       "----\n{} bytes\n{}\n", str->size(), str->toCppString());
 }
 
-bool AdminRequestHandler::handleDumpStaticStringsRequest(
-  const std::string& /*cmd*/, const std::string& filename) {
+bool AdminRequestHandler::handleDumpStaticStringsRequest(folly::File& file) {
   auto const& list = lookupDefinedStaticStrings();
-  std::ofstream out(filename.c_str());
-  SCOPE_EXIT { out.close(); };
   for (auto item : list) {
-    out << formatStaticString(item);
+    auto const line = formatStaticString(item);
+    folly::writeFull(file.fd(), line.data(), line.size());
     if (RuntimeOption::EvalPerfDataMap) {
       auto const len = std::min<size_t>(item->size(), 255);
       std::string str(item->data(), len);
       // Only print the first line (up to 255 characters). Since we want '\0' in
       // the list of characters to avoid, we need to use the version of
diff --git a/hphp/runtime/server/admin-request-handler.h b/hphp/runtime/server/admin-request-handler.h
index f042ed7f6987..4e582096a50b 100644
--- a/hphp/runtime/server/admin-request-handler.h
+++ b/hphp/runtime/server/admin-request-handler.h
@@ -17,10 +17,12 @@
 #pragma once
 
 #include "hphp/runtime/server/access-log.h"
 #include "hphp/runtime/server/server.h"
 
+#include <folly/File.h>
+
 namespace HPHP {
 ///////////////////////////////////////////////////////////////////////////////
 
 /**
  * Admin Command Ext allow you to register more admin commands that are not
@@ -70,12 +72,11 @@ private:
   bool handleConstSizeRequest (const std::string &cmd, Transport *transport);
   bool handleInvalidateUnitRequest(const std::string &cmd,
                                    Transport *transport);
   bool handleStaticStringsRequest(const std::string &cmd,
                                   Transport *transport);
-  bool handleDumpStaticStringsRequest(const std::string &cmd,
-                                      const std::string &filename);
+  bool handleDumpStaticStringsRequest(folly::File& file);
   bool handleRandomStaticStringsRequest(const std::string &cmd,
                                         Transport *transport);
   bool handleVMRequest      (const std::string &cmd, Transport *transport);
   void handleProxyRequest(const std::string& cmd, Transport *transport);
   bool handleRandomApcRequest (const std::string &cmd, Transport *transport);
diff --git a/hphp/test/slow/ext_ldap/t78806688.php b/hphp/test/slow/ext_ldap/t78806688.php
new file mode 100644
index 000000000000..e6b6d388b908
--- /dev/null
+++ b/hphp/test/slow/ext_ldap/t78806688.php
@@ -0,0 +1,9 @@
+<?hh
+
+<<__EntryPoint>>
+function main(): void {
+  $multiplier = 1431655769;
+  $s = str_repeat("a", $multiplier);
+  ldap_escape($s);
+  echo "FAIL!\n";
+}
diff --git a/hphp/test/slow/ext_ldap/t78806688.php.expectf b/hphp/test/slow/ext_ldap/t78806688.php.expectf
new file mode 100644
index 000000000000..c95c560e4a32
--- /dev/null
+++ b/hphp/test/slow/ext_ldap/t78806688.php.expectf
@@ -0,0 +1 @@
+Fatal error: String length exceeded: 4294967307 > %d in %s/t78806688.php on line 7
diff --git a/hphp/test/slow/ext_string/t76103217.php b/hphp/test/slow/ext_string/t76103217.php
new file mode 100644
index 000000000000..085629f965b6
--- /dev/null
+++ b/hphp/test/slow/ext_string/t76103217.php
@@ -0,0 +1,11 @@
+<?hh
+
+<<__EntryPoint>>
+function main(): void {
+  $key = "\x26\xbd\xbd\xbd\xff\x60\xbf\xff\xff\x60";
+  $salt1 = "\x24\x32\x78\x24\x31\x30\x24\x24\x35\x24\xad\x20\x20\x26\xff\x60\xbf\xff\xff\x60\x24\x31\x78\xa8\xa8\xa0\x01\x01\x01\x01\x01\x01";
+  $salt2 = "\x24\x32\x78\x24\x31\x30\x24\x24\x35";
+
+  var_dump(base64_encode(crypt($key, $salt1)));
+  var_dump(base64_encode(crypt($key, $salt2)));
+}
diff --git a/hphp/test/slow/ext_string/t76103217.php.expect b/hphp/test/slow/ext_string/t76103217.php.expect
new file mode 100644
index 000000000000..e4cff2a4627b
--- /dev/null
+++ b/hphp/test/slow/ext_string/t76103217.php.expect
@@ -0,0 +1,2 @@
+string(4) "KjA="
+string(12) "JDJ4JDEwJCQ1"
diff --git a/hphp/test/slow/ext_xml/xmlreader_empty_expand.php b/hphp/test/slow/ext_xml/xmlreader_empty_expand.php
new file mode 100644
index 000000000000..32e707ef2183
--- /dev/null
+++ b/hphp/test/slow/ext_xml/xmlreader_empty_expand.php
@@ -0,0 +1,9 @@
+<?hh
+
+<<__EntryPoint>>
+function main(): void {
+    $a = new XMLReader();
+    $b = new DOMNode();
+    $a->expand($b);
+    $a->expand($a);
+}
diff --git a/hphp/test/slow/ext_xml/xmlreader_empty_expand.php.expectf b/hphp/test/slow/ext_xml/xmlreader_empty_expand.php.expectf
new file mode 100644
index 000000000000..cbf2c8c318da
--- /dev/null
+++ b/hphp/test/slow/ext_xml/xmlreader_empty_expand.php.expectf
@@ -0,0 +1,3 @@
+Warning: Invalid State Error in %s on line %d
+
+Warning: Invalid State Error in %s on line %d
diff --git a/hphp/util/light-process.cpp b/hphp/util/light-process.cpp
index c7d23ae1d148..db30f8fc46ca 100644
--- a/hphp/util/light-process.cpp
+++ b/hphp/util/light-process.cpp
@@ -44,18 +44,21 @@
 #include "hphp/util/hardware-counter.h"
 #include "hphp/util/hash.h"
 #include "hphp/util/hugetlb.h"
 #include "hphp/util/logger.h"
 #include "hphp/util/process.h"
+#include "hphp/util/struct-log.h"
 #include "hphp/util/timer.h"
 #include "hphp/util/user-info.h"
 
 namespace HPHP {
 
 ///////////////////////////////////////////////////////////////////////////////
 // helper functions
 
+bool LightProcess::g_strictUser = false;
+
 namespace {
 
 __thread LightProcess* tl_proc;
 bool s_trackProcessTimes = false;
 Mutex s_mutex;
@@ -434,27 +437,67 @@ void do_waitpid(int afdt_fd) {
 void do_change_user(int afdt_fd) {
   std::string uname;
   lwp_read(afdt_fd, uname);
   if (!uname.length()) return;
 
+  StructuredLogEntry* log = nullptr;
+  int err = 0;
+  SCOPE_EXIT {
+    if (log) {
+      log->setInt("errno", err);
+      log->setStr("new_user", uname);
+      StructuredLog::log("hhvm_lightprocess_error", *log);
+      delete log;
+    }
+  };
   auto buf = PasswdBuffer{};
   struct passwd *pw;
   if (getpwnam_r(uname.c_str(), &buf.ent, buf.data.get(), buf.size, &pw)) {
-    // TODO(alexeyt) should we log something and/or fail to start?
+    err = errno;
+    log = new StructuredLogEntry();
+    log->setStr("function", "getpwnam_r");
+    if (LightProcess::g_strictUser) {
+      throw std::runtime_error{"getpwnam_r(): " + folly::errnoStr(err)};
+    }
     return;
   }
   if (!pw) {
-    // TODO(alexeyt) should we log something and/or fail to start?
+    log = new StructuredLogEntry();
+    log->setStr("function", "getpwnam_r");
+    if (LightProcess::g_strictUser) {
+      throw std::runtime_error{"getpwnam_r(): not found"};
+    }
     return;
   }
-
   if (pw->pw_gid) {
-    initgroups(pw->pw_name, pw->pw_gid);
-    setgid(pw->pw_gid);
+    if (initgroups(pw->pw_name, pw->pw_gid)) {
+      err = errno;
+      log = new StructuredLogEntry();
+      log->setStr("function", "initgroups");
+    }
+    if (setgid(pw->pw_gid)) {
+      if (!log) {
+        err = errno;
+        log = new StructuredLogEntry();
+        log->setStr("function", "setgid");
+        if (LightProcess::g_strictUser) {
+          throw std::runtime_error{"setgid():" + folly::errnoStr(err)};
+        }
+      }
+    }
   }
   if (pw->pw_uid) {
-    setuid(pw->pw_uid);
+    if (setuid(pw->pw_uid)) {
+      if (!log) {
+        err = errno;
+        log = new StructuredLogEntry();
+        log->setStr("function", "setuid");
+        if (LightProcess::g_strictUser) {
+          throw std::runtime_error{"setuid():" + folly::errnoStr(err)};
+        }
+      }
+    }
   }
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 // light-weight process
diff --git a/hphp/util/light-process.h b/hphp/util/light-process.h
index dfe8eccb9a2e..e5378852a79d 100644
--- a/hphp/util/light-process.h
+++ b/hphp/util/light-process.h
@@ -127,10 +127,14 @@ private:
 
   pid_t m_shadowProcess;
   Mutex m_procMutex;
   int m_afdt_fd;
   std::map<FILE*, pid_t> m_popenMap;
+
+ public:
+  // whether to abort when user change fails.
+  static bool g_strictUser;
 };
 
 ///////////////////////////////////////////////////////////////////////////////
 }
 
diff --git a/hphp/zend/crypt-blowfish.cpp b/hphp/zend/crypt-blowfish.cpp
index 310e2555fc79..80d72ea3f18c 100644
--- a/hphp/zend/crypt-blowfish.cpp
+++ b/hphp/zend/crypt-blowfish.cpp
@@ -694,11 +694,14 @@ static char *BF_crypt(const char *key, const char *setting,
     __set_errno(EINVAL);
     return NULL;
   }
 
   count = (BF_word)1 << ((setting[4] - '0') * 10 + (setting[5] - '0'));
-  if (count < min || BF_decode(data.binary.salt, &setting[7], 16)) {
+  if (count < min ||
+      BF_decode(data.binary.salt, &setting[7], 16) ||
+      ((unsigned int)(setting[7 + 22 - 1] - 0x20) >= 0x60)
+     ) {
     __set_errno(EINVAL);
     return NULL;
   }
   BF_swap(data.binary.salt, 4);
 
@@ -912,6 +915,5 @@ char *_crypt_gensalt_blowfish_rn(const char *prefix, unsigned long count,
   output[7 + 22] = '\0';
 
   return output;
 }
 #endif
-
diff --git a/hphp/zend/zend-string.cpp b/hphp/zend/zend-string.cpp
index 9adc79f1b6eb..6ba0c21f0776 100644
--- a/hphp/zend/zend-string.cpp
+++ b/hphp/zend/zend-string.cpp
@@ -394,32 +394,42 @@ static void ito64(char *s, long v, int n) {
     v >>= 6;
   }
 }
 
 char *string_crypt(const char *key, const char *salt) {
-  assert(key);
-  assert(salt);
+  assertx(key);
+  assertx(salt);
 
   char random_salt[12];
   if (!*salt) {
     memcpy(random_salt,"$1$",3);
     ito64(random_salt+3,rand(),8);
     random_salt[11] = '\0';
     return string_crypt(key, random_salt);
   }
 
-  if ((strlen(salt) > sizeof("$2X$00$")) &&
+  auto const saltLen = strlen(salt);
+  if ((saltLen > sizeof("$2X$00$")) &&
     (salt[0] == '$') &&
     (salt[1] == '2') &&
     (salt[2] >= 'a') && (salt[2] <= 'z') &&
     (salt[3] == '$') &&
     (salt[4] >= '0') && (salt[4] <= '3') &&
     (salt[5] >= '0') && (salt[5] <= '9') &&
     (salt[6] == '$')) {
     // Bundled blowfish crypt()
     char output[61];
-    if (php_crypt_blowfish_rn(key, salt, output, sizeof(output))) {
+
+    static constexpr size_t maxSaltLength = 123;
+    char paddedSalt[maxSaltLength + 1];
+    paddedSalt[0] = paddedSalt[maxSaltLength] = '\0';
+
+    memset(&paddedSalt[1], '$', maxSaltLength - 1);
+    memcpy(paddedSalt, salt, std::min(maxSaltLength, saltLen));
+    paddedSalt[saltLen] = '\0';
+
+    if (php_crypt_blowfish_rn(key, paddedSalt, output, sizeof(output))) {
       return strdup(output);
     }
 
   } else {
     // System crypt() function

Vulnerable code:   void dump(const std::string& filename);
void PCRECache::dump(const std::string& filename) {
  std::ofstream out(filename.c_str());
        out << it.first->data() << "\n";
          out << key.c_str() << "\n";
  out.close();
void pcre_dump_cache(const std::string& filename) {
  s_pcreCache.dump(filename);
 * Dump the contents of the PCRE cache to filename.
void pcre_dump_cache(const std::string& filename);
  if (salt && salt[0] == '\0') {
  req::ptr<XMLDocumentData> doc() const { return m_node->doc(); }
    + sy * (sizeof(int *) + sizeof(unsigned char *))
    + sx * sy * (sizeof(int) + sizeof(unsigned char));
  String result(3 * value.size(), ReserveString);
    docp = doc->docp();
    if (docp == nullptr) {
      auto filename = transport->getParam("file");
      if (filename == "") filename = "/tmp/static_strings";
      handleDumpStaticStringsRequest(cmd, filename);
      transport->sendString("OK\n");
      auto filename = transport->getParam("file");
      if (filename == "") filename = "/tmp/pcre_cache";
      pcre_dump_cache(filename);
      transport->sendString("OK\n");
bool AdminRequestHandler::handleDumpStaticStringsRequest(
  const std::string& /*cmd*/, const std::string& filename) {
  std::ofstream out(filename.c_str());
  SCOPE_EXIT { out.close(); };
    out << formatStaticString(item);
  bool handleDumpStaticStringsRequest(const std::string &cmd,
                                      const std::string &filename);
    // TODO(alexeyt) should we log something and/or fail to start?
    // TODO(alexeyt) should we log something and/or fail to start?

    initgroups(pw->pw_name, pw->pw_gid);
    setgid(pw->pw_gid);
    setuid(pw->pw_uid);
  if (count < min || BF_decode(data.binary.salt, &setting[7], 16)) {

  assert(key);
  assert(salt);
  if ((strlen(salt) > sizeof("$2X$00$")) &&
    if (php_crypt_blowfish_rn(key, salt, output, sizeof(output))) {
Fixed code: #include <folly/FileUtil.h>
  void dump(folly::File& file);
void PCRECache::dump(folly::File& file) {
        folly::writeFull(file.fd(), it.first->data(), it.first->size());
        folly::writeFull(file.fd(), "\n", 1);
          folly::writeFull(file.fd(), key.data(), key.size());
          folly::writeFull(file.fd(), "\n", 1);
void pcre_dump_cache(folly::File& file) {
  s_pcreCache.dump(file);
#include <folly/File.h>
 * Dump the contents of the PCRE cache to the given file.
void pcre_dump_cache(folly::File& file);
#include "hphp/util/light-process.h"
std::string RuntimeOption::AdminDumpPath;
    Config::Bind(LightProcess::g_strictUser, ini, config,
                 "Server.LightProcessStrictUser", false);
    Config::Bind(AdminDumpPath, ini, config,
                 "AdminServer.DumpPath", "/tmp/hhvm_admin_dump");
  static std::string AdminDumpPath;

  assertx(salt);
  if (salt[0] == '\0') {
  req::ptr<XMLDocumentData> doc() const {
    return m_node ? m_node->doc() : nullptr;
  }
    + (sizeof(int *) + sizeof(unsigned char *)) * sy
    + (sizeof(int) + sizeof(unsigned char)) * sx * sy;
  String result(3UL * value.size(), ReserveString);
    if (doc == nullptr || doc->docp() == nullptr) {
    docp = doc->docp();
#include <folly/File.h>
#include <folly/FileUtil.h>
#include <folly/Optional.h>
namespace {

// When this struct is destroyed, it will close the file.
struct DumpFile {
  std::string path;
  folly::File file;
};

folly::Optional<DumpFile> dump_file(const char* name) {
  auto const path = folly::sformat("{}/{}", RO::AdminDumpPath, name);

  // mkdir -p the directory prefix of `path`
  if (FileUtil::mkdir(path) != 0) return folly::none;

  // If remove fails because of a permissions issue, then we won't be
  // able to open the file for exclusive write below.
  remove(path.c_str());

  // Create the file, failing if it already exists. Doing so ensures
  // that we have write access to the file and that no other user does.
  auto const fd = open(path.c_str(), O_CREAT|O_EXCL|O_RDWR, 0666);
  if (fd < 0) return folly::none;

  return DumpFile{path, folly::File(fd, /*owns=*/true)};
}

}

      if (auto file = dump_file("static_strings")) {
        handleDumpStaticStringsRequest(file->file);
        transport->sendString(folly::sformat("dumped to {}\n", file->path));
      } else {
        transport->sendString("Unable to mkdir or file already exists.\n");
      }
      if (auto file = dump_file("pcre_cache")) {
        pcre_dump_cache(file->file);
        transport->sendString(folly::sformat("dumped to {}\n", file->path));
      } else {
        transport->sendString("Unable to mkdir or file already exists.\n");
      }
bool AdminRequestHandler::handleDumpStaticStringsRequest(folly::File& file) {
    auto const line = formatStaticString(item);
    folly::writeFull(file.fd(), line.data(), line.size());
#include <folly/File.h>

  bool handleDumpStaticStringsRequest(folly::File& file);
<?hh

<<__EntryPoint>>
function main(): void {
  $multiplier = 1431655769;
  $s = str_repeat("a", $multiplier);
  ldap_escape($s);
  echo "FAIL!\n";
}
Fatal error: String length exceeded: 4294967307 > %d in %s/t78806688.php on line 7
<?hh

<<__EntryPoint>>
function main(): void {
  $key = "\x26\xbd\xbd\xbd\xff\x60\xbf\xff\xff\x60";
  $salt1 = "\x24\x32\x78\x24\x31\x30\x24\x24\x35\x24\xad\x20\x20\x26\xff\x60\xbf\xff\xff\x60\x24\x31\x78\xa8\xa8\xa0\x01\x01\x01\x01\x01\x01";
  $salt2 = "\x24\x32\x78\x24\x31\x30\x24\x24\x35";

  var_dump(base64_encode(crypt($key, $salt1)));
  var_dump(base64_encode(crypt($key, $salt2)));
}
string(4) "KjA="
string(12) "JDJ4JDEwJCQ1"
<?hh

<<__EntryPoint>>
function main(): void {
    $a = new XMLReader();
    $b = new DOMNode();
    $a->expand($b);
    $a->expand($a);
}
Warning: Invalid State Error in %s on line %d

Warning: Invalid State Error in %s on line %d
#include "hphp/util/struct-log.h"
bool LightProcess::g_strictUser = false;

  StructuredLogEntry* log = nullptr;
  int err = 0;
  SCOPE_EXIT {
    if (log) {
      log->setInt("errno", err);
      log->setStr("new_user", uname);
      StructuredLog::log("hhvm_lightprocess_error", *log);
      delete log;
    }
  };
    err = errno;
    log = new StructuredLogEntry();
    log->setStr("function", "getpwnam_r");
    if (LightProcess::g_strictUser) {
      throw std::runtime_error{"getpwnam_r(): " + folly::errnoStr(err)};
    }
    log = new StructuredLogEntry();
    log->setStr("function", "getpwnam_r");
    if (LightProcess::g_strictUser) {
      throw std::runtime_error{"getpwnam_r(): not found"};
    }
    if (initgroups(pw->pw_name, pw->pw_gid)) {
      err = errno;
      log = new StructuredLogEntry();
      log->setStr("function", "initgroups");
    }
    if (setgid(pw->pw_gid)) {
      if (!log) {
        err = errno;
        log = new StructuredLogEntry();
        log->setStr("function", "setgid");
        if (LightProcess::g_strictUser) {
          throw std::runtime_error{"setgid():" + folly::errnoStr(err)};
        }
      }
    }
    if (setuid(pw->pw_uid)) {
      if (!log) {
        err = errno;
        log = new StructuredLogEntry();
        log->setStr("function", "setuid");
        if (LightProcess::g_strictUser) {
          throw std::runtime_error{"setuid():" + folly::errnoStr(err)};
        }
      }
    }

 public:
  // whether to abort when user change fails.
  static bool g_strictUser;
  if (count < min ||
      BF_decode(data.binary.salt, &setting[7], 16) ||
      ((unsigned int)(setting[7 + 22 - 1] - 0x20) >= 0x60)
     ) {
  assertx(key);
  assertx(salt);
  auto const saltLen = strlen(salt);
  if ((saltLen > sizeof("$2X$00$")) &&

    static constexpr size_t maxSaltLength = 123;
    char paddedSalt[maxSaltLength + 1];
    paddedSalt[0] = paddedSalt[maxSaltLength] = '\0';

    memset(&paddedSalt[1], '$', maxSaltLength - 1);
    memcpy(paddedSalt, salt, std::min(maxSaltLength, saltLen));
    paddedSalt[saltLen] = '\0';

    if (php_crypt_blowfish_rn(key, paddedSalt, output, sizeof(output))) {