CVE-2016-6875: Other (NVD-CWE-Other)
value: Infinite recursion in wddx in Facebook HHVM before 3.15.0 allows attackers to have unspecified impact via unknown vectors.
CWE description: NVD is only using a subset of CWE for mapping instead of the entire CWE, and the weakness type is not covered by that subset.
CVSS v3 base score: 9.8
Published: 2017-02-17T17:59Z
Repository: https://github.com/facebook/hhvm
Language: PHP
Files changed: hphp/runtime/ext/wddx/ext_wddx.cpp, hphp/runtime/ext/wddx/ext_wddx.h, hphp/test/slow/ext_wddx/recursion.php, hphp/test/slow/ext_wddx/recursion.php.expectf
Commit date: 2016-08-01 14:39:45 -0700
Commit message: Fix infinite recursion in wddx

Summary:
It wasn't checking for infinite recursion due to references or self-referential
objects. As it turns out closures always return themselves when converted to an
array. Raising a warning and returning is how PHP-src deals with this problem,
nothing special is done for closures.

Reviewed By: alexmalyshev

Differential Revision: D3465655

fbshipit-source-id: a42bc34d30cf4825faf33596139c0c05f8e4f5f1

Diff: diff --git a/hphp/runtime/ext/wddx/ext_wddx.cpp b/hphp/runtime/ext/wddx/ext_wddx.cpp
index 4cf4f3802f5..7ab7af05331 100644
--- a/hphp/runtime/ext/wddx/ext_wddx.cpp
+++ b/hphp/runtime/ext/wddx/ext_wddx.cpp
@@ -64,26 +64,47 @@ bool WddxPacket::serialize_value(const Variant& varVariant) {
 }
 
 bool WddxPacket::recursiveAddVar(const String& varName,
                                  const Variant& varVariant,
                                  bool hasVarTag) {
+  SeenContainers seen;
+  return recursiveAddVarImpl(varName, varVariant, hasVarTag, seen);
+}
 
+bool WddxPacket::recursiveAddVarImpl(const String& varName,
+                                     const Variant& varVariant,
+                                     bool hasVarTag,
+                                     SeenContainers& seen) {
   bool isArray = varVariant.isArray();
   bool isObject = varVariant.isObject();
 
   if (isArray || isObject) {
+    Array varAsArray;
+    Object varAsObject;
+    ArrayOrObject ptr;
+    if (isArray) {
+      varAsArray = varVariant.toArray();
+      ptr = varAsArray.get();
+    }
+    if (isObject) {
+      varAsObject = varVariant.toObject();
+      varAsArray = varAsObject.toArray();
+      ptr = varAsObject.get();
+    }
+    assert(!ptr.isNull());
+    if (!seen.emplace(ptr).second) {
+      raise_warning("recursion detected");
+      return false;
+    }
+    SCOPE_EXIT { seen.erase(ptr); };
+
     if (hasVarTag) {
       m_packetString.append("<var name='");
       m_packetString.append(varName.data());
       m_packetString.append("'>");
     }
 
-    Array varAsArray;
-    Object varAsObject = varVariant.toObject();
-    if (isArray) varAsArray = varVariant.toArray();
-    if (isObject) varAsArray = varAsObject.toArray();
-
     int length = varAsArray.length();
     if (length > 0) {
       ArrayIter it = ArrayIter(varAsArray);
       if (it.first().isString()) isObject = true;
       if (isObject) {
@@ -97,13 +118,13 @@ bool WddxPacket::recursiveAddVar(const String& varName,
         m_packetString.append("<array length='");
         m_packetString.append(std::to_string(length));
         m_packetString.append("'>");
       }
       for (ArrayIter it(varAsArray); it; ++it) {
-        Variant key = it.first();
-        Variant value = it.second();
-        recursiveAddVar(key.toString(), value, isObject);
+        auto key = it.first();
+        auto const& value = it.secondRef();
+        recursiveAddVarImpl(key.toString(), value, isObject, seen);
       }
       if (isObject) {
         m_packetString.append("</struct>");
       }
       else {
diff --git a/hphp/runtime/ext/wddx/ext_wddx.h b/hphp/runtime/ext/wddx/ext_wddx.h
index 82208e923b9..f184d90bac0 100644
--- a/hphp/runtime/ext/wddx/ext_wddx.h
+++ b/hphp/runtime/ext/wddx/ext_wddx.h
@@ -36,11 +36,11 @@ struct WddxPacket : ResourceData {
 
   bool add_var(const String& varName, bool hasVarTag);
   String packet_end();
   bool serialize_value(const Variant& varVariant);
   bool recursiveAddVar(const String& varName, const Variant& varVariant,
-                       bool hasVarTag );
+                       bool hasVarTag);
 
  private:
   String getWddxEncoded(const String& varType,
                         const String& varValue,
                         const String& varName,
@@ -50,10 +50,21 @@ struct WddxPacket : ResourceData {
                    const String& end,
                    const String& varValue,
                    const String& varName,
                    bool hasVarTag);
 
+  using ArrayOrObject = Either<ArrayData*,ObjectData*>;
+  struct EitherHash {
+    size_t operator()(const ArrayOrObject data) const {
+      return data.toOpaque();
+    }
+  };
+  using SeenContainers = req::hash_set<ArrayOrObject, EitherHash>;
+
+  bool recursiveAddVarImpl(const String& varName, const Variant& varVariant,
+                           bool hasVarTag, SeenContainers& seen);
+
   StringBuffer m_packetString;
   bool m_packetClosed;
   bool m_manualPacketCreation;
 };
 
diff --git a/hphp/test/slow/ext_wddx/recursion.php b/hphp/test/slow/ext_wddx/recursion.php
new file mode 100644
index 00000000000..f3f23a5f2f5
--- /dev/null
+++ b/hphp/test/slow/ext_wddx/recursion.php
@@ -0,0 +1,7 @@
+<?php
+$s1 = wddx_serialize_value(function () {});
+var_dump($s1);
+
+$a = []; $a[] =& $a;
+$s2 = wddx_serialize_value($a);
+var_dump($s2);
diff --git a/hphp/test/slow/ext_wddx/recursion.php.expectf b/hphp/test/slow/ext_wddx/recursion.php.expectf
new file mode 100644
index 00000000000..d9c6009b90a
--- /dev/null
+++ b/hphp/test/slow/ext_wddx/recursion.php.expectf
@@ -0,0 +1,6 @@
+
+Warning: recursion detected in %s/recursion.php on line 2
+string(%d) "<wddxPacket version='1.0'><header/><data><struct><var name='php_class_name'><string>Closure$;%s$%s$</string></var></struct></data></wddxPacket>"
+
+Warning: recursion detected in %s/recursion.php on line 6
+string(87) "<wddxPacket version='1.0'><header/><data><array length='1'></array></data></wddxPacket>"

Vulnerable code:     Array varAsArray;
    Object varAsObject = varVariant.toObject();
    if (isArray) varAsArray = varVariant.toArray();
    if (isObject) varAsArray = varAsObject.toArray();

        Variant key = it.first();
        Variant value = it.second();
        recursiveAddVar(key.toString(), value, isObject);
                       bool hasVarTag );
Fixed code:   SeenContainers seen;
  return recursiveAddVarImpl(varName, varVariant, hasVarTag, seen);
}
bool WddxPacket::recursiveAddVarImpl(const String& varName,
                                     const Variant& varVariant,
                                     bool hasVarTag,
                                     SeenContainers& seen) {
    Array varAsArray;
    Object varAsObject;
    ArrayOrObject ptr;
    if (isArray) {
      varAsArray = varVariant.toArray();
      ptr = varAsArray.get();
    }
    if (isObject) {
      varAsObject = varVariant.toObject();
      varAsArray = varAsObject.toArray();
      ptr = varAsObject.get();
    }
    assert(!ptr.isNull());
    if (!seen.emplace(ptr).second) {
      raise_warning("recursion detected");
      return false;
    }
    SCOPE_EXIT { seen.erase(ptr); };

        auto key = it.first();
        auto const& value = it.secondRef();
        recursiveAddVarImpl(key.toString(), value, isObject, seen);
                       bool hasVarTag);
  using ArrayOrObject = Either<ArrayData*,ObjectData*>;
  struct EitherHash {
    size_t operator()(const ArrayOrObject data) const {
      return data.toOpaque();
    }
  };
  using SeenContainers = req::hash_set<ArrayOrObject, EitherHash>;

  bool recursiveAddVarImpl(const String& varName, const Variant& varVariant,
                           bool hasVarTag, SeenContainers& seen);

<?php
$s1 = wddx_serialize_value(function () {});
var_dump($s1);

$a = []; $a[] =& $a;
$s2 = wddx_serialize_value($a);
var_dump($s2);

Warning: recursion detected in %s/recursion.php on line 2
string(%d) "<wddxPacket version='1.0'><header/><data><struct><var name='php_class_name'><string>Closure$;%s$%s$</string></var></struct></data></wddxPacket>"

Warning: recursion detected in %s/recursion.php on line 6
string(87) "<wddxPacket version='1.0'><header/><data><array length='1'></array></data></wddxPacket>"