Merge branch 'PHP-7.0' into PHP-7.1

krakjoe · krakjoe · commit 5ae45ef99374 · 2017-01-07T10:15:15.000Z
* PHP-7.0: Fix memleaks from #1755 and some pre-existing ones
diff --git a/ext/openssl/openssl.c b/ext/openssl/openssl.c
@@ -836,9 +836,9 @@ static void add_assoc_name_entry(zval * val, char * key, X509_NAME * name, int s
 	}
 
 	for (i = 0; i < X509_NAME_entry_count(name); i++) {
-		unsigned char *to_add;
+		unsigned char *to_add = NULL;
 		int to_add_len = 0;
-
+		int needs_free = 0;
 
 		ne = X509_NAME_get_entry(name, i);
 		obj = X509_NAME_ENTRY_get_object(ne);
@@ -852,8 +852,11 @@ static void add_assoc_name_entry(zval * val, char * key, X509_NAME * name, int s
 
 		str = X509_NAME_ENTRY_get_data(ne);
 		if (ASN1_STRING_type(str) != V_ASN1_UTF8STRING) {
+			/* ASN1_STRING_to_UTF8(3): The converted data is copied into a newly allocated buffer */
 			to_add_len = ASN1_STRING_to_UTF8(&to_add, str);
+			needs_free = 1;
 		} else {
+			/* ASN1_STRING_data(3): Since this is an internal pointer it should not be freed or modified in any way */
 			to_add = ASN1_STRING_data(str);
 			to_add_len = ASN1_STRING_length(str);
 		}
@@ -874,7 +877,13 @@ static void add_assoc_name_entry(zval * val, char * key, X509_NAME * name, int s
 		} else {
 			php_openssl_store_errors();
 		}
+
+		if (needs_free) {
+			/* ASN1_STRING_to_UTF8(3): The buffer out should be freed using free(3) */
+			free(to_add);
+		}
 	}
+
 	if (key != NULL) {
 		zend_hash_str_update(Z_ARRVAL_P(val), key, strlen(key), &subitem);
 	}
@@ -2266,7 +2275,10 @@ PHP_FUNCTION(openssl_x509_parse)
 	char *extname;
 	BIO *bio_out;
 	BUF_MEM *bio_buf;
-	char * hexserial;
+	ASN1_INTEGER *asn1_serial;
+	BIGNUM *bn_serial;
+	char *str_serial;
+	char *hex_serial;
 	char buf[256];
 
 	if (zend_parse_parameters(ZEND_NUM_ARGS(), "z|b", &zcert, &useshortnames) == FAILURE) {
@@ -2294,19 +2306,28 @@ PHP_FUNCTION(openssl_x509_parse)
 	add_assoc_name_entry(return_value, "issuer", 		X509_get_issuer_name(cert), useshortnames);
 	add_assoc_long(return_value, "version", 			X509_get_version(cert));
 
-	add_assoc_string(return_value, "serialNumber", i2s_ASN1_INTEGER(NULL, X509_get_serialNumber(cert)));
+	asn1_serial = X509_get_serialNumber(cert);
 
-	/* Return the hex representation of the serial number, as defined by OpenSSL */
-	hexserial = BN_bn2hex(ASN1_INTEGER_to_BN(X509_get_serialNumber(cert), NULL));
+	bn_serial = ASN1_INTEGER_to_BN(asn1_serial, NULL);
+	/* Can return NULL on error or memory allocation failure */
+	if (!bn_serial) {
+		RETURN_FALSE;
+	}
 
-	/* If we received null back from BN_bn2hex, there was a critical error in openssl,
-	 * and we should not continue.
-	 */
-	if (!hexserial) {
+	hex_serial = BN_bn2hex(bn_serial);
+	BN_free(bn_serial);
+	/* Can return NULL on error or memory allocation failure */
+	if (!hex_serial) {
 		RETURN_FALSE;
 	}
-	add_assoc_string(return_value, "serialNumberHex", hexserial); 
-	OPENSSL_free(hexserial);
+
+	str_serial = i2s_ASN1_INTEGER(NULL, asn1_serial);
+	add_assoc_string(return_value, "serialNumber", str_serial);
+	OPENSSL_free(str_serial);
+
+	/* Return the hex representation of the serial number, as defined by OpenSSL */
+	add_assoc_string(return_value, "serialNumberHex", hex_serial);
+	OPENSSL_free(hex_serial);
 
 	add_assoc_asn1_string(return_value, "validFrom", 	X509_get_notBefore(cert));
 	add_assoc_asn1_string(return_value, "validTo", 		X509_get_notAfter(cert));
