[Security] HHVM Accelerated Thrift: Protect against infinte loop in deserialization

stevegury · fredemmott · commit 7c61f20a51e4 · 2018-01-11T09:01:25.000-08:00
Summary:
As reported in T22402076, a maliciously crafted serialized message can trigger an infinite loop in the deserialization code of HHVM accelerated Thrift.

The cause of the bug if an overflow of the computed capacity which never exit the loop.
`while (Capacity(scale) &lt; n) scale *= 2;`

The added check is (I believe) negligle in performance as the branch will almost always be correctly predicted.

Reviewed By: alexeyt

Differential Revision: D6684202

fbshipit-source-id: ef5b72a67bbec9f6bda5db4b0bb5f3c7793bc5dd
diff --git a/hphp/runtime/base/hash-table-inl.h b/hphp/runtime/base/hash-table-inl.h
@@ -20,7 +20,9 @@ namespace array {
 
 ALWAYS_INLINE
 uint32_t HashTableCommon::computeScaleFromSize(uint32_t n) {
-  assert(n <= 0x7fffffffU);
+  if (n >= MaxSize) {
+    return MaxScale;
+  }
   auto scale = SmallScale;
   while (Capacity(scale) < n) scale *= 2;
   return scale;
diff --git a/hphp/runtime/base/hash-table.h b/hphp/runtime/base/hash-table.h
@@ -73,6 +73,7 @@ struct HashTableCommon {
   static constexpr uint32_t MaxMask = MaxHashSize - 1;
   static constexpr uint32_t MaxSize = MaxMask - MaxMask / LoadScale;
   static constexpr uint32_t MaxMakeSize = 4 * SmallSize;
+  static constexpr uint32_t MaxScale = MaxHashSize / LoadScale;
 
   constexpr static uint32_t HashSize(uint32_t scale) { return 4 * scale; }
   constexpr static uint32_t Capacity(uint32_t scale) { return 3 * scale; }
