CVE-2020-1899: Improper Restriction of Operations within the Bounds of a Memory Buffer (CWE-119)
value: The unserialize() function supported a type code, "S", which was meant to be supported only for APC serialization. This type code allowed arbitrary memory addresses to be accessed as if they were static StringData objects. This issue affected HHVM prior to v4.32.3, between versions 4.33.0 and 4.56.0, 4.57.0, 4.58.0, 4.58.1, 4.59.0, 4.60.0, 4.61.0, 4.62.0.
CWE description: The product performs operations on a memory buffer, but it reads from or writes to a memory location outside the buffer's intended boundary. This may result in read or write operations on unexpected memory locations that could be linked to other variables, data structures, or internal program data.
CVSS v3 base score: 7.5
Published: 2021-03-11T01:15Z
Repository: https://github.com/facebook/hhvm
Language: PHP
Files changed: hphp/runtime/base/variable-unserializer.cpp, hphp/test/slow/serialization/no-apc-string-pointer.php, hphp/test/slow/serialization/no-apc-string-pointer.php.expectf
Commit date: 2020-06-30 16:27:28 -0700
Commit message: CVE-2020-1899

VariableUnserializer should only recognize type 'S' in APCSerialization mode

Diff: diff --git a/hphp/runtime/base/variable-unserializer.cpp b/hphp/runtime/base/variable-unserializer.cpp
index 32dac12b4673..c03843bc9fed 100644
--- a/hphp/runtime/base/variable-unserializer.cpp
+++ b/hphp/runtime/base/variable-unserializer.cpp
@@ -483,11 +483,11 @@ void VariableUnserializer::addSleepingObject(const Object& o) {
 
 bool VariableUnserializer::matchString(folly::StringPiece str) {
   const char* p = m_buf;
   assertx(p <= m_end);
   int total = 0;
-  if (*p == 'S') {
+  if (*p == 'S' && type() == VariableUnserializer::Type::APCSerialize) {
     total = 2 + 8 + 1;
     if (p + total > m_end) return false;
     p++;
     if (*p++ != ':') return false;
     auto const sd = *reinterpret_cast<StringData*const*>(p);
diff --git a/hphp/test/slow/serialization/no-apc-string-pointer.php b/hphp/test/slow/serialization/no-apc-string-pointer.php
new file mode 100644
index 000000000000..14f41fec1423
--- /dev/null
+++ b/hphp/test/slow/serialization/no-apc-string-pointer.php
@@ -0,0 +1,11 @@
+<?hh
+// Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
+
+class Foo {
+  public $name;
+}
+
+<<__EntryPoint>>
+function main() {
+  var_dump(unserialize("O:3:\"foo\":1:{S:\x00\x00\x00\x00\x00\x00\x00\x00;s:5:\"value\";}"));
+}
diff --git a/hphp/test/slow/serialization/no-apc-string-pointer.php.expectf b/hphp/test/slow/serialization/no-apc-string-pointer.php.expectf
new file mode 100644
index 000000000000..f0e34fba0e84
--- /dev/null
+++ b/hphp/test/slow/serialization/no-apc-string-pointer.php.expectf
@@ -0,0 +1,2 @@
+Notice: Unable to unserialize: [O:3:"foo":1:{S:]. Unknown type 'S'. in %s/no-apc-string-pointer.php on line 10
+bool(false)

Vulnerable code:   if (*p == 'S') {
Fixed code:   if (*p == 'S' && type() == VariableUnserializer::Type::APCSerialize) {
<?hh
// Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.

class Foo {
  public $name;
}

<<__EntryPoint>>
function main() {
  var_dump(unserialize("O:3:\"foo\":1:{S:\x00\x00\x00\x00\x00\x00\x00\x00;s:5:\"value\";}"));
}
Notice: Unable to unserialize: [O:3:"foo":1:{S:]. Unknown type 'S'. in %s/no-apc-string-pointer.php on line 10
bool(false)