security fixes

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

Author: jjergus

hphp/runtime/base/preg.cpp

@@ -49,6 +49,7 @@
 #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 */
@@ -217,7 +218,7 @@ struct PCRECache {
             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:
@@ -512,12 +513,12 @@ void PCRECache::insert(
   }
 }
 
-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:
@@ -530,12 +531,12 @@ void PCRECache::dump(const std::string& filename) {
           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 {
@@ -572,8 +573,8 @@ 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*/) {

hphp/runtime/base/preg.h

@@ -18,6 +18,7 @@
 
 #include "hphp/runtime/base/type-string.h"
 
+#include <folly/File.h>
 #include <folly/Optional.h>
 
 #include <cstdint>
@@ -111,9 +112,9 @@ void pcre_reinit();
 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

hphp/runtime/base/runtime-option.cpp

@@ -54,6 +54,7 @@
 #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"
@@ -754,6 +755,7 @@ 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;
@@ -2398,6 +2400,8 @@ void RuntimeOption::Load(
                  "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,
@@ -2513,6 +2517,8 @@ void RuntimeOption::Load(
     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

hphp/runtime/base/runtime-option.h

@@ -496,6 +496,8 @@ struct RuntimeOption {
   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

hphp/runtime/base/string-util.cpp

@@ -435,7 +435,8 @@ String StringUtil::ROT13(const String& input) {
 }
 
 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.");

hphp/runtime/ext/domdocument/ext_domdocument.h

@@ -51,7 +51,9 @@ 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;

hphp/runtime/ext/gd/libgd/gd.cpp

@@ -185,8 +185,8 @@ gdImagePtr gdImageCreateTrueColor (int sx, int sy)
 
   // 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;

hphp/runtime/ext/ldap/ext_ldap.cpp

@@ -2139,7 +2139,7 @@ String HHVM_FUNCTION(ldap_escape,
 
   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++) {

hphp/runtime/ext/xmlreader/ext_xmlreader.cpp

@@ -598,11 +598,11 @@ Variant HHVM_METHOD(XMLReader, expand,
   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) {

hphp/runtime/server/admin-request-handler.cpp

@@ -76,6 +76,9 @@
 #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>
 
@@ -254,6 +257,34 @@ void AdminRequestHandler::teardownRequest(Transport* transport) noexcept {
   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();
@@ -714,10 +745,12 @@ void AdminRequestHandler::handleRequest(Transport *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) {
@@ -746,10 +779,12 @@ void AdminRequestHandler::handleRequest(Transport *transport) {
     }
 
     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;
     }
 
@@ -1368,13 +1403,11 @@ std::string formatStaticString(StringData* str) {
       "----\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);

hphp/runtime/server/admin-request-handler.h

@@ -19,6 +19,8 @@
 #include "hphp/runtime/server/access-log.h"
 #include "hphp/runtime/server/server.h"
 
+#include <folly/File.h>
+
 namespace HPHP {
 ///////////////////////////////////////////////////////////////////////////////
 
@@ -72,8 +74,7 @@ struct AdminRequestHandler : RequestHandler {
                                    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);

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";
+}

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

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)));
+}

hphp/test/slow/ext_string/t76103217.php.expect

@@ -0,0 +1,2 @@
+string(4) "KjA="
+string(12) "JDJ4JDEwJCQ1"

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);
+}

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