Fix #66797: mb_substr only takes 32-bit signed integer

`from` and `len` are `long`, but get passed to mbfl_substr() which expects
`int`s. Therefore we clamp the values to avoid the undefined conversion
behavior.

diff --git a/NEWS b/NEWS
index a05fa844c5104883f24ebb7cb8a7e3d8970af824..531d6266b4126f21afa76cb45ed5ba0f94bd6aa3 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -31,6 +31,9 @@ PHP                                                                        NEWS
 - JSON:
   . Fixed bug #72787 (json_decode reads out of bounds). (Jakub Zelenka)
 
+- mbstring:
+  . Fixed bug #66797 (mb_substr only takes 32-bit signed integer). (cmb)
+
 - MSSQL:
   . Fixed bug #72039 (Use of uninitialised value on mssql_guid_string). (Kalle)
 

diff --git a/ext/mbstring/mbstring.c b/ext/mbstring/mbstring.c
index 1cfaf2cc36efa54139a983626e339d0e631ecd4b..ee8a00912b3485f5a7db079bfdae323a466b3477 100644 (file)
--- a/ext/mbstring/mbstring.c
+++ b/ext/mbstring/mbstring.c
@@ -2799,6 +2799,13 @@ PHP_FUNCTION(mb_substr)
                RETURN_FALSE;
        }
 
+       if (from > INT_MAX) {
+               from = INT_MAX;
+       }
+       if (len > INT_MAX) {
+               len = INT_MAX;
+       }
+
        ret = mbfl_substr(&string, &result, from, len);
        if (NULL == ret) {
                RETURN_FALSE;

diff --git a/ext/mbstring/tests/bug66797.phpt b/ext/mbstring/tests/bug66797.phpt
new file mode 100644 (file)
index 0000000..df9e789
--- /dev/null
+++ b/ext/mbstring/tests/bug66797.phpt
@@ -0,0 +1,23 @@
+--TEST--
+Bug #66797 (mb_substr only takes 32-bit signed integer)
+--SKIPIF--
+<?php
+if (!extension_loaded('mbstring')) die('skip mbstring extension not available');
+if (PHP_INT_SIZE != 8) die('skip this test is for 64bit platforms only');
+?>
+--FILE--
+<?php
+var_dump(
+    mb_substr('bar', 0, 0x7fffffff),
+    mb_substr('bar', 0, 0x80000000),
+    mb_substr('bar', 0xffffffff, 1),
+    mb_substr('bar', 0x100000000, 1)
+);
+?>
+==DONE==
+--EXPECTF--
+string(3) "bar"
+string(3) "bar"
+string(0) ""
+string(0) ""
+==DONE==