From: Stanislav Malyshev <stas@php.net>
Date: Sun, 29 Nov 2009 08:35:01 +0000 (+0000)
Subject: proper fix for bug #50006
X-Git-Tag: php-5.4.0alpha1~191^2~2326
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=837f46854cea1c897a6407ad9eb849e4c486efbe;p=php

proper fix for bug #50006
add modify protection to all user array sorts
---

diff --git a/ext/standard/array.c b/ext/standard/array.c
index d122faa648..669af82429 100644
--- a/ext/standard/array.c
+++ b/ext/standard/array.c
@@ -683,6 +683,7 @@ PHP_FUNCTION(usort)
 PHP_FUNCTION(uasort)
 {
 	zval *array;
+	int refcount;
 	PHP_ARRAY_CMP_FUNC_VARS;
 
 	PHP_ARRAY_CMP_FUNC_BACKUP();
@@ -692,12 +693,31 @@ PHP_FUNCTION(uasort)
 		return;
 	}
 
+	/* Clear the is_ref flag, so the attemts to modify the array in user
+	 * comaprison function will create a copy of array and won't affect the
+	 * original array. The fact of modification is detected using refcount
+	 * comparison. The result of sorting in such case is undefined and the
+	 * function returns FALSE.
+	 */
+	Z_UNSET_ISREF_P(array);
+	refcount = Z_REFCOUNT_P(array);
+
 	if (zend_hash_sort(Z_ARRVAL_P(array), zend_qsort, php_array_user_compare, 0 TSRMLS_CC) == FAILURE) {
-		PHP_ARRAY_CMP_FUNC_RESTORE();
-		RETURN_FALSE;
+		RETVAL_FALSE;
+	} else {
+		if (refcount > Z_REFCOUNT_P(array)) {
+			php_error_docref(NULL TSRMLS_CC, E_WARNING, "Array was modified by the user comparison function");
+			RETVAL_FALSE;
+		} else {
+			RETVAL_TRUE;
+		}
 	}
+	
+	if (Z_REFCOUNT_P(array) > 1) {
+		Z_SET_ISREF_P(array);
+	}
+
 	PHP_ARRAY_CMP_FUNC_RESTORE();
-	RETURN_TRUE;
 }
 /* }}} */
 
@@ -767,6 +787,7 @@ static int php_array_user_key_compare(const void *a, const void *b TSRMLS_DC) /*
 PHP_FUNCTION(uksort)
 {
 	zval *array;
+	int refcount;
 	PHP_ARRAY_CMP_FUNC_VARS;
 
 	PHP_ARRAY_CMP_FUNC_BACKUP();
@@ -776,14 +797,31 @@ PHP_FUNCTION(uksort)
 		return;
 	}
 
-	if (zend_hash_sort(Z_ARRVAL_P(array), zend_qsort, php_array_user_key_compare, 0 TSRMLS_CC) == FAILURE) {
-		PHP_ARRAY_CMP_FUNC_RESTORE();
+	/* Clear the is_ref flag, so the attemts to modify the array in user
+	 * comaprison function will create a copy of array and won't affect the
+	 * original array. The fact of modification is detected using refcount
+	 * comparison. The result of sorting in such case is undefined and the
+	 * function returns FALSE.
+	 */
+	Z_UNSET_ISREF_P(array);
+	refcount = Z_REFCOUNT_P(array);
 
-		RETURN_FALSE;
+	if (zend_hash_sort(Z_ARRVAL_P(array), zend_qsort, php_array_user_key_compare, 0 TSRMLS_CC) == FAILURE) {
+		RETVAL_FALSE;
+	} else {
+		if (refcount > Z_REFCOUNT_P(array)) {
+			php_error_docref(NULL TSRMLS_CC, E_WARNING, "Array was modified by the user comparison function");
+			RETVAL_FALSE;
+		} else {
+			RETVAL_TRUE;
+		}
+	}
+	
+	if (Z_REFCOUNT_P(array) > 1) {
+		Z_SET_ISREF_P(array);
 	}
 
 	PHP_ARRAY_CMP_FUNC_RESTORE();
-	RETURN_TRUE;
 }
 /* }}} */
 
diff --git a/ext/standard/tests/array/bug50006_1.phpt b/ext/standard/tests/array/bug50006_1.phpt
new file mode 100644
index 0000000000..fbb7ddd594
--- /dev/null
+++ b/ext/standard/tests/array/bug50006_1.phpt
@@ -0,0 +1,29 @@
+--TEST--
+Bug #50006 (Segfault caused by uksort()) - usort variant
+--FILE--
+<?php
+
+$data = array(
+    'bar-bazbazbaz.',
+    'bar-bazbazbaz-', 
+    'foo'
+);
+usort($data, 'magic_sort_cmp');
+print_r($data);
+
+function magic_sort_cmp($a, $b) {
+  $a = substr($a, 1);
+  $b = substr($b, 1);
+  if (!$a) return $b ? -1 : 0;
+  if (!$b) return 1;
+  return magic_sort_cmp($a, $b);
+}
+
+?>
+--EXPECTF--
+Array
+(
+    [0] => foo
+    [1] => bar-bazbazbaz-
+    [2] => bar-bazbazbaz.
+)
diff --git a/ext/standard/tests/array/bug50006_2.phpt b/ext/standard/tests/array/bug50006_2.phpt
new file mode 100644
index 0000000000..19c0d14252
--- /dev/null
+++ b/ext/standard/tests/array/bug50006_2.phpt
@@ -0,0 +1,29 @@
+--TEST--
+Bug #50006 (Segfault caused by uksort()) - uasort variant
+--FILE--
+<?php
+
+$data = array(
+    'bar-bazbazbaz.',
+    'bar-bazbazbaz-', 
+    'foo'
+);
+uasort($data, 'magic_sort_cmp');
+print_r($data);
+
+function magic_sort_cmp($a, $b) {
+  $a = substr($a, 1);
+  $b = substr($b, 1);
+  if (!$a) return $b ? -1 : 0;
+  if (!$b) return 1;
+  return magic_sort_cmp($a, $b);
+}
+
+?>
+--EXPECTF--
+Array
+(
+    [2] => foo
+    [1] => bar-bazbazbaz-
+    [0] => bar-bazbazbaz.
+)
diff --git a/ext/standard/tests/array/unexpected_array_mod_bug.phpt b/ext/standard/tests/array/unexpected_array_mod_bug.phpt
new file mode 100755
index 0000000000..58f2249205
--- /dev/null
+++ b/ext/standard/tests/array/unexpected_array_mod_bug.phpt
@@ -0,0 +1,21 @@
+--TEST--
+Crash when function parameter modified via reference
+--FILE--
+<?php
+function usercompare($a,$b) {
+  unset($GLOBALS['my_var'][2]); 
+  return 0;
+}
+$my_var = array(1 => "entry_1",
+2 => "entry_2",
+3 => "entry_3",
+4 => "entry_4",
+5 => "entry_5");
+usort($my_var, "usercompare");
+
+echo "Done.\n";
+?>
+--EXPECTF--
+
+Warning: usort(): Array was modified by the user comparison function in %s on line %d
+Done.