fix SplObjectStorage unserialization (CVE-2010-2225)

4 files changed, 86 insertions, 11 deletions

diff --git a/NEWS b/NEWS
index e792a55d61..9ce2c8f038 100644
--- a/NEWS
+++ b/NEWS
@@ -1,6 +1,8 @@
 PHP                                                                        NEWS
 |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
 ?? Jun 2010, PHP 5.2.14
+- Fixed SplObjectStorage unserialization problems (CVE-2010-2225). (Stas)
+
 - Fixed bug #52163 (SplFileObject::fgetss() fails due to parameter that can't
   be set). (Felipe)
 - Fixed bug #52162 (custom request header variables with numbers are removed). 
diff --git a/ext/spl/SplObjectStorage_unserialize_bad.phpt b/ext/spl/SplObjectStorage_unserialize_bad.phpt
new file mode 100644
index 0000000000..1e5f19f001
--- /dev/null
+++ b/ext/spl/SplObjectStorage_unserialize_bad.phpt
@@ -0,0 +1,24 @@
+--TEST--
+SPL: Test that serialized blob contains unique elements (CVE-2010-2225)
+--FILE--
+<?php
+
+$badblobs = array(
+'x:i:2;i:0;;i:0;;m:a:0:{}',
+'x:i:2;O:8:"stdClass":0:{};R:1;;m:a:0:{}',
+'x:i:3;O:8:"stdClass":0:{};r:1;;r:1;;m:a:0:{}',
+);
+foreach($badblobs as $blob) {
+try {
+  $so = new SplObjectStorage();
+  $so->unserialize($blob);
+  var_dump($so);
+} catch(UnexpectedValueException $e) {
+	echo $e->getMessage()."\n";
+}
+}
+--EXPECTF--
+Error at offset 6 of 24 bytes
+Error at offset 26 of 39 bytes
+object(SplObjectStorage)#2 (0) {
+}
diff --git a/ext/spl/SplObjectStorage_unserialize_nested.phpt b/ext/spl/SplObjectStorage_unserialize_nested.phpt
new file mode 100644
index 0000000000..b72f374712
--- /dev/null
+++ b/ext/spl/SplObjectStorage_unserialize_nested.phpt
@@ -0,0 +1,33 @@
+--TEST--
+SPL: Test unserializing tested & linked storage
+--FILE--
+<?php
+$o = new StdClass();
+$a = new StdClass();
+
+$o->a = $a;
+
+$so = new SplObjectStorage();
+
+$so->attach($o);
+$so->attach($a);
+
+$s = serialize($so);
+echo $s."\n";
+
+$so1 = unserialize($s);
+var_dump($so1);
+foreach($so1 as $obj) {
+  var_dump($obj);
+}
+--EXPECTF--
+C:16:"SplObjectStorage":66:{x:i:2;O:8:"stdClass":1:{s:1:"a";O:8:"stdClass":0:{}};r:2;;m:a:0:{}}
+object(SplObjectStorage)#4 (0) {
+}
+object(stdClass)#5 (1) {
+  ["a"]=>
+  object(stdClass)#6 (0) {
+  }
+}
+object(stdClass)#6 (0) {
+}
diff --git a/ext/spl/spl_observer.c b/ext/spl/spl_observer.c
index e3c497a1df..132351afbf 100755
--- a/ext/spl/spl_observer.c
+++ b/ext/spl/spl_observer.c
@@ -182,6 +182,21 @@ SPL_METHOD(SplObjectStorage, detach)
 	intern->index = 0;
 } /* }}} */
 
+int spl_object_storage_contains(spl_SplObjectStorage *intern, zval *obj TSRMLS_DC) /* {{{ */
+{
+#if HAVE_PACKED_OBJECT_VALUE
+	return zend_hash_exists(&intern->storage, (char*)&Z_OBJVAL_P(obj), sizeof(zend_object_value));
+#else
+	{
+		zend_object_value zvalue;
+		memset(&zvalue, 0, sizeof(zend_object_value));
+		zvalue.handle = Z_OBJ_HANDLE_P(obj);
+		zvalue.handlers = Z_OBJ_HT_P(obj);
+		return zend_hash_exists(&intern->storage, (char*)&zvalue, sizeof(zend_object_value));
+	}
+#endif
+} /* }}} */
+
 /* {{{ proto bool SplObjectStorage::contains($obj)
  Determine whethe an object is contained in the storage */
 SPL_METHOD(SplObjectStorage, contains)
@@ -193,17 +208,7 @@ SPL_METHOD(SplObjectStorage, contains)
 		return;
 	}
 
-#if HAVE_PACKED_OBJECT_VALUE
-	RETURN_BOOL(zend_hash_exists(&intern->storage, (char*)&Z_OBJVAL_P(obj), sizeof(zend_object_value)));
-#else
-	{
-		zend_object_value zvalue;
-		memset(&zvalue, 0, sizeof(zend_object_value));
-		zvalue.handle = Z_OBJ_HANDLE_P(obj);
-		zvalue.handlers = Z_OBJ_HT_P(obj);
-		RETURN_BOOL(zend_hash_exists(&intern->storage, (char*)&zvalue, sizeof(zend_object_value)));
-	}
-#endif
+	RETURN_BOOL(spl_object_storage_contains(intern, obj TSRMLS_CC));
 } /* }}} */
 
 /* {{{ proto int SplObjectStorage::count()
@@ -362,11 +367,22 @@ SPL_METHOD(SplObjectStorage, unserialize)
 			goto outexcept;
 		}
 		++p;
+		if(*p != 'O' && *p != 'C' && *p != 'r') {
+			goto outexcept;
+		}
 		ALLOC_INIT_ZVAL(pentry);
 		if (!php_var_unserialize(&pentry, &p, s + buf_len, &var_hash TSRMLS_CC)) {
 			zval_ptr_dtor(&pentry);
 			goto outexcept;
 		}
+		if(Z_TYPE_P(pentry) != IS_OBJECT) {
+			zval_ptr_dtor(&pentry);
+			goto outexcept;
+		}
+		if(spl_object_storage_contains(intern, pentry TSRMLS_CC)) {
+			zval_ptr_dtor(&pentry);
+			continue;
+		}
 		spl_object_storage_attach(intern, pentry TSRMLS_CC);
 		zval_ptr_dtor(&pentry);
 	}