Fix proptable canonicalization bypass in ArrayObject

When an ArrayObject wraps an object, we should be using the
proptable canonicalilzation rules, which require all keys to be
strings.

2 files changed, 179 insertions, 174 deletions

diff --git a/ext/spl/spl_array.c b/ext/spl/spl_array.c
index dc66241e44..880c3c67db 100644
--- a/ext/spl/spl_array.c
+++ b/ext/spl/spl_array.c
@@ -275,11 +275,74 @@ static zend_object *spl_array_object_clone(zend_object *old_object)
 }
 /* }}} */
 
+typedef struct {
+	zend_string *key;
+	zend_ulong h;
+	bool release_key;
+} spl_hash_key;
+
+static void spl_hash_key_release(spl_hash_key *key) {
+	if (key->release_key) {
+		zend_string_release_ex(key->key, 0);
+	}
+}
+
+static zend_result get_hash_key(spl_hash_key *key, spl_array_object *intern, zval *offset)
+{
+	key->release_key = false;
+try_again:
+	switch (Z_TYPE_P(offset)) {
+	case IS_NULL:
+		key->key = ZSTR_EMPTY_ALLOC();
+		return SUCCESS;
+	case IS_STRING:
+		key->key = Z_STR_P(offset);
+		if (ZEND_HANDLE_NUMERIC(key->key, key->h)) {
+			key->key = NULL;
+			break;
+		}
+		return SUCCESS;
+	case IS_RESOURCE:
+		zend_error(E_WARNING, "Resource ID#%d used as offset, casting to integer (%d)",
+			Z_RES_P(offset)->handle, Z_RES_P(offset)->handle);
+		key->key = NULL;
+		key->h = Z_RES_P(offset)->handle;
+		break;
+	case IS_DOUBLE:
+		key->key = NULL;
+		key->h = (zend_long) Z_DVAL_P(offset);
+		break;
+	case IS_FALSE:
+		key->key = NULL;
+		key->h = 0;
+		break;
+	case IS_TRUE:
+		key->key = NULL;
+		key->h = 1;
+		break;
+	case IS_LONG:
+		key->key = NULL;
+		key->h = Z_LVAL_P(offset);
+		break;
+	case IS_REFERENCE:
+		ZVAL_DEREF(offset);
+		goto try_again;
+	default:
+		zend_type_error("Illegal offset type");
+		return FAILURE;
+	}
+
+	if (spl_array_is_object(intern)) {
+		key->key = zend_long_to_str(key->h);
+		key->release_key = true;
+	}
+	return SUCCESS;
+}
+
 static zval *spl_array_get_dimension_ptr(int check_inherited, spl_array_object *intern, zval *offset, int type) /* {{{ */
 {
 	zval *retval;
-	zend_long index;
-	zend_string *offset_key;
+	spl_hash_key key;
 	HashTable *ht = spl_array_get_hash_table(intern);
 
 	if (!offset || Z_ISUNDEF_P(offset) || !ht) {
@@ -291,28 +354,27 @@ static zval *spl_array_get_dimension_ptr(int check_inherited, spl_array_object *
 		return &EG(error_zval);
 	}
 
-try_again:
-	switch (Z_TYPE_P(offset)) {
-	case IS_NULL:
-	   offset_key = ZSTR_EMPTY_ALLOC();
-	   goto fetch_dim_string;
-	case IS_STRING:
-	   offset_key = Z_STR_P(offset);
-fetch_dim_string:
-		retval = zend_symtable_find(ht, offset_key);
+	if (get_hash_key(&key, intern, offset) == FAILURE) {
+		zend_type_error("Illegal offset type");
+		return (type == BP_VAR_W || type == BP_VAR_RW) ?
+			&EG(error_zval) : &EG(uninitialized_zval);
+	}
+
+	if (key.key) {
+		retval = zend_hash_find(ht, key.key);
 		if (retval) {
 			if (Z_TYPE_P(retval) == IS_INDIRECT) {
 				retval = Z_INDIRECT_P(retval);
 				if (Z_TYPE_P(retval) == IS_UNDEF) {
 					switch (type) {
 						case BP_VAR_R:
-							zend_error(E_WARNING, "Undefined array key \"%s\"", ZSTR_VAL(offset_key));
+							zend_error(E_WARNING, "Undefined array key \"%s\"", ZSTR_VAL(key.key));
 						case BP_VAR_UNSET:
 						case BP_VAR_IS:
 							retval = &EG(uninitialized_zval);
 							break;
 						case BP_VAR_RW:
-							zend_error(E_WARNING,"Undefined array key \"%s\"", ZSTR_VAL(offset_key));
+							zend_error(E_WARNING,"Undefined array key \"%s\"", ZSTR_VAL(key.key));
 						case BP_VAR_W: {
 							ZVAL_NULL(retval);
 						}
@@ -322,63 +384,41 @@ fetch_dim_string:
 		} else {
 			switch (type) {
 				case BP_VAR_R:
-					zend_error(E_WARNING, "Undefined array key \"%s\"", ZSTR_VAL(offset_key));
+					zend_error(E_WARNING, "Undefined array key \"%s\"", ZSTR_VAL(key.key));
 				case BP_VAR_UNSET:
 				case BP_VAR_IS:
 					retval = &EG(uninitialized_zval);
 					break;
 				case BP_VAR_RW:
-					zend_error(E_WARNING,"Undefined array key \"%s\"", ZSTR_VAL(offset_key));
+					zend_error(E_WARNING,"Undefined array key \"%s\"", ZSTR_VAL(key.key));
 				case BP_VAR_W: {
 				    zval value;
 					ZVAL_NULL(&value);
-				    retval = zend_symtable_update(ht, offset_key, &value);
+				    retval = zend_hash_update(ht, key.key, &value);
 				}
 			}
 		}
-		return retval;
-	case IS_RESOURCE:
-		zend_error(E_WARNING, "Resource ID#%d used as offset, casting to integer (%d)", Z_RES_P(offset)->handle, Z_RES_P(offset)->handle);
-		index = Z_RES_P(offset)->handle;
-		goto num_index;
-	case IS_DOUBLE:
-		index = (zend_long)Z_DVAL_P(offset);
-		goto num_index;
-	case IS_FALSE:
-		index = 0;
-		goto num_index;
-	case IS_TRUE:
-		index = 1;
-		goto num_index;
-	case IS_LONG:
-		index = Z_LVAL_P(offset);
-num_index:
-		if ((retval = zend_hash_index_find(ht, index)) == NULL) {
+		spl_hash_key_release(&key);
+	} else {
+		if ((retval = zend_hash_index_find(ht, key.h)) == NULL) {
 			switch (type) {
 				case BP_VAR_R:
-					zend_error(E_WARNING, "Undefined array key " ZEND_LONG_FMT, index);
+					zend_error(E_WARNING, "Undefined array key " ZEND_LONG_FMT, key.h);
 				case BP_VAR_UNSET:
 				case BP_VAR_IS:
 					retval = &EG(uninitialized_zval);
 					break;
 				case BP_VAR_RW:
-					zend_error(E_WARNING, "Undefined array key " ZEND_LONG_FMT, index);
+					zend_error(E_WARNING, "Undefined array key " ZEND_LONG_FMT, key.h);
 				case BP_VAR_W: {
 				    zval value;
 					ZVAL_UNDEF(&value);
-					retval = zend_hash_index_update(ht, index, &value);
+					retval = zend_hash_index_update(ht, key.h, &value);
 			   }
 			}
 		}
-		return retval;
-	case IS_REFERENCE:
-		ZVAL_DEREF(offset);
-		goto try_again;
-	default:
-		zend_type_error("Illegal offset type");
-		return (type == BP_VAR_W || type == BP_VAR_RW) ?
-			&EG(error_zval) : &EG(uninitialized_zval);
 	}
+	return retval;
 } /* }}} */
 
 static int spl_array_has_dimension(zend_object *object, zval *offset, int check_empty);
@@ -435,8 +475,8 @@ static zval *spl_array_read_dimension(zend_object *object, zval *offset, int typ
 static void spl_array_write_dimension_ex(int check_inherited, zend_object *object, zval *offset, zval *value) /* {{{ */
 {
 	spl_array_object *intern = spl_array_from_obj(object);
-	zend_long index;
 	HashTable *ht;
+	spl_hash_key key;
 
 	if (check_inherited && intern->fptr_offset_set) {
 		zval tmp;
@@ -455,47 +495,24 @@ static void spl_array_write_dimension_ex(int check_inherited, zend_object *objec
 	}
 
 	Z_TRY_ADDREF_P(value);
-	if (!offset) {
+	if (!offset || Z_TYPE_P(offset) == IS_NULL) {
 		ht = spl_array_get_hash_table(intern);
 		zend_hash_next_index_insert(ht, value);
 		return;
 	}
 
-try_again:
-	switch (Z_TYPE_P(offset)) {
-		case IS_STRING:
-			ht = spl_array_get_hash_table(intern);
-			zend_symtable_update_ind(ht, Z_STR_P(offset), value);
-			return;
-		case IS_DOUBLE:
-			index = (zend_long)Z_DVAL_P(offset);
-			goto num_index;
-		case IS_RESOURCE:
-			index = Z_RES_HANDLE_P(offset);
-			goto num_index;
-		case IS_FALSE:
-			index = 0;
-			goto num_index;
-		case IS_TRUE:
-			index = 1;
-			goto num_index;
-		case IS_LONG:
-			index = Z_LVAL_P(offset);
-num_index:
-			ht = spl_array_get_hash_table(intern);
-			zend_hash_index_update(ht, index, value);
-			return;
-		case IS_NULL:
-			ht = spl_array_get_hash_table(intern);
-			zend_hash_next_index_insert(ht, value);
-			return;
-		case IS_REFERENCE:
-			ZVAL_DEREF(offset);
-			goto try_again;
-		default:
-			zend_type_error("Illegal offset type");
-			zval_ptr_dtor(value);
-			return;
+	if (get_hash_key(&key, intern, offset) == FAILURE) {
+		zend_type_error("Illegal offset type");
+		zval_ptr_dtor(value);
+		return;
+	}
+
+	ht = spl_array_get_hash_table(intern);
+	if (key.key) {
+		zend_hash_update_ind(ht, key.key, value);
+		spl_hash_key_release(&key);
+	} else {
+		zend_hash_index_update(ht, key.h, value);
 	}
 } /* }}} */
 
@@ -506,9 +523,9 @@ static void spl_array_write_dimension(zend_object *object, zval *offset, zval *v
 
 static void spl_array_unset_dimension_ex(int check_inherited, zend_object *object, zval *offset) /* {{{ */
 {
-	zend_long index;
 	HashTable *ht;
 	spl_array_object *intern = spl_array_from_obj(object);
+	spl_hash_key key;
 
 	if (check_inherited && intern->fptr_offset_del) {
 		zend_call_method_with_1_params(object, object->ce, &intern->fptr_offset_del, "offsetUnset", NULL, offset);
@@ -520,61 +537,39 @@ static void spl_array_unset_dimension_ex(int check_inherited, zend_object *objec
 		return;
 	}
 
-try_again:
-	switch (Z_TYPE_P(offset)) {
-	case IS_STRING:
-		ht = spl_array_get_hash_table(intern);
+	if (get_hash_key(&key, intern, offset) == FAILURE) {
+		zend_type_error("Illegal offset type in unset");
+		return;
+	}
 
-		{
-			zval *data = zend_symtable_find(ht, Z_STR_P(offset));
-			if (data) {
-				if (Z_TYPE_P(data) == IS_INDIRECT) {
-					data = Z_INDIRECT_P(data);
-					if (Z_TYPE_P(data) == IS_UNDEF) {
-						zend_error(E_WARNING,"Undefined array key \"%s\"", Z_STRVAL_P(offset));
-					} else {
-						zval_ptr_dtor(data);
-						ZVAL_UNDEF(data);
-						HT_FLAGS(ht) |= HASH_FLAG_HAS_EMPTY_IND;
-						zend_hash_move_forward_ex(ht, spl_array_get_pos_ptr(ht, intern));
-						if (spl_array_is_object(intern)) {
-							spl_array_skip_protected(intern, ht);
-						}
+	ht = spl_array_get_hash_table(intern);
+	if (key.key) {
+		zval *data = zend_hash_find(ht, key.key);
+		if (data) {
+			if (Z_TYPE_P(data) == IS_INDIRECT) {
+				data = Z_INDIRECT_P(data);
+				if (Z_TYPE_P(data) == IS_UNDEF) {
+					zend_error(E_WARNING,"Undefined array key \"%s\"", ZSTR_VAL(key.key));
+				} else {
+					zval_ptr_dtor(data);
+					ZVAL_UNDEF(data);
+					HT_FLAGS(ht) |= HASH_FLAG_HAS_EMPTY_IND;
+					zend_hash_move_forward_ex(ht, spl_array_get_pos_ptr(ht, intern));
+					if (spl_array_is_object(intern)) {
+						spl_array_skip_protected(intern, ht);
 					}
-				} else if (zend_symtable_del(ht, Z_STR_P(offset)) == FAILURE) {
-					zend_error(E_WARNING,"Undefined array key \"%s\"", Z_STRVAL_P(offset));
 				}
-			} else {
-				zend_error(E_WARNING,"Undefined array key \"%s\"", Z_STRVAL_P(offset));
+			} else if (zend_hash_del(ht, key.key) == FAILURE) {
+				zend_error(E_WARNING,"Undefined array key \"%s\"", ZSTR_VAL(key.key));
 			}
+		} else {
+			zend_error(E_WARNING,"Undefined array key \"%s\"", ZSTR_VAL(key.key));
 		}
-		break;
-	case IS_DOUBLE:
-		index = (zend_long)Z_DVAL_P(offset);
-		goto num_index;
-	case IS_RESOURCE:
-		index = Z_RES_HANDLE_P(offset);
-		goto num_index;
-	case IS_FALSE:
-		index = 0;
-		goto num_index;
-	case IS_TRUE:
-		index = 1;
-		goto num_index;
-	case IS_LONG:
-		index = Z_LVAL_P(offset);
-num_index:
-		ht = spl_array_get_hash_table(intern);
-		if (zend_hash_index_del(ht, index) == FAILURE) {
-			zend_error(E_WARNING,"Undefined array key " ZEND_LONG_FMT, index);
+		spl_hash_key_release(&key);
+	} else {
+		if (zend_hash_index_del(ht, key.h) == FAILURE) {
+			zend_error(E_WARNING, "Undefined array key " ZEND_LONG_FMT, key.h);
 		}
-		break;
-	case IS_REFERENCE:
-		ZVAL_DEREF(offset);
-		goto try_again;
-	default:
-		zend_type_error("Illegal offset type in unset");
-		return;
 	}
 } /* }}} */
 
@@ -586,7 +581,6 @@ static void spl_array_unset_dimension(zend_object *object, zval *offset) /* {{{
 static int spl_array_has_dimension_ex(int check_inherited, zend_object *object, zval *offset, int check_empty) /* {{{ */
 {
 	spl_array_object *intern = spl_array_from_obj(object);
-	zend_long index;
 	zval rv, *value = NULL, *tmp;
 
 	if (check_inherited && intern->fptr_offset_has) {
@@ -607,48 +601,26 @@ static int spl_array_has_dimension_ex(int check_inherited, zend_object *object,
 
 	if (!value) {
 		HashTable *ht = spl_array_get_hash_table(intern);
+		spl_hash_key key;
 
-try_again:
-		switch (Z_TYPE_P(offset)) {
-			case IS_STRING:
-				if ((tmp = zend_symtable_find(ht, Z_STR_P(offset))) != NULL) {
-					if (check_empty == 2) {
-						return 1;
-					}
-				} else {
-					return 0;
-				}
-				break;
-
-			case IS_DOUBLE:
-				index = (zend_long)Z_DVAL_P(offset);
-				goto num_index;
-			case IS_RESOURCE:
-				index = Z_RES_HANDLE_P(offset);
-				goto num_index;
-			case IS_FALSE:
-				index = 0;
-				goto num_index;
-			case IS_TRUE:
-				index = 1;
-				goto num_index;
-			case IS_LONG:
-				index = Z_LVAL_P(offset);
-num_index:
-				if ((tmp = zend_hash_index_find(ht, index)) != NULL) {
-					if (check_empty == 2) {
-						return 1;
-					}
-				} else {
-					return 0;
-				}
-				break;
-			case IS_REFERENCE:
-				ZVAL_DEREF(offset);
-				goto try_again;
-			default:
-				zend_type_error("Illegal offset type in isset or empty");
-				return 0;
+		if (get_hash_key(&key, intern, offset) == FAILURE) {
+			zend_type_error("Illegal offset type in isset or empty");
+			return 0;
+		}
+
+		if (key.key) {
+			tmp = zend_hash_find(ht, key.key);
+			spl_hash_key_release(&key);
+		} else {
+			tmp = zend_hash_index_find(ht, key.h);
+		}
+
+		if (tmp) {
+			if (check_empty == 2) {
+				return 1;
+			}
+		} else {
+			return 0;
 		}
 
 		if (check_empty && check_inherited && intern->fptr_offset_get) {
diff --git a/ext/spl/tests/ArrayObject_proptable_canonicalization.phpt b/ext/spl/tests/ArrayObject_proptable_canonicalization.phpt
new file mode 100644
index 0000000000..006a152602
--- /dev/null
+++ b/ext/spl/tests/ArrayObject_proptable_canonicalization.phpt
@@ -0,0 +1,33 @@
+--TEST--
+When ArrayObject wraps an object, we should use proptable canonicalization
+--FILE--
+<?php
+
+$o = new stdClass;
+$ao = new ArrayObject($o);
+$ao[0] = 1;
+var_dump($o);
+$ao[0] += 1;
+var_dump($o);
+var_dump(isset($ao[0]));
+var_dump((array) $ao);
+unset($ao[0]);
+var_dump($o);
+
+?>
+--EXPECT--
+object(stdClass)#1 (1) {
+  ["0"]=>
+  int(1)
+}
+object(stdClass)#1 (1) {
+  ["0"]=>
+  int(2)
+}
+bool(true)
+array(1) {
+  [0]=>
+  int(2)
+}
+object(stdClass)#1 (0) {
+}