CVE-2016-9934: NULL Pointer Dereference (CWE-476)
value: ext/wddx/wddx.c in PHP before 5.6.28 and 7.x before 7.0.13 allows remote attackers to cause a denial of service (NULL pointer dereference) via crafted serialized data in a wddxPacket XML document, as demonstrated by a PDORow string.
CWE description: The product dereferences a pointer that it expects to be valid but is NULL.
CVSS v3 base score: 7.5
Published: 2017-01-04T20:59Z
Repository: https://github.com/php/php-src
Language: Other
Files changed: ext/pdo/pdo_stmt.c, ext/wddx/tests/bug45901.phpt, ext/wddx/tests/bug72790.phpt, ext/wddx/tests/bug73331.phpt, ext/wddx/wddx.c
Commit date: 2016-10-23 20:09:23 -0700
Commit message: Fix bug #73331 - do not try to serialize/unserialize objects wddx can not handle

Proper soltion would be to call serialize/unserialize and deal with the result,
but this requires more work that should be done by wddx maintainer (not me).

Diff: diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c
index f5c295c26c3..9f9ada9be3b 100644
--- a/ext/pdo/pdo_stmt.c
+++ b/ext/pdo/pdo_stmt.c
@@ -2336,10 +2336,11 @@ void pdo_stmt_init(TSRMLS_D)
 	INIT_CLASS_ENTRY(ce, "PDORow", pdo_row_functions);
 	pdo_row_ce = zend_register_internal_class(&ce TSRMLS_CC);
 	pdo_row_ce->ce_flags |= ZEND_ACC_FINAL_CLASS; /* when removing this a lot of handlers need to be redone */
 	pdo_row_ce->create_object = pdo_row_new;
 	pdo_row_ce->serialize = pdo_row_serialize;
+	pdo_row_ce->unserialize = zend_class_unserialize_deny;
 }
 
 static void free_statement(pdo_stmt_t *stmt TSRMLS_DC)
 {
 	if (stmt->bound_params) {
diff --git a/ext/wddx/tests/bug45901.phpt b/ext/wddx/tests/bug45901.phpt
index 4084ccbf202..e76e47eab72 100644
--- a/ext/wddx/tests/bug45901.phpt
+++ b/ext/wddx/tests/bug45901.phpt
@@ -12,7 +12,9 @@
 $xml->addChild('test');
 echo wddx_serialize_value($xml, 'Variables') . "\n";
 echo "DONE";
 ?>
 --EXPECTF--
-<wddxPacket version='1.0'><header><comment>Variables</comment></header><data><struct><var name='php_class_name'><string>SimpleXMLElement</string></var><var name='test'><struct><var name='php_class_name'><string>SimpleXMLElement</string></var></struct></var></struct></data></wddxPacket>
+
+Warning: wddx_serialize_value(): Class SimpleXMLElement can not be serialized in %sbug45901.php on line %d
+<wddxPacket version='1.0'><header><comment>Variables</comment></header><data></data></wddxPacket>
 DONE
\ No newline at end of file
diff --git a/ext/wddx/tests/bug72790.phpt b/ext/wddx/tests/bug72790.phpt
index a60524bdaf1..3b08e58cc68 100644
--- a/ext/wddx/tests/bug72790.phpt
+++ b/ext/wddx/tests/bug72790.phpt
@@ -1,7 +1,7 @@
 --TEST--
-Bug 72790: wddx_deserialize null dereference with invalid xml
+Bug #72790: wddx_deserialize null dereference with invalid xml
 --SKIPIF--
 <?php
 if (!extension_loaded('wddx')) {
     die('skip. wddx not available');
 }
diff --git a/ext/wddx/tests/bug73331.phpt b/ext/wddx/tests/bug73331.phpt
new file mode 100644
index 00000000000..7e4cda1ea11
--- /dev/null
+++ b/ext/wddx/tests/bug73331.phpt
@@ -0,0 +1,15 @@
+--TEST--
+Bug #73331 (NULL Pointer Dereference in WDDX Packet Deserialization with PDORow)
+--SKIPIF--
+<?php if (!extension_loaded("wddx") || !extension_loaded("pdo")) print "skip"; ?>
+--FILE--
+<?php
+
+$wddx = "<wddxPacket version='1.0'><header/><data><struct><var name='php_class_name'><string>PDORow</string></var></struct></data></wddxPacket>";
+var_dump(wddx_deserialize($wddx));
+?>
+--EXPECTF--
+
+Warning: wddx_deserialize(): Class pdorow can not be unserialized in %s73331.php on line %d
+NULL
+
diff --git a/ext/wddx/wddx.c b/ext/wddx/wddx.c
index 0e77826ce84..59cc8ddda8a 100644
--- a/ext/wddx/wddx.c
+++ b/ext/wddx/wddx.c
@@ -469,35 +469,38 @@ static void php_wddx_serialize_object(wddx_packet *packet, zval *obj)
 	zval *retval = NULL;
 	const char *key;
 	ulong idx;
 	char tmp_buf[WDDX_BUF_LEN];
 	HashTable *objhash, *sleephash;
+	zend_class_entry *ce;
+	PHP_CLASS_ATTRIBUTES;
 	TSRMLS_FETCH();
 
+	PHP_SET_CLASS_ATTRIBUTES(obj);
+	ce = Z_OBJCE_P(obj);
+	if (!ce || ce->serialize || ce->unserialize) {
+		php_error_docref(NULL TSRMLS_CC, E_WARNING, "Class %s can not be serialized", class_name);
+		PHP_CLEANUP_CLASS_ATTRIBUTES();
+		return;
+	}
+
 	MAKE_STD_ZVAL(fname);
 	ZVAL_STRING(fname, "__sleep", 1);
-
 	/*
 	 * We try to call __sleep() method on object. It's supposed to return an
 	 * array of property names to be serialized.
 	 */
 	if (call_user_function_ex(CG(function_table), &obj, fname, &retval, 0, 0, 1, NULL TSRMLS_CC) == SUCCESS) {
 		if (retval && (sleephash = HASH_OF(retval))) {
-			PHP_CLASS_ATTRIBUTES;
-
-			PHP_SET_CLASS_ATTRIBUTES(obj);
-
 			php_wddx_add_chunk_static(packet, WDDX_STRUCT_S);
 			snprintf(tmp_buf, WDDX_BUF_LEN, WDDX_VAR_S, PHP_CLASS_NAME_VAR);
 			php_wddx_add_chunk(packet, tmp_buf);
 			php_wddx_add_chunk_static(packet, WDDX_STRING_S);
 			php_wddx_add_chunk_ex(packet, class_name, name_len);
 			php_wddx_add_chunk_static(packet, WDDX_STRING_E);
 			php_wddx_add_chunk_static(packet, WDDX_VAR_E);
 
-			PHP_CLEANUP_CLASS_ATTRIBUTES();
-
 			objhash = HASH_OF(obj);
 
 			for (zend_hash_internal_pointer_reset(sleephash);
 				 zend_hash_get_current_data(sleephash, (void **)&varname) == SUCCESS;
 				 zend_hash_move_forward(sleephash)) {
@@ -514,24 +517,18 @@ static void php_wddx_serialize_object(wddx_packet *packet, zval *obj)
 			php_wddx_add_chunk_static(packet, WDDX_STRUCT_E);
 		}
 	} else {
 		uint key_len;
 
-		PHP_CLASS_ATTRIBUTES;
-
-		PHP_SET_CLASS_ATTRIBUTES(obj);
-
 		php_wddx_add_chunk_static(packet, WDDX_STRUCT_S);
 		snprintf(tmp_buf, WDDX_BUF_LEN, WDDX_VAR_S, PHP_CLASS_NAME_VAR);
 		php_wddx_add_chunk(packet, tmp_buf);
 		php_wddx_add_chunk_static(packet, WDDX_STRING_S);
 		php_wddx_add_chunk_ex(packet, class_name, name_len);
 		php_wddx_add_chunk_static(packet, WDDX_STRING_E);
 		php_wddx_add_chunk_static(packet, WDDX_VAR_E);
 
-		PHP_CLEANUP_CLASS_ATTRIBUTES();
-
 		objhash = HASH_OF(obj);
 		for (zend_hash_internal_pointer_reset(objhash);
 			 zend_hash_get_current_data(objhash, (void**)&ent) == SUCCESS;
 			 zend_hash_move_forward(objhash)) {
 			if (*ent == obj) {
@@ -549,10 +546,12 @@ static void php_wddx_serialize_object(wddx_packet *packet, zval *obj)
 			}
 		}
 		php_wddx_add_chunk_static(packet, WDDX_STRUCT_E);
 	}
 
+	PHP_CLEANUP_CLASS_ATTRIBUTES();
+
 	zval_dtor(fname);
 	FREE_ZVAL(fname);
 
 	if (retval) {
 		zval_ptr_dtor(&retval);
@@ -1010,30 +1009,34 @@ static void php_wddx_pop_element(void *user_data, const XML_Char *name)
 										   Z_STRLEN_P(ent1->data)+1, (void **) &pce)==FAILURE) {
 							incomplete_class = 1;
 							pce = &PHP_IC_ENTRY;
 						}
 
-						/* Initialize target object */
-						MAKE_STD_ZVAL(obj);
-						object_init_ex(obj, *pce);
-
-						/* Merge current hashtable with object's default properties */
-						zend_hash_merge(Z_OBJPROP_P(obj),
-										Z_ARRVAL_P(ent2->data),
-										(void (*)(void *)) zval_add_ref,
-										(void *) &tmp, sizeof(zval *), 0);
-
-						if (incomplete_class) {
-							php_store_class_name(obj, Z_STRVAL_P(ent1->data), Z_STRLEN_P(ent1->data));
+						if (pce != &PHP_IC_ENTRY && ((*pce)->serialize || (*pce)->unserialize)) {
+							ent2->data = NULL;
+							php_error_docref(NULL TSRMLS_CC, E_WARNING, "Class %s can not be unserialized", Z_STRVAL_P(ent1->data));
+						} else {
+							/* Initialize target object */
+							MAKE_STD_ZVAL(obj);
+							object_init_ex(obj, *pce);
+
+							/* Merge current hashtable with object's default properties */
+							zend_hash_merge(Z_OBJPROP_P(obj),
+											Z_ARRVAL_P(ent2->data),
+											(void (*)(void *)) zval_add_ref,
+											(void *) &tmp, sizeof(zval *), 0);
+
+							if (incomplete_class) {
+								php_store_class_name(obj, Z_STRVAL_P(ent1->data), Z_STRLEN_P(ent1->data));
+							}
+
+							/* Clean up old array entry */
+							zval_ptr_dtor(&ent2->data);
+
+							/* Set stack entry to point to the newly created object */
+							ent2->data = obj;
 						}
-
-						/* Clean up old array entry */
-						zval_ptr_dtor(&ent2->data);
-
-						/* Set stack entry to point to the newly created object */
-						ent2->data = obj;
-
 						/* Clean up class name var entry */
 						zval_ptr_dtor(&ent1->data);
 					} else if (Z_TYPE_P(ent2->data) == IS_OBJECT) {
 						zend_class_entry *old_scope = EG(scope);
 

Vulnerable code: <wddxPacket version='1.0'><header><comment>Variables</comment></header><data><struct><var name='php_class_name'><string>SimpleXMLElement</string></var><var name='test'><struct><var name='php_class_name'><string>SimpleXMLElement</string></var></struct></var></struct></data></wddxPacket>
Bug 72790: wddx_deserialize null dereference with invalid xml

			PHP_CLASS_ATTRIBUTES;

			PHP_SET_CLASS_ATTRIBUTES(obj);

			PHP_CLEANUP_CLASS_ATTRIBUTES();

		PHP_CLASS_ATTRIBUTES;

		PHP_SET_CLASS_ATTRIBUTES(obj);

		PHP_CLEANUP_CLASS_ATTRIBUTES();

						/* Initialize target object */
						MAKE_STD_ZVAL(obj);
						object_init_ex(obj, *pce);

						/* Merge current hashtable with object's default properties */
						zend_hash_merge(Z_OBJPROP_P(obj),
										Z_ARRVAL_P(ent2->data),
										(void (*)(void *)) zval_add_ref,
										(void *) &tmp, sizeof(zval *), 0);

						if (incomplete_class) {
							php_store_class_name(obj, Z_STRVAL_P(ent1->data), Z_STRLEN_P(ent1->data));

						/* Clean up old array entry */
						zval_ptr_dtor(&ent2->data);

						/* Set stack entry to point to the newly created object */
						ent2->data = obj;

Fixed code: 	pdo_row_ce->unserialize = zend_class_unserialize_deny;

Warning: wddx_serialize_value(): Class SimpleXMLElement can not be serialized in %sbug45901.php on line %d
<wddxPacket version='1.0'><header><comment>Variables</comment></header><data></data></wddxPacket>
Bug #72790: wddx_deserialize null dereference with invalid xml
--TEST--
Bug #73331 (NULL Pointer Dereference in WDDX Packet Deserialization with PDORow)
--SKIPIF--
<?php if (!extension_loaded("wddx") || !extension_loaded("pdo")) print "skip"; ?>
--FILE--
<?php

$wddx = "<wddxPacket version='1.0'><header/><data><struct><var name='php_class_name'><string>PDORow</string></var></struct></data></wddxPacket>";
var_dump(wddx_deserialize($wddx));
?>
--EXPECTF--

Warning: wddx_deserialize(): Class pdorow can not be unserialized in %s73331.php on line %d
NULL

	zend_class_entry *ce;
	PHP_CLASS_ATTRIBUTES;
	PHP_SET_CLASS_ATTRIBUTES(obj);
	ce = Z_OBJCE_P(obj);
	if (!ce || ce->serialize || ce->unserialize) {
		php_error_docref(NULL TSRMLS_CC, E_WARNING, "Class %s can not be serialized", class_name);
		PHP_CLEANUP_CLASS_ATTRIBUTES();
		return;
	}

	PHP_CLEANUP_CLASS_ATTRIBUTES();

						if (pce != &PHP_IC_ENTRY && ((*pce)->serialize || (*pce)->unserialize)) {
							ent2->data = NULL;
							php_error_docref(NULL TSRMLS_CC, E_WARNING, "Class %s can not be unserialized", Z_STRVAL_P(ent1->data));
						} else {
							/* Initialize target object */
							MAKE_STD_ZVAL(obj);
							object_init_ex(obj, *pce);

							/* Merge current hashtable with object's default properties */
							zend_hash_merge(Z_OBJPROP_P(obj),
											Z_ARRVAL_P(ent2->data),
											(void (*)(void *)) zval_add_ref,
											(void *) &tmp, sizeof(zval *), 0);

							if (incomplete_class) {
								php_store_class_name(obj, Z_STRVAL_P(ent1->data), Z_STRLEN_P(ent1->data));
							}

							/* Clean up old array entry */
							zval_ptr_dtor(&ent2->data);

							/* Set stack entry to point to the newly created object */
							ent2->data = obj;