Blob Blame History Raw
From e8429400d40e3c3aa4b22ba701991d698a2f3b2f Mon Sep 17 00:00:00 2001
From: Stanislav Malyshev <stas@php.net>
Date: Mon, 31 Aug 2015 21:28:11 -0700
Subject: [PATCH] Fix bug #70172 - Use After Free Vulnerability in
 unserialize()

---
 ext/standard/tests/serialize/bug70172.phpt | 52 ++++++++++++++++++++
 ext/standard/var.c                         | 23 +++++++--
 ext/standard/var_unserializer.c            | 76 ++++++++++++++++--------------
 ext/standard/var_unserializer.re           | 12 +++--
 4 files changed, 121 insertions(+), 42 deletions(-)
 create mode 100644 ext/standard/tests/serialize/bug70172.phpt

diff --git a/ext/standard/tests/serialize/bug70172.phpt b/ext/standard/tests/serialize/bug70172.phpt
new file mode 100644
index 0000000..0e9d7ed
--- /dev/null
+++ b/ext/standard/tests/serialize/bug70172.phpt
@@ -0,0 +1,52 @@
+--TEST--
+Bug #70172 - Use After Free Vulnerability in unserialize()
+--FILE--
+<?php
+class obj implements Serializable {
+	var $data;
+	function serialize() {
+		return serialize($this->data);
+	}
+	function unserialize($data) {
+		$this->data = unserialize($data);
+	}
+}
+
+$fakezval = ptr2str(1122334455);
+$fakezval .= ptr2str(0);
+$fakezval .= "\x00\x00\x00\x00";
+$fakezval .= "\x01";
+$fakezval .= "\x00";
+$fakezval .= "\x00\x00";
+
+$inner = 'r:2;';
+$exploit = 'a:2:{i:0;i:1;i:1;C:3:"obj":'.strlen($inner).':{'.$inner.'}}';
+
+$data = unserialize($exploit);
+
+for ($i = 0; $i < 5; $i++) {
+	$v[$i] = $fakezval.$i;
+}
+
+var_dump($data);
+
+function ptr2str($ptr)
+{
+	$out = '';
+	for ($i = 0; $i < 8; $i++) {
+		$out .= chr($ptr & 0xff);
+		$ptr >>= 8;
+	}
+	return $out;
+}
+?>
+--EXPECTF--
+array(2) {
+  [0]=>
+  int(1)
+  [1]=>
+  object(obj)#%d (1) {
+    ["data"]=>
+    int(1)
+  }
+}
\ No newline at end of file
diff --git a/ext/standard/var.c b/ext/standard/var.c
index 7603ff2..33b976f 100644
--- a/ext/standard/var.c
+++ b/ext/standard/var.c
@@ -951,6 +951,8 @@ PHP_FUNCTION(unserialize)
 	int buf_len;
 	const unsigned char *p;
 	php_unserialize_data_t var_hash;
+	int oldlevel;
+	zval *old_rval = return_value;
 
 	if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s", &buf, &buf_len) == FAILURE) {
 		RETURN_FALSE;
@@ -970,6 +972,19 @@ PHP_FUNCTION(unserialize)
 		}
 		RETURN_FALSE;
 	}
+	if (return_value != old_rval) {
+		/*
+		 * Terrible hack due to the fact that executor passes us zval *,
+		 * but unserialize with r/R wants to replace it with another zval *
+		 */
+		zval_dtor(old_rval);
+		*old_rval = *return_value;
+		zval_copy_ctor(old_rval);
+		var_push_dtor_no_addref(&var_hash, &return_value);
+		var_push_dtor_no_addref(&var_hash, &old_rval);
+	} else {
+		var_push_dtor(&var_hash, &return_value);
+	}
 	PHP_VAR_UNSERIALIZE_DESTROY(var_hash);
 }
 /* }}} */
diff --git a/ext/standard/var_unserializer.c b/ext/standard/var_unserializer.c
index ffaf680..5f2336e 100644
--- a/ext/standard/var_unserializer.c
+++ b/ext/standard/var_unserializer.c
@@ -67,7 +67,7 @@
 
 	var_hash = (*var_hashx)->last_dtor;
 #if VAR_ENTRIES_DBG
-	fprintf(stderr, "var_push_dtor(%ld): %d\n", var_hash?var_hash->used_slots:-1L, Z_TYPE_PP(rval));
+	fprintf(stderr, "var_push_dtor(%p, %ld): %d\n", *rval, var_hash?var_hash->used_slots:-1L, Z_TYPE_PP(rval));
 #endif
 
 	if (!var_hash || var_hash->used_slots == VAR_ENTRIES_MAX) {
@@ -98,7 +98,7 @@
 
     var_hash = (*var_hashx)->last_dtor;
 #if VAR_ENTRIES_DBG
-	fprintf(stderr, "var_push_dtor_no_addref(%ld): %d (%d)\n", var_hash?var_hash->used_slots:-1L, Z_TYPE_PP(rval), Z_REFCOUNT_PP(rval));
+	fprintf(stderr, "var_push_dtor_no_addref(%p, %ld): %d (%d)\n", *rval, var_hash?var_hash->used_slots:-1L, Z_TYPE_PP(rval), Z_REFCOUNT_PP(rval));
 #endif
 
 	if (!var_hash || var_hash->used_slots == VAR_ENTRIES_MAX) {
@@ -177,6 +177,9 @@
 	
 	while (var_hash) {
 		for (i = 0; i < var_hash->used_slots; i++) {
+#if VAR_ENTRIES_DBG
+    fprintf(stderr, "var_destroy dtor(%p, %ld)\n", var_hash->data[i], Z_REFCOUNT_P(var_hash->data[i]));
+#endif
 			zval_ptr_dtor(&var_hash->data[i]);
 		}
 		next = var_hash->next;
@@ -629,6 +632,7 @@
 	zval **args[1];
 	zval *arg_func_name;
 
+    if (!var_hash) return 0;
 	if (*start == 'C') {
 		custom_object = 1;
 	}
@@ -784,6 +788,7 @@
 	if (yych != '"') goto yy18;
 	++YYCURSOR;
 	{
+    if (!var_hash) return 0;
 
 	INIT_PZVAL(*rval);
 	
@@ -814,6 +819,7 @@
 	long elements = parse_iv(start + 2);
 	/* use iv() not uiv() in order to check data range */
 	*p = YYCURSOR;
+    if (!var_hash) return 0;
 
 	if (elements < 0) {
 		return 0;
@@ -1243,7 +1249,7 @@
 	}
 
 	if (*rval != NULL) {
-		zval_ptr_dtor(rval);
+		var_push_dtor_no_addref(var_hash, rval);
 	}
 	*rval = *rval_ref;
 	Z_ADDREF_PP(rval);
diff --git a/ext/standard/var_unserializer.re b/ext/standard/var_unserializer.re
index f02602c..ed82152 100644
--- a/ext/standard/var_unserializer.re
+++ b/ext/standard/var_unserializer.re
@@ -66,7 +66,7 @@
 
 	var_hash = (*var_hashx)->last_dtor;
 #if VAR_ENTRIES_DBG
-	fprintf(stderr, "var_push_dtor(%ld): %d\n", var_hash?var_hash->used_slots:-1L, Z_TYPE_PP(rval));
+	fprintf(stderr, "var_push_dtor(%p, %ld): %d\n", *rval, var_hash?var_hash->used_slots:-1L, Z_TYPE_PP(rval));
 #endif
 
 	if (!var_hash || var_hash->used_slots == VAR_ENTRIES_MAX) {
@@ -97,7 +97,7 @@
 
     var_hash = (*var_hashx)->last_dtor;
 #if VAR_ENTRIES_DBG
-	fprintf(stderr, "var_push_dtor_no_addref(%ld): %d (%d)\n", var_hash?var_hash->used_slots:-1L, Z_TYPE_PP(rval), Z_REFCOUNT_PP(rval));
+	fprintf(stderr, "var_push_dtor_no_addref(%p, %ld): %d (%d)\n", *rval, var_hash?var_hash->used_slots:-1L, Z_TYPE_PP(rval), Z_REFCOUNT_PP(rval));
 #endif
 
 	if (!var_hash || var_hash->used_slots == VAR_ENTRIES_MAX) {
@@ -176,6 +176,9 @@
 	
 	while (var_hash) {
 		for (i = 0; i < var_hash->used_slots; i++) {
+#if VAR_ENTRIES_DBG
+    fprintf(stderr, "var_destroy dtor(%p, %ld)\n", var_hash->data[i], Z_REFCOUNT_P(var_hash->data[i]));
+#endif
 			zval_ptr_dtor(&var_hash->data[i]);
 		}
 		next = var_hash->next;
@@ -496,7 +499,7 @@
 	}
 
 	if (*rval != NULL) {
-		zval_ptr_dtor(rval);
+		var_push_dtor_no_addref(var_hash, rval);
 	}
 	*rval = *rval_ref;
 	Z_ADDREF_PP(rval);
@@ -655,6 +658,7 @@
 	long elements = parse_iv(start + 2);
 	/* use iv() not uiv() in order to check data range */
 	*p = YYCURSOR;
+    if (!var_hash) return 0;
 
 	if (elements < 0) {
 		return 0;
@@ -672,6 +676,7 @@
 }
 
 "o:" iv ":" ["] {
+    if (!var_hash) return 0;
 
 	INIT_PZVAL(*rval);
 	
@@ -694,6 +699,7 @@
 	zval **args[1];
 	zval *arg_func_name;
 
+    if (!var_hash) return 0;
 	if (*start == 'C') {
 		custom_object = 1;
 	}
-- 
2.1.4

From 7c31203935589ab4fcb104041ef9d87f747bfee4 Mon Sep 17 00:00:00 2001
From: Stanislav Malyshev <stas@php.net>
Date: Tue, 1 Sep 2015 11:38:15 -0700
Subject: [PATCH] Improve fix for #70172

---
 ext/standard/tests/serialize/bug70172.phpt   |  2 +
 ext/standard/tests/serialize/bug70172_2.phpt | 68 ++++++++++++++++++++++++++++
 ext/standard/var.c                           |  3 +-
 3 files changed, 72 insertions(+), 1 deletion(-)
 create mode 100644 ext/standard/tests/serialize/bug70172_2.phpt

diff --git a/ext/standard/tests/serialize/bug70172.phpt b/ext/standard/tests/serialize/bug70172.phpt
index 0e9d7ed..0a4aa4b 100644
--- a/ext/standard/tests/serialize/bug70172.phpt
+++ b/ext/standard/tests/serialize/bug70172.phpt
@@ -1,5 +1,7 @@
 --TEST--
 Bug #70172 - Use After Free Vulnerability in unserialize()
+--XFAIL--
+Memory leak on debug build, needs fix.
 --FILE--
 <?php
 class obj implements Serializable {
diff --git a/ext/standard/tests/serialize/bug70172_2.phpt b/ext/standard/tests/serialize/bug70172_2.phpt
new file mode 100644
index 0000000..ae48341
--- /dev/null
+++ b/ext/standard/tests/serialize/bug70172_2.phpt
@@ -0,0 +1,68 @@
+--TEST--
+Bug #70172 - Use After Free Vulnerability in unserialize()
+--FILE--
+<?php
+class obj implements Serializable {
+	var $data;
+	function serialize() {
+		return serialize($this->data);
+	}
+	function unserialize($data) {
+		$this->data = unserialize($data);
+	}
+}
+
+class obj2 {
+	var $ryat;
+	function __wakeup() {
+		$this->ryat = 1;
+	}
+}
+
+$fakezval = ptr2str(1122334455);
+$fakezval .= ptr2str(0);
+$fakezval .= "\x00\x00\x00\x00";
+$fakezval .= "\x01";
+$fakezval .= "\x00";
+$fakezval .= "\x00\x00";
+
+$inner = 'r:2;';
+$exploit = 'a:2:{i:0;O:4:"obj2":1:{s:4:"ryat";C:3:"obj":'.strlen($inner).':{'.$inner.'}}i:1;a:1:{i:0;a:1:{i:0;R:4;}}}';
+
+$data = unserialize($exploit);
+
+for ($i = 0; $i < 5; $i++) {
+	$v[$i] = $fakezval.$i;
+}
+
+var_dump($data);
+
+function ptr2str($ptr)
+{
+	$out = '';
+	for ($i = 0; $i < 8; $i++) {
+		$out .= chr($ptr & 0xff);
+		$ptr >>= 8;
+	}
+	return $out;
+}
+?>
+--EXPECTF--
+array(2) {
+  [0]=>
+  object(obj2)#%d (1) {
+    ["ryat"]=>
+    int(1)
+  }
+  [1]=>
+  array(1) {
+    [0]=>
+    array(1) {
+      [0]=>
+      object(obj2)#%d (1) {
+        ["ryat"]=>
+        int(1)
+      }
+    }
+  }
+}
\ No newline at end of file
diff --git a/ext/standard/var.c b/ext/standard/var.c
index 33b976f..113b8cb 100644
--- a/ext/standard/var.c
+++ b/ext/standard/var.c
@@ -981,7 +981,8 @@ PHP_FUNCTION(unserialize)
 		*old_rval = *return_value;
 		zval_copy_ctor(old_rval);
 		var_push_dtor_no_addref(&var_hash, &return_value);
-		var_push_dtor_no_addref(&var_hash, &old_rval);
+		/* FIXME: old_rval is not freed in some scenarios, see bug #70172
+		   var_push_dtor_no_addref(&var_hash, &old_rval); */
 	} else {
 		var_push_dtor(&var_hash, &return_value);
 	}
-- 
2.1.4