CVE-2019-3561: Out-of-bounds Read (CWE-125)
value: Insufficient boundary checks for the strrpos and strripos functions allow access to out-of-bounds memory. This affects all supported versions of HHVM (4.0.3, 3.30.4, and 3.27.7 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-04-29T16:29Z
Repository: https://github.com/facebook/hhvm
Language: PHP
Files changed: hphp/runtime/base/zend-string.cpp, hphp/test/slow/ext_string/ext_string.php, hphp/test/slow/ext_string/ext_string.php.expectf
Commit date: 2019-04-06 08:57:21 -0700
Commit message: Prevent strrpos and strripos from visiting out-of-bounds memory.

Reviewed By: alexeyt

Differential Revision: D14669668

fbshipit-source-id: 16bb4674bccf83ffd737e60d1fd9fd4a0093f77c

Diff: diff --git a/hphp/runtime/base/zend-string.cpp b/hphp/runtime/base/zend-string.cpp
index 40267cffd6f7..e128af3ecd41 100644
--- a/hphp/runtime/base/zend-string.cpp
+++ b/hphp/runtime/base/zend-string.cpp
@@ -19,10 +19,11 @@
 #include "hphp/runtime/base/zend-printf.h"
 #include "hphp/runtime/base/zend-math.h"
 
 #include "hphp/util/lock.h"
 #include "hphp/util/overflow.h"
+#include <algorithm>
 #include <cmath>
 
 #ifndef _MSC_VER
 #include <monetary.h>
 #endif
@@ -383,17 +384,17 @@ int string_rfind(const char *input, int len, const char *s, int s_len,
   void *ptr;
   if (case_sensitive) {
     if (pos >= 0) {
       ptr = bstrrstr(input + pos, len - pos, s, s_len);
     } else {
-      ptr = bstrrstr(input, len + pos + s_len, s, s_len);
+      ptr = bstrrstr(input, len + std::min(pos + s_len, 0), s, s_len);
     }
   } else {
     if (pos >= 0) {
       ptr = bstrrcasestr(input + pos, len - pos, s, s_len);
     } else {
-      ptr = bstrrcasestr(input, len + pos + s_len, s, s_len);
+      ptr = bstrrcasestr(input, len + std::min(pos + s_len, 0), s, s_len);
     }
   }
   if (ptr != nullptr) {
     return (int)((const char *)ptr - input);
   }
diff --git a/hphp/test/slow/ext_string/ext_string.php b/hphp/test/slow/ext_string/ext_string.php
index 836d8b0c6843..ce3f3651280b 100644
--- a/hphp/test/slow/ext_string/ext_string.php
+++ b/hphp/test/slow/ext_string/ext_string.php
@@ -389,10 +389,19 @@ var_dump(strrpos("0123456789a123456789b123456789c", "7", 20));
 var_dump(strrpos("0123456789a123456789b123456789c", "7", 28));
 
 
 var_dump(strripos("abcdef abcdef", "A"));
 
+var_dump(strrpos("abc", "c\0", -1));
+var_dump(strripos("abc", "c\0", -1));
+var_dump(strrpos("abc", "abc", -3));
+var_dump(strripos("abc", "abc", -3));
+var_dump(strrpos("aaaa", "aa", -1));
+var_dump(strripos("aaaa", "aa", -1));
+var_dump(strrpos("aaaa", "aa", -2));
+var_dump(strripos("aaaa", "aa", -2));
+
 $text = "This is a test";
 var_dump(substr_count($text, "is"));
 var_dump(substr_count($text, "is", 3));
 var_dump(substr_count($text, "is", 3, 3));
 var_dump(substr_count("gcdgcdgcd", "gcdgcd"));
diff --git a/hphp/test/slow/ext_string/ext_string.php.expectf b/hphp/test/slow/ext_string/ext_string.php.expectf
index 6ba58f635e9f..f9bb69954ad6 100644
--- a/hphp/test/slow/ext_string/ext_string.php.expectf
+++ b/hphp/test/slow/ext_string/ext_string.php.expectf
@@ -218,10 +218,18 @@ int(7)
 int(7)
 int(17)
 int(27)
 bool(false)
 int(7)
+bool(false)
+bool(false)
+int(0)
+int(0)
+int(2)
+int(2)
+int(2)
+int(2)
 int(2)
 int(1)
 int(0)
 int(1)
 int(2)

Vulnerable code:       ptr = bstrrstr(input, len + pos + s_len, s, s_len);
      ptr = bstrrcasestr(input, len + pos + s_len, s, s_len);
Fixed code: #include <algorithm>
      ptr = bstrrstr(input, len + std::min(pos + s_len, 0), s, s_len);
      ptr = bstrrcasestr(input, len + std::min(pos + s_len, 0), s, s_len);
var_dump(strrpos("abc", "c\0", -1));
var_dump(strripos("abc", "c\0", -1));
var_dump(strrpos("abc", "abc", -3));
var_dump(strripos("abc", "abc", -3));
var_dump(strrpos("aaaa", "aa", -1));
var_dump(strripos("aaaa", "aa", -1));
var_dump(strrpos("aaaa", "aa", -2));
var_dump(strripos("aaaa", "aa", -2));

bool(false)
bool(false)
int(0)
int(0)
int(2)
int(2)
int(2)
int(2)