Fixed bug #74670

Validate that "C" serialization payload is followed by "}" prior to
calling the unserialize() handler. This mitigates issues caused by
unserialize() not correctly handling strings that are not NUL
terminated. Making sure that there is a "}" at the end avoids the
problem.

7 files changed, 67 insertions, 41 deletions

diff --git a/NEWS b/NEWS
index b2310d8c45..17c1b81781 100644
--- a/NEWS
+++ b/NEWS
@@ -15,6 +15,10 @@ PHP                                                                        NEWS
   . Fixed bug #73342 (Vulnerability in php-fpm by changing stdin to
     non-blocking). (Nikita)
 
+- GMP:
+  . Fixed bug #76470 (Integer Underflow when unserializing GMP and possible
+    other classes). (Nikita)
+
 - intl:
   . Fixed bug #76556 (get_debug_info handler for BreakIterator shows wrong
     type). (cmb)
diff --git a/ext/gmp/bug74670.phpt b/ext/gmp/bug74670.phpt
new file mode 100644
index 0000000000..a28640b5c6
--- /dev/null
+++ b/ext/gmp/bug74670.phpt
@@ -0,0 +1,10 @@
+--TEST--
+Bug #74670: Integer Underflow when unserializing GMP and possible other classes
+--FILE--
+<?php
+$str = 'C:3:"GMP":4:{s:6666666666:""}';
+var_dump(unserialize($str));
+?>
+--EXPECTF--
+Notice: unserialize(): Error at offset 13 of 29 bytes in %s on line %d
+bool(false)
diff --git a/ext/gmp/tests/serialize.phpt b/ext/gmp/tests/serialize.phpt
index 5e308fa14e..e9b9bb549f 100644
--- a/ext/gmp/tests/serialize.phpt
+++ b/ext/gmp/tests/serialize.phpt
@@ -18,7 +18,7 @@ try {
 } catch (Exception $e) { var_dump($e->getMessage()); }
 
 try {
-    unserialize('C:3:"GMP":8:{s:2:"42";}');
+    unserialize('C:3:"GMP":9:{s:2:"42";}');
 } catch (Exception $e) { var_dump($e->getMessage()); }
 
 ?>
diff --git a/ext/standard/tests/serialize/bug71311.phpt b/ext/standard/tests/serialize/bug71311.phpt
index 3c70783d37..70267c5b0b 100644
--- a/ext/standard/tests/serialize/bug71311.phpt
+++ b/ext/standard/tests/serialize/bug71311.phpt
@@ -2,7 +2,7 @@
 Bug #71311 Use-after-free vulnerability in SPL(ArrayObject, unserialize)
 --FILE--
 <?php
-$data = unserialize("C:11:\"ArrayObject\":11:{x:i:0;r:3;XX");
+$data = unserialize("C:11:\"ArrayObject\":11:{x:i:0;r:3;X}");
 var_dump($data);
 ?>
 --EXPECTF--
diff --git a/ext/standard/tests/serialize/bug73341.phpt b/ext/standard/tests/serialize/bug73341.phpt
index 55423217c3..86fd457c60 100644
--- a/ext/standard/tests/serialize/bug73341.phpt
+++ b/ext/standard/tests/serialize/bug73341.phpt
@@ -3,7 +3,7 @@ Bug #73144 (Use-afte-free in ArrayObject Deserialization)
 --FILE--
 <?php
 try {
-$token = 'a:2:{i:0;O:1:"0":2:0s:1:"0";i:0;s:1:"0";a:1:{i:0;C:11:"ArrayObject":7:0x:i:0;r0';
+$token = 'a:2:{i:0;O:1:"0":2:0s:1:"0";i:0;s:1:"0";a:1:{i:0;C:11:"ArrayObject":7:{x:i:0;r}';
 $obj = unserialize($token);
 } catch(Exception $e) {
 	echo $e->getMessage()."\n";
@@ -21,4 +21,4 @@ unserialize($exploit);
 Error at offset 6 of 7 bytes
 
 Notice: ArrayObject::unserialize(): Unexpected end of serialized data in %sbug73341.php on line %d
-Error at offset 24 of 34 bytes
\ No newline at end of file
+Error at offset 24 of 34 bytes
diff --git a/ext/standard/var_unserializer.c b/ext/standard/var_unserializer.c
index 35c6160138..218a5c2273 100644
--- a/ext/standard/var_unserializer.c
+++ b/ext/standard/var_unserializer.c
@@ -1,4 +1,4 @@
-/* Generated by re2c 0.16 */
+/* Generated by re2c 1.0.1 */
 #line 1 "ext/standard/var_unserializer.re"
 /*
   +----------------------------------------------------------------------+
@@ -477,6 +477,13 @@ static inline int object_custom(UNSERIALIZE_PARAMETER, zend_class_entry *ce)
 		return 0;
 	}
 
+	/* Check that '}' is present before calling ce->unserialize() to mitigate issues
+	 * with unserialize reading past the end of the passed buffer if the string is not
+	 * appropriately terminated (usually NUL terminated, but '}' is also sufficient.) */
+	if ((*p)[datalen] != '}') {
+		return 0;
+	}
+
 	if (ce->unserialize == NULL) {
 		zend_error(E_WARNING, "Class %s has no unserializer", ZSTR_VAL(ce->name));
 		object_init_ex(rval, ce);
@@ -484,9 +491,8 @@ static inline int object_custom(UNSERIALIZE_PARAMETER, zend_class_entry *ce)
 		return 0;
 	}
 
-	(*p) += datalen;
-
-	return finish_nested_data(UNSERIALIZE_PASSTHRU);
+	(*p) += datalen + 1; /* +1 for '}' */
+	return 1;
 }
 
 static inline zend_long object_common1(UNSERIALIZE_PARAMETER, zend_class_entry *ce)
@@ -603,7 +609,7 @@ static int php_var_unserialize_internal(UNSERIALIZE_PARAMETER)
 	start = cursor;
 
 
-#line 607 "ext/standard/var_unserializer.c"
+#line 613 "ext/standard/var_unserializer.c"
 {
 	YYCTYPE yych;
 	static const unsigned char yybm[] = {
@@ -661,9 +667,9 @@ static int php_var_unserialize_internal(UNSERIALIZE_PARAMETER)
 yy2:
 	++YYCURSOR;
 yy3:
-#line 982 "ext/standard/var_unserializer.re"
+#line 988 "ext/standard/var_unserializer.re"
 	{ return 0; }
-#line 667 "ext/standard/var_unserializer.c"
+#line 673 "ext/standard/var_unserializer.c"
 yy4:
 	yych = *(YYMARKER = ++YYCURSOR);
 	if (yych == ':') goto yy17;
@@ -710,13 +716,13 @@ yy14:
 	goto yy3;
 yy15:
 	++YYCURSOR;
-#line 976 "ext/standard/var_unserializer.re"
+#line 982 "ext/standard/var_unserializer.re"
 	{
 	/* this is the case where we have less data than planned */
 	php_error_docref(NULL, E_NOTICE, "Unexpected end of serialized data");
 	return 0; /* not sure if it should be 0 or 1 here? */
 }
-#line 720 "ext/standard/var_unserializer.c"
+#line 726 "ext/standard/var_unserializer.c"
 yy17:
 	yych = *++YYCURSOR;
 	if (yybm[0+yych] & 128) {
@@ -728,13 +734,13 @@ yy18:
 	goto yy3;
 yy19:
 	++YYCURSOR;
-#line 660 "ext/standard/var_unserializer.re"
+#line 666 "ext/standard/var_unserializer.re"
 	{
 	*p = YYCURSOR;
 	ZVAL_NULL(rval);
 	return 1;
 }
-#line 738 "ext/standard/var_unserializer.c"
+#line 744 "ext/standard/var_unserializer.c"
 yy21:
 	yych = *++YYCURSOR;
 	if (yych <= ',') {
@@ -984,7 +990,7 @@ yy62:
 	goto yy18;
 yy63:
 	++YYCURSOR;
-#line 611 "ext/standard/var_unserializer.re"
+#line 617 "ext/standard/var_unserializer.re"
 	{
 	zend_long id;
 
@@ -1009,7 +1015,7 @@ yy63:
 
 	return 1;
 }
-#line 1013 "ext/standard/var_unserializer.c"
+#line 1019 "ext/standard/var_unserializer.c"
 yy65:
 	yych = *++YYCURSOR;
 	if (yych == '"') goto yy84;
@@ -1020,13 +1026,13 @@ yy66:
 	goto yy18;
 yy67:
 	++YYCURSOR;
-#line 666 "ext/standard/var_unserializer.re"
+#line 672 "ext/standard/var_unserializer.re"
 	{
 	*p = YYCURSOR;
 	ZVAL_BOOL(rval, parse_iv(start + 2));
 	return 1;
 }
-#line 1030 "ext/standard/var_unserializer.c"
+#line 1036 "ext/standard/var_unserializer.c"
 yy69:
 	++YYCURSOR;
 	if ((YYLIMIT - YYCURSOR) < 4) YYFILL(4);
@@ -1046,7 +1052,7 @@ yy69:
 	}
 yy71:
 	++YYCURSOR;
-#line 714 "ext/standard/var_unserializer.re"
+#line 720 "ext/standard/var_unserializer.re"
 	{
 #if SIZEOF_ZEND_LONG == 4
 use_double:
@@ -1055,7 +1061,7 @@ use_double:
 	ZVAL_DOUBLE(rval, zend_strtod((const char *)start + 2, NULL));
 	return 1;
 }
-#line 1059 "ext/standard/var_unserializer.c"
+#line 1065 "ext/standard/var_unserializer.c"
 yy73:
 	yych = *++YYCURSOR;
 	if (yych <= ',') {
@@ -1077,7 +1083,7 @@ yy75:
 	goto yy18;
 yy76:
 	++YYCURSOR;
-#line 672 "ext/standard/var_unserializer.re"
+#line 678 "ext/standard/var_unserializer.re"
 	{
 #if SIZEOF_ZEND_LONG == 4
 	int digits = YYCURSOR - start - 3;
@@ -1103,14 +1109,14 @@ yy76:
 	ZVAL_LONG(rval, parse_iv(start + 2));
 	return 1;
 }
-#line 1107 "ext/standard/var_unserializer.c"
+#line 1113 "ext/standard/var_unserializer.c"
 yy78:
 	yych = *++YYCURSOR;
 	if (yych == '"') goto yy92;
 	goto yy18;
 yy79:
 	++YYCURSOR;
-#line 636 "ext/standard/var_unserializer.re"
+#line 642 "ext/standard/var_unserializer.re"
 	{
 	zend_long id;
 
@@ -1134,14 +1140,14 @@ yy79:
 
 	return 1;
 }
-#line 1138 "ext/standard/var_unserializer.c"
+#line 1144 "ext/standard/var_unserializer.c"
 yy81:
 	yych = *++YYCURSOR;
 	if (yych == '"') goto yy94;
 	goto yy18;
 yy82:
 	++YYCURSOR;
-#line 824 "ext/standard/var_unserializer.re"
+#line 830 "ext/standard/var_unserializer.re"
 	{
 	size_t len, len2, len3, maxlen;
 	zend_long elements;
@@ -1293,10 +1299,10 @@ yy82:
 
 	return object_common2(UNSERIALIZE_PASSTHRU, elements);
 }
-#line 1297 "ext/standard/var_unserializer.c"
+#line 1303 "ext/standard/var_unserializer.c"
 yy84:
 	++YYCURSOR;
-#line 755 "ext/standard/var_unserializer.re"
+#line 761 "ext/standard/var_unserializer.re"
 	{
 	size_t len, maxlen;
 	zend_string *str;
@@ -1330,10 +1336,10 @@ yy84:
 	ZVAL_STR(rval, str);
 	return 1;
 }
-#line 1334 "ext/standard/var_unserializer.c"
+#line 1340 "ext/standard/var_unserializer.c"
 yy86:
 	++YYCURSOR;
-#line 789 "ext/standard/var_unserializer.re"
+#line 795 "ext/standard/var_unserializer.re"
 	{
 	zend_long elements = parse_iv(start + 2);
 	/* use iv() not uiv() in order to check data range */
@@ -1357,7 +1363,7 @@ yy86:
 
 	return finish_nested_data(UNSERIALIZE_PASSTHRU);
 }
-#line 1361 "ext/standard/var_unserializer.c"
+#line 1367 "ext/standard/var_unserializer.c"
 yy88:
 	yych = *++YYCURSOR;
 	if (yych <= ',') {
@@ -1382,7 +1388,7 @@ yy91:
 	goto yy18;
 yy92:
 	++YYCURSOR;
-#line 813 "ext/standard/var_unserializer.re"
+#line 819 "ext/standard/var_unserializer.re"
 	{
 	zend_long elements;
     if (!var_hash) return 0;
@@ -1393,10 +1399,10 @@ yy92:
 	}
 	return object_common2(UNSERIALIZE_PASSTHRU, elements);
 }
-#line 1397 "ext/standard/var_unserializer.c"
+#line 1403 "ext/standard/var_unserializer.c"
 yy94:
 	++YYCURSOR;
-#line 723 "ext/standard/var_unserializer.re"
+#line 729 "ext/standard/var_unserializer.re"
 	{
 	size_t len, maxlen;
 	char *str;
@@ -1428,7 +1434,7 @@ yy94:
 	ZVAL_STRINGL(rval, str, len);
 	return 1;
 }
-#line 1432 "ext/standard/var_unserializer.c"
+#line 1438 "ext/standard/var_unserializer.c"
 yy96:
 	yych = *++YYCURSOR;
 	if (yych <= '/') goto yy18;
@@ -1436,7 +1442,7 @@ yy96:
 	goto yy18;
 yy97:
 	++YYCURSOR;
-#line 698 "ext/standard/var_unserializer.re"
+#line 704 "ext/standard/var_unserializer.re"
 	{
 	*p = YYCURSOR;
 
@@ -1452,9 +1458,9 @@ yy97:
 
 	return 1;
 }
-#line 1456 "ext/standard/var_unserializer.c"
+#line 1462 "ext/standard/var_unserializer.c"
 }
-#line 984 "ext/standard/var_unserializer.re"
+#line 990 "ext/standard/var_unserializer.re"
 
 
 	return 0;
diff --git a/ext/standard/var_unserializer.re b/ext/standard/var_unserializer.re
index 5ff512bb68..221a03489a 100644
--- a/ext/standard/var_unserializer.re
+++ b/ext/standard/var_unserializer.re
@@ -481,6 +481,13 @@ static inline int object_custom(UNSERIALIZE_PARAMETER, zend_class_entry *ce)
 		return 0;
 	}
 
+	/* Check that '}' is present before calling ce->unserialize() to mitigate issues
+	 * with unserialize reading past the end of the passed buffer if the string is not
+	 * appropriately terminated (usually NUL terminated, but '}' is also sufficient.) */
+	if ((*p)[datalen] != '}') {
+		return 0;
+	}
+
 	if (ce->unserialize == NULL) {
 		zend_error(E_WARNING, "Class %s has no unserializer", ZSTR_VAL(ce->name));
 		object_init_ex(rval, ce);
@@ -488,9 +495,8 @@ static inline int object_custom(UNSERIALIZE_PARAMETER, zend_class_entry *ce)
 		return 0;
 	}
 
-	(*p) += datalen;
-
-	return finish_nested_data(UNSERIALIZE_PASSTHRU);
+	(*p) += datalen + 1; /* +1 for '}' */
+	return 1;
 }
 
 static inline zend_long object_common1(UNSERIALIZE_PARAMETER, zend_class_entry *ce)