Skip to content

Commit 43942b4

Browse files
ndosschendossche
committed
openssl: Fix checks on BIO_get_mem_ptr()
If the call fails, then the pointer remains uninitialized, and this triggers undefined behaviour or the reading of a dangling pointer. In my own tests this came out as a UAF. However, it seems not exploitable by an attacker as failure should be not controllable. It's worth pointing out that OpenSSL checks the return value in its own code that calls this function as well [1]. [1] https://github.com/openssl/openssl/blob/b2ecef451ccede07366023da4553f113f6e4fe71/apps/lib/apps.c#L3307-L3311
1 parent 9add43a commit 43942b4

1 file changed

Lines changed: 27 additions & 40 deletions

File tree

‎ext/openssl/openssl.c‎

Lines changed: 27 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1129,6 +1129,7 @@ PHP_FUNCTION(openssl_spki_export)
11291129
EVP_PKEY *pkey = NULL;
11301130
NETSCAPE_SPKI *spki = NULL;
11311131
BIO *out = NULL;
1132+
BUF_MEM *bio_buf;
11321133

11331134
if (zend_parse_parameters(ZEND_NUM_ARGS(), "s", &spkstr, &spkstr_len) == FAILURE) {
11341135
RETURN_THROWS();
@@ -1158,10 +1159,7 @@ PHP_FUNCTION(openssl_spki_export)
11581159
}
11591160

11601161
out = BIO_new(BIO_s_mem());
1161-
if (out && PEM_write_bio_PUBKEY(out, pkey)) {
1162-
BUF_MEM *bio_buf;
1163-
1164-
BIO_get_mem_ptr(out, &bio_buf);
1162+
if (out && PEM_write_bio_PUBKEY(out, pkey) && BIO_get_mem_ptr(out, &bio_buf) > 0) {
11651163
RETVAL_STRINGL((char *)bio_buf->data, bio_buf->length);
11661164
} else {
11671165
php_openssl_store_errors();
@@ -1232,6 +1230,7 @@ PHP_FUNCTION(openssl_x509_export)
12321230
zval *zout;
12331231
bool notext = 1;
12341232
BIO * bio_out;
1233+
BUF_MEM *bio_buf;
12351234

12361235
ZEND_PARSE_PARAMETERS_START(2, 3)
12371236
Z_PARAM_OBJ_OF_CLASS_OR_STR(cert_obj, php_openssl_certificate_ce, cert_str)
@@ -1255,10 +1254,7 @@ PHP_FUNCTION(openssl_x509_export)
12551254
}
12561255
if (!notext && !X509_print(bio_out, cert)) {
12571256
php_openssl_store_errors();
1258-
} else if (PEM_write_bio_X509(bio_out, cert)) {
1259-
BUF_MEM *bio_buf;
1260-
1261-
BIO_get_mem_ptr(bio_out, &bio_buf);
1257+
} else if (PEM_write_bio_X509(bio_out, cert) && BIO_get_mem_ptr(bio_out, &bio_buf) > 0) {
12621258
ZEND_TRY_ASSIGN_REF_STRINGL(zout, bio_buf->data, bio_buf->length);
12631259

12641260
RETVAL_TRUE;
@@ -1539,16 +1535,18 @@ PHP_FUNCTION(openssl_x509_parse)
15391535
goto err_subitem;
15401536
}
15411537
if (nid == NID_subject_alt_name) {
1542-
if (openssl_x509v3_subjectAltName(bio_out, extension) == 0) {
1543-
BIO_get_mem_ptr(bio_out, &bio_buf);
1538+
if (openssl_x509v3_subjectAltName(bio_out, extension) == 0 && BIO_get_mem_ptr(bio_out, &bio_buf) > 0) {
15441539
add_assoc_stringl(&subitem, extname, bio_buf->data, bio_buf->length);
15451540
} else {
15461541
BIO_free(bio_out);
15471542
goto err_subitem;
15481543
}
15491544
}
15501545
else if (X509V3_EXT_print(bio_out, extension, 0, 0) > 0) {
1551-
BIO_get_mem_ptr(bio_out, &bio_buf);
1546+
if (BIO_get_mem_ptr(bio_out, &bio_buf) <= 0) {
1547+
BIO_free(bio_out);
1548+
goto err_subitem;
1549+
}
15521550
add_assoc_stringl(&subitem, extname, bio_buf->data, bio_buf->length);
15531551
} else {
15541552
php_openssl_add_assoc_asn1_string(&subitem, extname, X509_EXTENSION_get_data(extension));
@@ -1854,11 +1852,9 @@ PHP_FUNCTION(openssl_pkcs12_export)
18541852
p12 = PKCS12_create(pass, friendly_name, priv_key, cert, ca, 0, 0, 0, 0, 0);
18551853

18561854
if (p12 != NULL) {
1855+
BUF_MEM *bio_buf;
18571856
bio_out = BIO_new(BIO_s_mem());
1858-
if (bio_out && i2d_PKCS12_bio(bio_out, p12)) {
1859-
BUF_MEM *bio_buf;
1860-
1861-
BIO_get_mem_ptr(bio_out, &bio_buf);
1857+
if (bio_out && i2d_PKCS12_bio(bio_out, p12) && BIO_get_mem_ptr(bio_out, &bio_buf) > 0) {
18621858
ZEND_TRY_ASSIGN_REF_STRINGL(zout, bio_buf->data, bio_buf->length);
18631859

18641860
RETVAL_TRUE;
@@ -1920,10 +1916,9 @@ PHP_FUNCTION(openssl_pkcs12_read)
19201916
}
19211917

19221918
if (cert) {
1919+
BUF_MEM *bio_buf;
19231920
bio_out = BIO_new(BIO_s_mem());
1924-
if (bio_out && PEM_write_bio_X509(bio_out, cert)) {
1925-
BUF_MEM *bio_buf;
1926-
BIO_get_mem_ptr(bio_out, &bio_buf);
1921+
if (bio_out && PEM_write_bio_X509(bio_out, cert) && BIO_get_mem_ptr(bio_out, &bio_buf) > 0) {
19271922
ZVAL_STRINGL(&zcert, bio_buf->data, bio_buf->length);
19281923
add_assoc_zval(zout, "cert", &zcert);
19291924
} else {
@@ -1933,14 +1928,13 @@ PHP_FUNCTION(openssl_pkcs12_read)
19331928
}
19341929

19351930
if (pkey) {
1931+
BUF_MEM *bio_buf;
19361932
bio_out = BIO_new(BIO_s_mem());
19371933
if (!bio_out) {
19381934
goto cleanup;
19391935
}
19401936

1941-
if (PEM_write_bio_PrivateKey(bio_out, pkey, NULL, NULL, 0, 0, NULL)) {
1942-
BUF_MEM *bio_buf;
1943-
BIO_get_mem_ptr(bio_out, &bio_buf);
1937+
if (PEM_write_bio_PrivateKey(bio_out, pkey, NULL, NULL, 0, 0, NULL) && BIO_get_mem_ptr(bio_out, &bio_buf) > 0) {
19441938
ZVAL_STRINGL(&zpkey, bio_buf->data, bio_buf->length);
19451939
add_assoc_zval(zout, "pkey", &zpkey);
19461940
} else {
@@ -1959,12 +1953,11 @@ PHP_FUNCTION(openssl_pkcs12_read)
19591953

19601954
for (i = 0; i < cert_num; i++) {
19611955
zval zextracert;
1956+
BUF_MEM *bio_buf;
19621957
X509* aCA = sk_X509_pop(ca);
19631958
if (!aCA) break;
19641959

1965-
if (PEM_write_bio_X509(bio_out, aCA)) {
1966-
BUF_MEM *bio_buf;
1967-
BIO_get_mem_ptr(bio_out, &bio_buf);
1960+
if (PEM_write_bio_X509(bio_out, aCA) && BIO_get_mem_ptr(bio_out, &bio_buf) > 0) {
19681961
ZVAL_STRINGL(&zextracert, bio_buf->data, bio_buf->length);
19691962
add_index_zval(&zextracerts, i, &zextracert);
19701963
}
@@ -2061,6 +2054,7 @@ PHP_FUNCTION(openssl_csr_export)
20612054
zval *zout;
20622055
bool notext = 1;
20632056
BIO * bio_out;
2057+
BUF_MEM *bio_buf;
20642058

20652059
ZEND_PARSE_PARAMETERS_START(2, 3)
20662060
Z_PARAM_OBJ_OF_CLASS_OR_STR(csr_obj, php_openssl_request_ce, csr_str)
@@ -2082,10 +2076,7 @@ PHP_FUNCTION(openssl_csr_export)
20822076
bio_out = BIO_new(BIO_s_mem());
20832077
if (!notext && !X509_REQ_print(bio_out, csr)) {
20842078
php_openssl_store_errors();
2085-
} else if (PEM_write_bio_X509_REQ(bio_out, csr)) {
2086-
BUF_MEM *bio_buf;
2087-
2088-
BIO_get_mem_ptr(bio_out, &bio_buf);
2079+
} else if (PEM_write_bio_X509_REQ(bio_out, csr) && BIO_get_mem_ptr(bio_out, &bio_buf) > 0) {
20892080
ZEND_TRY_ASSIGN_REF_STRINGL(zout, bio_buf->data, bio_buf->length);
20902081

20912082
RETVAL_TRUE;
@@ -3243,11 +3234,10 @@ PHP_FUNCTION(openssl_pkcs7_read)
32433234
goto clean_exit;
32443235
}
32453236
for (i = 0; i < sk_X509_num(certs); i++) {
3237+
BUF_MEM *bio_buf;
32463238
X509* ca = sk_X509_value(certs, i);
32473239

3248-
if (PEM_write_bio_X509(bio_out, ca)) {
3249-
BUF_MEM *bio_buf;
3250-
BIO_get_mem_ptr(bio_out, &bio_buf);
3240+
if (PEM_write_bio_X509(bio_out, ca) && BIO_get_mem_ptr(bio_out, &bio_buf) > 0) {
32513241
ZVAL_STRINGL(&zcert, bio_buf->data, bio_buf->length);
32523242
add_index_zval(zout, i, &zcert);
32533243
}
@@ -3262,11 +3252,10 @@ PHP_FUNCTION(openssl_pkcs7_read)
32623252
goto clean_exit;
32633253
}
32643254
for (i = 0; i < sk_X509_CRL_num(crls); i++) {
3255+
BUF_MEM *bio_buf;
32653256
X509_CRL* crl = sk_X509_CRL_value(crls, i);
32663257

3267-
if (PEM_write_bio_X509_CRL(bio_out, crl)) {
3268-
BUF_MEM *bio_buf;
3269-
BIO_get_mem_ptr(bio_out, &bio_buf);
3258+
if (PEM_write_bio_X509_CRL(bio_out, crl) && BIO_get_mem_ptr(bio_out, &bio_buf) > 0) {
32703259
ZVAL_STRINGL(&zcert, bio_buf->data, bio_buf->length);
32713260
add_index_zval(zout, i, &zcert);
32723261
}
@@ -3919,11 +3908,10 @@ PHP_FUNCTION(openssl_cms_read)
39193908
}
39203909

39213910
for (i = 0; i < sk_X509_num(certs); i++) {
3911+
BUF_MEM *bio_buf;
39223912
X509* ca = sk_X509_value(certs, i);
39233913

3924-
if (PEM_write_bio_X509(bio_out, ca)) {
3925-
BUF_MEM *bio_buf;
3926-
BIO_get_mem_ptr(bio_out, &bio_buf);
3914+
if (PEM_write_bio_X509(bio_out, ca) && BIO_get_mem_ptr(bio_out, &bio_buf) > 0) {
39273915
ZVAL_STRINGL(&zcert, bio_buf->data, bio_buf->length);
39283916
add_index_zval(zout, i, &zcert);
39293917
}
@@ -3939,11 +3927,10 @@ PHP_FUNCTION(openssl_cms_read)
39393927
}
39403928

39413929
for (i = 0; i < sk_X509_CRL_num(crls); i++) {
3930+
BUF_MEM *bio_buf;
39423931
X509_CRL* crl = sk_X509_CRL_value(crls, i);
39433932

3944-
if (PEM_write_bio_X509_CRL(bio_out, crl)) {
3945-
BUF_MEM *bio_buf;
3946-
BIO_get_mem_ptr(bio_out, &bio_buf);
3933+
if (PEM_write_bio_X509_CRL(bio_out, crl) && BIO_get_mem_ptr(bio_out, &bio_buf) > 0) {
39473934
ZVAL_STRINGL(&zcert, bio_buf->data, bio_buf->length);
39483935
add_index_zval(zout, i, &zcert);
39493936
}

0 commit comments

Comments
 (0)