Some simplification to eliminate memory management issues

Author: StephenWall

ext/openssl/openssl.c

@@ -1400,7 +1400,7 @@ PHP_FUNCTION(openssl_x509_parse)
 	char *str_serial;
 	char *hex_serial;
 	char buf[256];
-	zval *altname = NULL;
+	zval altname;
 
 	ZEND_PARSE_PARAMETERS_START(1, 2)
 		Z_PARAM_OBJ_OF_CLASS_OR_STR(cert_obj, php_openssl_certificate_ce, cert_str)
@@ -1516,6 +1516,7 @@ PHP_FUNCTION(openssl_x509_parse)
 
 	array_init(&subitem);
 	array_init(&critext);
+	array_init(&altname);
 
 	for (i = 0; i < X509_get_ext_count(cert); i++) {
 		int nid;
@@ -1562,6 +1563,10 @@ PHP_FUNCTION(openssl_x509_parse)
 	if (altname != NULL) {
 	    add_assoc_zval(return_value, "subjectAlternativeName", altname);
 	}
+	ulong altcount = zend_hash_num_elements(Z_ARRVAL_P(&altname));
+	if (altcount > 0) {
+		add_assoc_zval(return_value, "subjectAlternativeName", &altname);
+	}
 	if (cert_str) {
 		X509_free(cert);
 	}
@@ -1570,10 +1575,7 @@ PHP_FUNCTION(openssl_x509_parse)
 err_subitem:
 	zval_ptr_dtor(&subitem);
 	zval_ptr_dtor(&critext);
-	if (altname != NULL) {
-	    zval_ptr_dtor(altname);
-	    efree(altname);
-	}
+	zval_ptr_dtor(&altname);
 err:
 	zend_array_destroy(Z_ARR_P(return_value));
 	if (cert_str) {

ext/openssl/openssl_backend_common.c

@@ -670,7 +670,7 @@ static void print_asn1_type(BIO *bio, ASN1_TYPE *ptr)
 /* Special handling of subjectAltName, see CVE-2013-4073
  * Christian Heimes
  */
-int openssl_x509v3_subjectAltName(BIO *bio, X509_EXTENSION *extension, zval **altname)
+int openssl_x509v3_subjectAltName(BIO *bio, X509_EXTENSION *extension, zval *altname)
 {
 	GENERAL_NAMES *names;
 	const X509V3_EXT_METHOD *method = NULL;
@@ -699,12 +699,6 @@ int openssl_x509v3_subjectAltName(BIO *bio, X509_EXTENSION *extension, zval **al
 	}
 
 	num = sk_GENERAL_NAME_num(names);
-	if (altname != NULL) {
-		if (*altname == NULL) {
-			*altname = (zval *)safe_emalloc(1, sizeof(zval), 0);
-		}
-		array_init(*altname);
-	}
 	for (i = 0; i < num; i++) {
 		GENERAL_NAME *name;
 		ASN1_STRING *as;
@@ -720,7 +714,7 @@ int openssl_x509v3_subjectAltName(BIO *bio, X509_EXTENSION *extension, zval **al
 				if (altname != NULL) {
 					add_assoc_string(&entry, "type", "email");
 					php_openssl_add_assoc_asn1_string(&entry, "value", as);
-					add_index_zval(*altname, index++, &entry);
+					add_index_zval(altname, index++, &entry);
 				}
 				break;
 			case GEN_DNS:
@@ -731,7 +725,7 @@ int openssl_x509v3_subjectAltName(BIO *bio, X509_EXTENSION *extension, zval **al
 				if (altname != NULL) {
 					add_assoc_string(&entry, "type", "DNS");
 					php_openssl_add_assoc_asn1_string(&entry, "value", as);
-					add_index_zval(*altname, index++, &entry);
+					add_index_zval(altname, index++, &entry);
 				}
 				break;
 			case GEN_URI:
@@ -742,15 +736,15 @@ int openssl_x509v3_subjectAltName(BIO *bio, X509_EXTENSION *extension, zval **al
 				if (altname != NULL) {
 					add_assoc_string(&entry, "type", "URI");
 					php_openssl_add_assoc_asn1_string(&entry, "value", as);
-					add_index_zval(*altname, index++, &entry);
+					add_index_zval(altname, index++, &entry);
 				}
 				break;
 			case GEN_DIRNAME:
 				GENERAL_NAME_print(bio, name);
 				if (altname != NULL) {
 					add_assoc_string(&entry, "type", "DirName");
 					php_openssl_add_assoc_name_entry(&entry, "value", name->d.dirn, PHP_OPENSSL_OID);
-					add_index_zval(*altname, index++, &entry);
+					add_index_zval(altname, index++, &entry);
 				}
 				break;
 			case GEN_RID:
@@ -760,7 +754,7 @@ int openssl_x509v3_subjectAltName(BIO *bio, X509_EXTENSION *extension, zval **al
 					OBJ_obj2txt(buf, sizeof(buf)-1, name->d.rid, 1);
 					add_assoc_string(&entry, "type", "Registered ID");
 					add_assoc_string(&entry, "value", buf);
-					add_index_zval(*altname, index++, &entry);
+					add_index_zval(altname, index++, &entry);
 				}
 				break;
 			case GEN_IPADD:
@@ -776,7 +770,7 @@ int openssl_x509v3_subjectAltName(BIO *bio, X509_EXTENSION *extension, zval **al
 					}
 					add_assoc_string(&entry, "type", "IP Address");
 					add_assoc_string(&entry, "value", buf);
-					add_index_zval(*altname, index++, &entry);
+					add_index_zval(altname, index++, &entry);
 				}
 				break;
 			case GEN_OTHERNAME:
@@ -797,7 +791,7 @@ int openssl_x509v3_subjectAltName(BIO *bio, X509_EXTENSION *extension, zval **al
 					add_assoc_stringl(&value, oid, bio_buf->data, bio_buf->length);
 					add_assoc_string(&entry, "type", "othername");
 					add_assoc_zval(&entry, "value", &value);
-					add_index_zval(*altname, index++, &entry);
+					add_index_zval(altname, index++, &entry);
 					BIO_free(bio_out);
 				}
 				break;
@@ -821,7 +815,7 @@ int openssl_x509v3_subjectAltName(BIO *bio, X509_EXTENSION *extension, zval **al
 							break;
 					}
 					add_assoc_stringl(&entry, "value", bio_buf->data, bio_buf->length);
-					add_index_zval(*altname, index++, &entry);
+					add_index_zval(altname, index++, &entry);
 					BIO_free(bio_out);
 				}
 		}

ext/openssl/php_openssl_backend.h

@@ -272,7 +272,7 @@ X509 *php_openssl_x509_from_zval(
 
 zend_string* php_openssl_x509_fingerprint(X509 *peer, const char *method, bool raw);
 
-int openssl_x509v3_subjectAltName(BIO *bio, X509_EXTENSION *extension, zval **altname);
+int openssl_x509v3_subjectAltName(BIO *bio, X509_EXTENSION *extension, zval *altname);
 
 STACK_OF(X509) *php_openssl_load_all_certs_from_file(
 		char *cert_file, size_t cert_file_len, uint32_t arg_num);