CVE-2019-3557: Out-of-bounds Read (CWE-125)
value: The implementations of streams for bz2 and php://output improperly implemented their readImpl functions, returning -1 consistently. This behavior caused some stream functions, such as stream_get_line, to trigger an out-of-bounds read when operating on such malformed streams. The implementations were updated to return valid values consistently. This affects all supported versions of HHVM (3.30 and 3.27.4 and below).
CWE description: The product reads data past the end, or before the beginning, of the intended buffer.
CVSS v3 base score: 9.8
Published: 2019-01-15T22:29Z
Repository: https://github.com/facebook/hhvm
Language: C++
Files changed: hphp/runtime/base/output-file.cpp, hphp/runtime/ext/bz2/bz2-file.cpp, hphp/test/slow/oob_read_file.php, hphp/test/slow/oob_read_file.php.expectf
Commit date: 2019-01-14 12:27:23 -0800
Commit message: CVE-2019-3557: Fix OOB read in readRecord on BZ2Files/OutputFiles

Summary:
These File subclasses return -1 on read errors which is not what is
expected for readImpl--this made File::readRecord behave unusually if the read
fails, causing it to read (size_t)(-1) bytes from its stream buffer; which,
unsurprisingly produces a out-of-bounds heap read.

Reviewed By: leikahing, jjgriego

Differential Revision: D13659395

fbshipit-source-id: 359ed6e3ff9f9cf49b752b666f51c4e0b3ce4b8a

Diff: diff --git a/hphp/runtime/base/output-file.cpp b/hphp/runtime/base/output-file.cpp
index f512b7896d8c..1b2711b51f22 100644
--- a/hphp/runtime/base/output-file.cpp
+++ b/hphp/runtime/base/output-file.cpp
@@ -64,11 +64,11 @@ bool OutputFile::closeImpl() {
 ///////////////////////////////////////////////////////////////////////////////
 // virtual functions
 
 int64_t OutputFile::readImpl(char* /*buffer*/, int64_t /*length*/) {
   raise_warning("cannot read from a php://output stream");
-  return -1;
+  return 0;
 }
 
 int OutputFile::getc() {
   raise_warning("cannot read from a php://output stream");
   return -1;
diff --git a/hphp/runtime/ext/bz2/bz2-file.cpp b/hphp/runtime/ext/bz2/bz2-file.cpp
index 3b180f730384..687d01455907 100644
--- a/hphp/runtime/ext/bz2/bz2-file.cpp
+++ b/hphp/runtime/ext/bz2/bz2-file.cpp
@@ -93,11 +93,11 @@ int64_t BZ2File::readImpl(char * buf, int64_t length) {
    * the file - so, only set EOF after a failed read. This matches PHP5.
    */
   if (len <= 0) {
     setEof(true);
     if (len < 0) {
-      return -1;
+      return 0;
     }
   }
   return len;
 }
 
diff --git a/hphp/test/slow/oob_read_file.php b/hphp/test/slow/oob_read_file.php
new file mode 100644
index 000000000000..49a9904bdb77
--- /dev/null
+++ b/hphp/test/slow/oob_read_file.php
@@ -0,0 +1,12 @@
+<?hh
+
+<<__EntryPoint>>
+function main() {
+    $a = bzopen("/dev/null", "w");
+    $tmp = stream_get_line($a, 1, "1");
+    var_dump($tmp);
+
+    $a = fopen("php://output", "w");
+    $tmp = stream_get_line($a, 1, "1");
+    var_dump($tmp);
+}
diff --git a/hphp/test/slow/oob_read_file.php.expectf b/hphp/test/slow/oob_read_file.php.expectf
new file mode 100644
index 000000000000..d2d24bc1eb44
--- /dev/null
+++ b/hphp/test/slow/oob_read_file.php.expectf
@@ -0,0 +1,4 @@
+string(0) ""
+
+Warning: cannot read from a php://output stream in %s on line %d
+string(0) ""

Vulnerable code:   return -1;
      return -1;
Fixed code:   return 0;
      return 0;
<?hh

<<__EntryPoint>>
function main() {
    $a = bzopen("/dev/null", "w");
    $tmp = stream_get_line($a, 1, "1");
    var_dump($tmp);

    $a = fopen("php://output", "w");
    $tmp = stream_get_line($a, 1, "1");
    var_dump($tmp);
}
string(0) ""

Warning: cannot read from a php://output stream in %s on line %d
string(0) ""