Skip to content

Instantly share code, notes, and snippets.

/72434.diff Secret
Created Jun 21, 2016

Embed
What would you like to do?
Patch for 72434
commit f6aef68089221c5ea047d4a74224ee3deead99a6
Author: Stanislav Malyshev <stas@php.net>
Date: Mon Jun 20 21:35:22 2016 -0700
Fix bug #72434: ZipArchive class Use After Free Vulnerability in PHP's GC algorithm and unserialize
diff --git a/ext/standard/tests/strings/bug72434.phpt b/ext/standard/tests/strings/bug72434.phpt
new file mode 100644
index 0000000..1408b8f
--- /dev/null
+++ b/ext/standard/tests/strings/bug72434.phpt
@@ -0,0 +1,33 @@
+--TEST--
+Bug #72434: ZipArchive class Use After Free Vulnerability in PHP's GC algorithm and unserialize
+--SKIPIF--
+<?php
+if(!class_exists('zip')) die('ZipArchive');
+?>
+--FILE--
+<?php
+// The following array will be serialized and this representation will be freed later on.
+$free_me = array(new StdClass());
+// Create our payload and unserialize it.
+$serialized_payload = 'a:3:{i:1;N;i:2;O:10:"ZipArchive":1:{s:8:"filename";'.serialize($free_me).'}i:1;R:4;}';
+$unserialized_payload = unserialize($serialized_payload);
+gc_collect_cycles();
+// The reference counter for $free_me is at -1 for PHP 7 right now.
+// Increment the reference counter by 1 -> rc is 0
+$a = $unserialized_payload[1];
+// Increment the reference counter by 1 again -> rc is 1
+$b = $a;
+// Trigger free of $free_me (referenced by $m[1]).
+unset($b);
+$fill_freed_space_1 = "filler_zval_1";
+$fill_freed_space_2 = "filler_zval_2";
+$fill_freed_space_3 = "filler_zval_3";
+$fill_freed_space_4 = "filler_zval_4";
+debug_zval_dump($unserialized_payload[1]);
+?>
+--EXPECTF--
+array(1) refcount(1){
+ [0]=>
+ object(stdClass)#%d (0) refcount(3){
+ }
+}
diff --git a/ext/zip/php_zip.c b/ext/zip/php_zip.c
index 99c293c..57d060f 100644
--- a/ext/zip/php_zip.c
+++ b/ext/zip/php_zip.c
@@ -1015,6 +1015,14 @@ static int php_zip_has_property(zval *object, zval *member, int type, const zend
}
/* }}} */
+static HashTable *php_zip_get_gc(zval *object, zval ***gc_data, int *gc_data_count TSRMLS_DC) /* {{{ */
+{
+ *gc_data = NULL;
+ *gc_data_count = 0;
+ return zend_std_get_properties(object TSRMLS_CC);
+}
+/* }}} */
+
static HashTable *php_zip_get_properties(zval *object TSRMLS_DC)/* {{{ */
{
ze_zip_object *obj;
@@ -2777,6 +2785,7 @@ static PHP_MINIT_FUNCTION(zip)
zip_object_handlers.clone_obj = NULL;
zip_object_handlers.get_property_ptr_ptr = php_zip_get_property_ptr_ptr;
+ zip_object_handlers.get_gc = php_zip_get_gc;
zip_object_handlers.get_properties = php_zip_get_properties;
zip_object_handlers.read_property = php_zip_read_property;
zip_object_handlers.has_property = php_zip_has_property;
@pierrejoye

This comment has been minimized.

Copy link

commented Jun 21, 2016

Thanks!
Do we have a note in the internal upgrade guide about that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.