CVE-2019-3556: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal') (CWE-22)
value: HHVM supports the use of an "admin" server which accepts administrative requests over HTTP. One of those request handlers, dump-pcre-cache, can be used to output cached regular expressions from the current execution context into a file. The handler takes a parameter which specifies where on the filesystem to write this data. The parameter is not validated, allowing a malicious user to overwrite arbitrary files where the user running HHVM has write access. This issue affects HHVM versions prior to 4.56.2, all versions between 4.57.0 and 4.78.0, as well as 4.79.0, 4.80.0, 4.81.0, 4.82.0, and 4.83.0.
CWE description: The product uses external input to construct a pathname that is intended to identify a file or directory that is located underneath a restricted parent directory, but the product does not properly neutralize special elements within the pathname that can cause the pathname to resolve to a location that is outside of the restricted directory.
CVSS v3 base score: 8.1
Published: 2021-10-26T20: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))) {