Skip to content

Commit b7fa677

Browse files
smalyshevsmalyshev
committed
Fix bug #70068 (Dangling pointer in the unserialization of ArrayObject items)
1 parent e488690 commit b7fa677

File tree

2 files changed

+56
-43
lines changed

2 files changed

+56
-43
lines changed

‎ext/spl/spl_array.c

+47-43
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,7 @@ static zval **spl_array_get_dimension_ptr_ptr(int check_inherited, zval *object,
309309
if (!offset) {
310310
return &EG(uninitialized_zval_ptr);
311311
}
312-
312+
313313
if ((type == BP_VAR_W || type == BP_VAR_RW) && (ht->nApplyCount > 0)) {
314314
zend_error(E_WARNING, "Modification of ArrayObject during sorting is prohibited");
315315
return &EG(error_zval_ptr);;
@@ -341,8 +341,8 @@ static zval **spl_array_get_dimension_ptr_ptr(int check_inherited, zval *object,
341341
case IS_RESOURCE:
342342
zend_error(E_STRICT, "Resource ID#%ld used as offset, casting to integer (%ld)", Z_LVAL_P(offset), Z_LVAL_P(offset));
343343
case IS_DOUBLE:
344-
case IS_BOOL:
345-
case IS_LONG:
344+
case IS_BOOL:
345+
case IS_LONG:
346346
if (offset->type == IS_DOUBLE) {
347347
index = (long)Z_DVAL_P(offset);
348348
} else {
@@ -386,7 +386,7 @@ static zval *spl_array_read_dimension_ex(int check_inherited, zval *object, zval
386386
} else {
387387
SEPARATE_ARG_IF_REF(offset);
388388
}
389-
zend_call_method_with_1_params(&object, Z_OBJCE_P(object), &intern->fptr_offset_get, "offsetGet", &rv, offset);
389+
zend_call_method_with_1_params(&object, Z_OBJCE_P(object), &intern->fptr_offset_get, "offsetGet", &rv, offset);
390390
zval_ptr_dtor(&offset);
391391
if (rv) {
392392
zval_ptr_dtor(&intern->retval);
@@ -444,7 +444,7 @@ static void spl_array_write_dimension_ex(int check_inherited, zval *object, zval
444444
zval_ptr_dtor(&offset);
445445
return;
446446
}
447-
447+
448448
if (!offset) {
449449
ht = spl_array_get_hash_table(intern, 0 TSRMLS_CC);
450450
if (ht->nApplyCount > 0) {
@@ -467,8 +467,8 @@ static void spl_array_write_dimension_ex(int check_inherited, zval *object, zval
467467
return;
468468
case IS_DOUBLE:
469469
case IS_RESOURCE:
470-
case IS_BOOL:
471-
case IS_LONG:
470+
case IS_BOOL:
471+
case IS_LONG:
472472
ht = spl_array_get_hash_table(intern, 0 TSRMLS_CC);
473473
if (ht->nApplyCount > 0) {
474474
zend_error(E_WARNING, "Modification of ArrayObject during sorting is prohibited");
@@ -556,13 +556,13 @@ static void spl_array_unset_dimension_ex(int check_inherited, zval *object, zval
556556
obj->std.properties_table[property_info->offset] = NULL;
557557
}
558558
}
559-
}
559+
}
560560
}
561561
break;
562562
case IS_DOUBLE:
563563
case IS_RESOURCE:
564-
case IS_BOOL:
565-
case IS_LONG:
564+
case IS_BOOL:
565+
case IS_LONG:
566566
if (offset->type == IS_DOUBLE) {
567567
index = (long)Z_DVAL_P(offset);
568568
} else {
@@ -608,7 +608,7 @@ static int spl_array_has_dimension_ex(int check_inherited, zval *object, zval *o
608608
}
609609
return 0;
610610
}
611-
611+
612612
switch(Z_TYPE_P(offset)) {
613613
case IS_STRING:
614614
{
@@ -627,9 +627,9 @@ static int spl_array_has_dimension_ex(int check_inherited, zval *object, zval *o
627627
return 0;
628628
case IS_DOUBLE:
629629
case IS_RESOURCE:
630-
case IS_BOOL:
630+
case IS_BOOL:
631631
case IS_LONG:
632-
{
632+
{
633633
HashTable *ht = spl_array_get_hash_table(intern, 0 TSRMLS_CC);
634634
if (offset->type == IS_DOUBLE) {
635635
index = (long)Z_DVAL_P(offset);
@@ -727,7 +727,7 @@ void spl_array_iterator_append(zval *object, zval *append_value TSRMLS_DC) /* {{
727727
php_error_docref(NULL TSRMLS_CC, E_NOTICE, "Array was modified outside object and is no longer an array");
728728
return;
729729
}
730-
730+
731731
if (Z_TYPE_P(intern->array) == IS_OBJECT) {
732732
php_error_docref(NULL TSRMLS_CC, E_RECOVERABLE_ERROR, "Cannot append properties to objects, use %s::offsetSet() instead", Z_OBJCE_P(object)->name);
733733
return;
@@ -771,7 +771,7 @@ SPL_METHOD(Array, getArrayCopy)
771771
{
772772
zval *object = getThis(), *tmp;
773773
spl_array_object *intern = (spl_array_object*)zend_object_store_get_object(object TSRMLS_CC);
774-
774+
775775
array_init(return_value);
776776
zend_hash_copy(HASH_OF(return_value), spl_array_get_hash_table(intern, 0 TSRMLS_CC), (copy_ctor_func_t) zval_add_ref, &tmp, sizeof(zval*));
777777
} /* }}} */
@@ -990,7 +990,7 @@ static void spl_array_it_dtor(zend_object_iterator *iter TSRMLS_DC) /* {{{ */
990990
efree(iterator);
991991
}
992992
/* }}} */
993-
993+
994994
static int spl_array_it_valid(zend_object_iterator *iter TSRMLS_DC) /* {{{ */
995995
{
996996
spl_array_it *iterator = (spl_array_it *)iter;
@@ -1037,7 +1037,7 @@ static int spl_array_it_get_current_key(zend_object_iterator *iter, char **str_k
10371037
if (spl_array_object_verify_pos_ex(object, aht, "ArrayIterator::current(): " TSRMLS_CC) == FAILURE) {
10381038
return HASH_KEY_NON_EXISTANT;
10391039
}
1040-
1040+
10411041
return zend_hash_get_current_key_ex(aht, str_key, str_key_len, int_key, 1, &object->pos);
10421042
}
10431043
}
@@ -1057,7 +1057,7 @@ static void spl_array_it_move_forward(zend_object_iterator *iter TSRMLS_DC) /* {
10571057
php_error_docref(NULL TSRMLS_CC, E_NOTICE, "ArrayIterator::current(): Array was modified outside object and is no longer an array");
10581058
return;
10591059
}
1060-
1060+
10611061
if ((object->ar_flags & SPL_ARRAY_IS_REF) && spl_hash_verify_pos_ex(object, aht TSRMLS_CC) == FAILURE) {
10621062
php_error_docref(NULL TSRMLS_CC, E_NOTICE, "ArrayIterator::next(): Array was modified outside object and internal position is no longer valid");
10631063
} else {
@@ -1115,7 +1115,7 @@ static void spl_array_set_array(zval *object, spl_array_object *intern, zval **a
11151115
if (just_array) {
11161116
spl_array_object *other = (spl_array_object*)zend_object_store_get_object(*array TSRMLS_CC);
11171117
ar_flags = other->ar_flags & ~SPL_ARRAY_INT_MASK;
1118-
}
1118+
}
11191119
ar_flags |= SPL_ARRAY_USE_OTHER;
11201120
intern->array = *array;
11211121
} else {
@@ -1173,7 +1173,7 @@ zend_object_iterator *spl_array_get_iterator(zend_class_entry *ce, zval *object,
11731173
iterator->intern.ce = ce;
11741174
iterator->intern.value = NULL;
11751175
iterator->object = array_object;
1176-
1176+
11771177
return (zend_object_iterator*)iterator;
11781178
}
11791179
/* }}} */
@@ -1238,7 +1238,7 @@ SPL_METHOD(Array, getIteratorClass)
12381238
{
12391239
zval *object = getThis();
12401240
spl_array_object *intern = (spl_array_object*)zend_object_store_get_object(object TSRMLS_CC);
1241-
1241+
12421242
if (zend_parse_parameters_none() == FAILURE) {
12431243
return;
12441244
}
@@ -1253,11 +1253,11 @@ SPL_METHOD(Array, getFlags)
12531253
{
12541254
zval *object = getThis();
12551255
spl_array_object *intern = (spl_array_object*)zend_object_store_get_object(object TSRMLS_CC);
1256-
1256+
12571257
if (zend_parse_parameters_none() == FAILURE) {
12581258
return;
12591259
}
1260-
1260+
12611261
RETURN_LONG(intern->ar_flags & ~SPL_ARRAY_INT_MASK);
12621262
}
12631263
/* }}} */
@@ -1273,7 +1273,7 @@ SPL_METHOD(Array, setFlags)
12731273
if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "l", &ar_flags) == FAILURE) {
12741274
return;
12751275
}
1276-
1276+
12771277
intern->ar_flags = (intern->ar_flags & SPL_ARRAY_INT_MASK) | (ar_flags & ~SPL_ARRAY_INT_MASK);
12781278
}
12791279
/* }}} */
@@ -1287,7 +1287,7 @@ SPL_METHOD(Array, exchangeArray)
12871287

12881288
array_init(return_value);
12891289
zend_hash_copy(HASH_OF(return_value), spl_array_get_hash_table(intern, 0 TSRMLS_CC), (copy_ctor_func_t) zval_add_ref, &tmp, sizeof(zval*));
1290-
1290+
12911291
if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "Z", &array) == FAILURE) {
12921292
return;
12931293
}
@@ -1305,7 +1305,7 @@ SPL_METHOD(Array, getIterator)
13051305
spl_array_object *intern = (spl_array_object*)zend_object_store_get_object(object TSRMLS_CC);
13061306
spl_array_object *iterator;
13071307
HashTable *aht = spl_array_get_hash_table(intern, 0 TSRMLS_CC);
1308-
1308+
13091309
if (zend_parse_parameters_none() == FAILURE) {
13101310
return;
13111311
}
@@ -1328,7 +1328,7 @@ SPL_METHOD(Array, rewind)
13281328
{
13291329
zval *object = getThis();
13301330
spl_array_object *intern = (spl_array_object*)zend_object_store_get_object(object TSRMLS_CC);
1331-
1331+
13321332
if (zend_parse_parameters_none() == FAILURE) {
13331333
return;
13341334
}
@@ -1361,9 +1361,9 @@ SPL_METHOD(Array, seek)
13611361
if (position >= 0) { /* negative values are not supported */
13621362
spl_array_rewind(intern TSRMLS_CC);
13631363
result = SUCCESS;
1364-
1364+
13651365
while (position-- > 0 && (result = spl_array_next(intern TSRMLS_CC)) == SUCCESS);
1366-
1366+
13671367
if (result == SUCCESS && zend_hash_has_more_elements_ex(aht, &intern->pos) == SUCCESS) {
13681368
return; /* ok */
13691369
}
@@ -1383,7 +1383,7 @@ int static spl_array_object_count_elements_helper(spl_array_object *intern, long
13831383
}
13841384

13851385
if (Z_TYPE_P(intern->array) == IS_OBJECT) {
1386-
/* We need to store the 'pos' since we'll modify it in the functions
1386+
/* We need to store the 'pos' since we'll modify it in the functions
13871387
* we're going to call and which do not support 'pos' as parameter. */
13881388
pos = intern->pos;
13891389
*count = 0;
@@ -1427,7 +1427,7 @@ SPL_METHOD(Array, count)
14271427
{
14281428
long count;
14291429
spl_array_object *intern = (spl_array_object*)zend_object_store_get_object(getThis() TSRMLS_CC);
1430-
1430+
14311431
if (zend_parse_parameters_none() == FAILURE) {
14321432
return;
14331433
}
@@ -1443,11 +1443,11 @@ static void spl_array_method(INTERNAL_FUNCTION_PARAMETERS, char *fname, int fnam
14431443
HashTable *aht = spl_array_get_hash_table(intern, 0 TSRMLS_CC);
14441444
zval *tmp, *arg = NULL;
14451445
zval *retval_ptr = NULL;
1446-
1446+
14471447
MAKE_STD_ZVAL(tmp);
14481448
Z_TYPE_P(tmp) = IS_ARRAY;
14491449
Z_ARRVAL_P(tmp) = aht;
1450-
1450+
14511451
if (!use_arg) {
14521452
aht->nApplyCount++;
14531453
zend_call_method(NULL, NULL, NULL, fname, fname_len, &retval_ptr, 1, tmp, NULL TSRMLS_CC);
@@ -1524,7 +1524,7 @@ SPL_METHOD(Array, current)
15241524
spl_array_object *intern = (spl_array_object*)zend_object_store_get_object(object TSRMLS_CC);
15251525
zval **entry;
15261526
HashTable *aht = spl_array_get_hash_table(intern, 0 TSRMLS_CC);
1527-
1527+
15281528
if (zend_parse_parameters_none() == FAILURE) {
15291529
return;
15301530
}
@@ -1547,7 +1547,7 @@ SPL_METHOD(Array, key)
15471547
if (zend_parse_parameters_none() == FAILURE) {
15481548
return;
15491549
}
1550-
1550+
15511551
spl_array_iterator_key(getThis(), return_value TSRMLS_CC);
15521552
} /* }}} */
15531553

@@ -1594,7 +1594,7 @@ SPL_METHOD(Array, next)
15941594

15951595
spl_array_next_no_verify(intern, aht TSRMLS_CC);
15961596
}
1597-
/* }}} */
1597+
/* }}} */
15981598

15991599
/* {{{ proto bool ArrayIterator::valid()
16001600
Check whether array contains more entries */
@@ -1623,7 +1623,7 @@ SPL_METHOD(Array, hasChildren)
16231623
zval *object = getThis(), **entry;
16241624
spl_array_object *intern = (spl_array_object*)zend_object_store_get_object(object TSRMLS_CC);
16251625
HashTable *aht = spl_array_get_hash_table(intern, 0 TSRMLS_CC);
1626-
1626+
16271627
if (zend_parse_parameters_none() == FAILURE) {
16281628
return;
16291629
}
@@ -1647,7 +1647,7 @@ SPL_METHOD(Array, getChildren)
16471647
zval *object = getThis(), **entry, *flags;
16481648
spl_array_object *intern = (spl_array_object*)zend_object_store_get_object(object TSRMLS_CC);
16491649
HashTable *aht = spl_array_get_hash_table(intern, 0 TSRMLS_CC);
1650-
1650+
16511651
if (zend_parse_parameters_none() == FAILURE) {
16521652
return;
16531653
}
@@ -1687,7 +1687,7 @@ SPL_METHOD(Array, serialize)
16871687
php_serialize_data_t var_hash;
16881688
smart_str buf = {0};
16891689
zval *flags;
1690-
1690+
16911691
if (zend_parse_parameters_none() == FAILURE) {
16921692
return;
16931693
}
@@ -1747,7 +1747,7 @@ SPL_METHOD(Array, unserialize)
17471747
zval *pmembers, *pflags = NULL;
17481748
HashTable *aht;
17491749
long flags;
1750-
1750+
17511751
if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s", &buf, &buf_len) == FAILURE) {
17521752
return;
17531753
}
@@ -1774,13 +1774,11 @@ SPL_METHOD(Array, unserialize)
17741774

17751775
ALLOC_INIT_ZVAL(pflags);
17761776
if (!php_var_unserialize(&pflags, &p, s + buf_len, &var_hash TSRMLS_CC) || Z_TYPE_P(pflags) != IS_LONG) {
1777-
zval_ptr_dtor(&pflags);
17781777
goto outexcept;
17791778
}
17801779

17811780
--p; /* for ';' */
17821781
flags = Z_LVAL_P(pflags);
1783-
zval_ptr_dtor(&pflags);
17841782
/* flags needs to be verified and we also need to verify whether the next
17851783
* thing we get is ';'. After that we require an 'm' or somethign else
17861784
* where 'm' stands for members and anything else should be an array. If
@@ -1830,10 +1828,16 @@ SPL_METHOD(Array, unserialize)
18301828
/* done reading $serialized */
18311829

18321830
PHP_VAR_UNSERIALIZE_DESTROY(var_hash);
1831+
if (pflags) {
1832+
zval_ptr_dtor(&pflags);
1833+
}
18331834
return;
18341835

18351836
outexcept:
18361837
PHP_VAR_UNSERIALIZE_DESTROY(var_hash);
1838+
if (pflags) {
1839+
zval_ptr_dtor(&pflags);
1840+
}
18371841
zend_throw_exception_ex(spl_ce_UnexpectedValueException, 0 TSRMLS_CC, "Error at offset %ld of %d bytes", (long)((char*)p - buf), buf_len);
18381842
return;
18391843

@@ -1982,7 +1986,7 @@ PHP_MINIT_FUNCTION(spl_array)
19821986
REGISTER_SPL_IMPLEMENTS(ArrayIterator, Countable);
19831987
memcpy(&spl_handler_ArrayIterator, &spl_handler_ArrayObject, sizeof(zend_object_handlers));
19841988
spl_ce_ArrayIterator->get_iterator = spl_array_get_iterator;
1985-
1989+
19861990
REGISTER_SPL_SUB_CLASS_EX(RecursiveArrayIterator, ArrayIterator, spl_array_object_new, spl_funcs_RecursiveArrayIterator);
19871991
REGISTER_SPL_IMPLEMENTS(RecursiveArrayIterator, RecursiveIterator);
19881992
spl_ce_RecursiveArrayIterator->get_iterator = spl_array_get_iterator;

‎ext/spl/tests/bug70068.phpt

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
--TEST--
2+
Bug #70068 (Dangling pointer in the unserialization of ArrayObject items)
3+
--FILE--
4+
<?php
5+
$a = unserialize('a:3:{i:0;C:11:"ArrayObject":20:{x:i:0;r:3;;m:a:0:{};}i:1;d:11;i:2;S:31:"AAAAAAAABBBBCCCC\01\00\00\00\04\00\00\00\00\00\00\00\00\00\00";}');
6+
?>
7+
OK
8+
--EXPECT--
9+
OK

0 commit comments

Comments
 (0)