Skip to content

Commit 4cf037c

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 85b23b0 commit 4cf037c

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
@@ -738,6 +738,7 @@ PHP_FUNCTION(openssl_spki_export)
738738
EVP_PKEY *pkey = NULL;
739739
NETSCAPE_SPKI *spki = NULL;
740740
BIO *out = NULL;
741+
BUF_MEM *bio_buf;
741742

742743
if (zend_parse_parameters(ZEND_NUM_ARGS(), "s", &spkstr, &spkstr_len) == FAILURE) {
743744
RETURN_THROWS();
@@ -767,10 +768,7 @@ PHP_FUNCTION(openssl_spki_export)
767768
}
768769

769770
out = BIO_new(BIO_s_mem());
770-
if (out && PEM_write_bio_PUBKEY(out, pkey)) {
771-
BUF_MEM *bio_buf;
772-
773-
BIO_get_mem_ptr(out, &bio_buf);
771+
if (out && PEM_write_bio_PUBKEY(out, pkey) && BIO_get_mem_ptr(out, &bio_buf) > 0) {
774772
RETVAL_STRINGL((char *)bio_buf->data, bio_buf->length);
775773
} else {
776774
php_openssl_store_errors();
@@ -841,6 +839,7 @@ PHP_FUNCTION(openssl_x509_export)
841839
zval *zout;
842840
bool notext = 1;
843841
BIO * bio_out;
842+
BUF_MEM *bio_buf;
844843

845844
ZEND_PARSE_PARAMETERS_START(2, 3)
846845
Z_PARAM_OBJ_OF_CLASS_OR_STR(cert_obj, php_openssl_certificate_ce, cert_str)
@@ -865,10 +864,7 @@ PHP_FUNCTION(openssl_x509_export)
865864
if (!notext && !X509_print(bio_out, cert)) {
866865
php_openssl_store_errors();
867866
}
868-
if (PEM_write_bio_X509(bio_out, cert)) {
869-
BUF_MEM *bio_buf;
870-
871-
BIO_get_mem_ptr(bio_out, &bio_buf);
867+
if (PEM_write_bio_X509(bio_out, cert) && BIO_get_mem_ptr(bio_out, &bio_buf) > 0) {
872868
ZEND_TRY_ASSIGN_REF_STRINGL(zout, bio_buf->data, bio_buf->length);
873869

874870
RETVAL_TRUE;
@@ -1148,16 +1144,18 @@ PHP_FUNCTION(openssl_x509_parse)
11481144
goto err_subitem;
11491145
}
11501146
if (nid == NID_subject_alt_name) {
1151-
if (openssl_x509v3_subjectAltName(bio_out, extension) == 0) {
1152-
BIO_get_mem_ptr(bio_out, &bio_buf);
1147+
if (openssl_x509v3_subjectAltName(bio_out, extension) == 0 && BIO_get_mem_ptr(bio_out, &bio_buf) > 0) {
11531148
add_assoc_stringl(&subitem, extname, bio_buf->data, bio_buf->length);
11541149
} else {
11551150
BIO_free(bio_out);
11561151
goto err_subitem;
11571152
}
11581153
}
11591154
else if (X509V3_EXT_print(bio_out, extension, 0, 0)) {
1160-
BIO_get_mem_ptr(bio_out, &bio_buf);
1155+
if (BIO_get_mem_ptr(bio_out, &bio_buf) <= 0) {
1156+
BIO_free(bio_out);
1157+
goto err_subitem;
1158+
}
11611159
add_assoc_stringl(&subitem, extname, bio_buf->data, bio_buf->length);
11621160
} else {
11631161
php_openssl_add_assoc_asn1_string(&subitem, extname, X509_EXTENSION_get_data(extension));
@@ -1452,11 +1450,9 @@ PHP_FUNCTION(openssl_pkcs12_export)
14521450
p12 = PKCS12_create(pass, friendly_name, priv_key, cert, ca, 0, 0, 0, 0, 0);
14531451

14541452
if (p12 != NULL) {
1453+
BUF_MEM *bio_buf;
14551454
bio_out = BIO_new(BIO_s_mem());
1456-
if (i2d_PKCS12_bio(bio_out, p12)) {
1457-
BUF_MEM *bio_buf;
1458-
1459-
BIO_get_mem_ptr(bio_out, &bio_buf);
1455+
if (i2d_PKCS12_bio(bio_out, p12) && BIO_get_mem_ptr(bio_out, &bio_buf) > 0) {
14601456
ZEND_TRY_ASSIGN_REF_STRINGL(zout, bio_buf->data, bio_buf->length);
14611457

14621458
RETVAL_TRUE;
@@ -1517,10 +1513,9 @@ PHP_FUNCTION(openssl_pkcs12_read)
15171513
}
15181514

15191515
if (cert) {
1516+
BUF_MEM *bio_buf;
15201517
bio_out = BIO_new(BIO_s_mem());
1521-
if (PEM_write_bio_X509(bio_out, cert)) {
1522-
BUF_MEM *bio_buf;
1523-
BIO_get_mem_ptr(bio_out, &bio_buf);
1518+
if (PEM_write_bio_X509(bio_out, cert) && BIO_get_mem_ptr(bio_out, &bio_buf) > 0) {
15241519
ZVAL_STRINGL(&zcert, bio_buf->data, bio_buf->length);
15251520
add_assoc_zval(zout, "cert", &zcert);
15261521
} else {
@@ -1530,10 +1525,9 @@ PHP_FUNCTION(openssl_pkcs12_read)
15301525
}
15311526

15321527
if (pkey) {
1528+
BUF_MEM *bio_buf;
15331529
bio_out = BIO_new(BIO_s_mem());
1534-
if (PEM_write_bio_PrivateKey(bio_out, pkey, NULL, NULL, 0, 0, NULL)) {
1535-
BUF_MEM *bio_buf;
1536-
BIO_get_mem_ptr(bio_out, &bio_buf);
1530+
if (PEM_write_bio_PrivateKey(bio_out, pkey, NULL, NULL, 0, 0, NULL) && BIO_get_mem_ptr(bio_out, &bio_buf) > 0) {
15371531
ZVAL_STRINGL(&zpkey, bio_buf->data, bio_buf->length);
15381532
add_assoc_zval(zout, "pkey", &zpkey);
15391533
} else {
@@ -1548,13 +1542,12 @@ PHP_FUNCTION(openssl_pkcs12_read)
15481542

15491543
for (i = 0; i < cert_num; i++) {
15501544
zval zextracert;
1545+
BUF_MEM *bio_buf;
15511546
X509* aCA = sk_X509_pop(ca);
15521547
if (!aCA) break;
15531548

15541549
bio_out = BIO_new(BIO_s_mem());
1555-
if (PEM_write_bio_X509(bio_out, aCA)) {
1556-
BUF_MEM *bio_buf;
1557-
BIO_get_mem_ptr(bio_out, &bio_buf);
1550+
if (PEM_write_bio_X509(bio_out, aCA) && BIO_get_mem_ptr(bio_out, &bio_buf) > 0) {
15581551
ZVAL_STRINGL(&zextracert, bio_buf->data, bio_buf->length);
15591552
add_index_zval(&zextracerts, i, &zextracert);
15601553
}
@@ -1650,6 +1643,7 @@ PHP_FUNCTION(openssl_csr_export)
16501643
zval *zout;
16511644
bool notext = 1;
16521645
BIO * bio_out;
1646+
BUF_MEM *bio_buf;
16531647

16541648
ZEND_PARSE_PARAMETERS_START(2, 3)
16551649
Z_PARAM_OBJ_OF_CLASS_OR_STR(csr_obj, php_openssl_request_ce, csr_str)
@@ -1673,10 +1667,7 @@ PHP_FUNCTION(openssl_csr_export)
16731667
php_openssl_store_errors();
16741668
}
16751669

1676-
if (PEM_write_bio_X509_REQ(bio_out, csr)) {
1677-
BUF_MEM *bio_buf;
1678-
1679-
BIO_get_mem_ptr(bio_out, &bio_buf);
1670+
if (PEM_write_bio_X509_REQ(bio_out, csr) && BIO_get_mem_ptr(bio_out, &bio_buf) > 0) {
16801671
ZEND_TRY_ASSIGN_REF_STRINGL(zout, bio_buf->data, bio_buf->length);
16811672

16821673
RETVAL_TRUE;
@@ -2815,12 +2806,11 @@ PHP_FUNCTION(openssl_pkcs7_read)
28152806

28162807
if (certs != NULL) {
28172808
for (i = 0; i < sk_X509_num(certs); i++) {
2809+
BUF_MEM *bio_buf;
28182810
X509* ca = sk_X509_value(certs, i);
28192811

28202812
bio_out = BIO_new(BIO_s_mem());
2821-
if (bio_out && PEM_write_bio_X509(bio_out, ca)) {
2822-
BUF_MEM *bio_buf;
2823-
BIO_get_mem_ptr(bio_out, &bio_buf);
2813+
if (bio_out && PEM_write_bio_X509(bio_out, ca) && BIO_get_mem_ptr(bio_out, &bio_buf) > 0) {
28242814
ZVAL_STRINGL(&zcert, bio_buf->data, bio_buf->length);
28252815
add_index_zval(zout, i, &zcert);
28262816
}
@@ -2830,12 +2820,11 @@ PHP_FUNCTION(openssl_pkcs7_read)
28302820

28312821
if (crls != NULL) {
28322822
for (i = 0; i < sk_X509_CRL_num(crls); i++) {
2823+
BUF_MEM *bio_buf;
28332824
X509_CRL* crl = sk_X509_CRL_value(crls, i);
28342825

28352826
bio_out = BIO_new(BIO_s_mem());
2836-
if (bio_out && PEM_write_bio_X509_CRL(bio_out, crl)) {
2837-
BUF_MEM *bio_buf;
2838-
BIO_get_mem_ptr(bio_out, &bio_buf);
2827+
if (bio_out && PEM_write_bio_X509_CRL(bio_out, crl) && BIO_get_mem_ptr(bio_out, &bio_buf) > 0) {
28392828
ZVAL_STRINGL(&zcert, bio_buf->data, bio_buf->length);
28402829
add_index_zval(zout, i, &zcert);
28412830
}
@@ -3482,12 +3471,11 @@ PHP_FUNCTION(openssl_cms_read)
34823471

34833472
if (certs != NULL) {
34843473
for (i = 0; i < sk_X509_num(certs); i++) {
3474+
BUF_MEM *bio_buf;
34853475
X509* ca = sk_X509_value(certs, i);
34863476

34873477
bio_out = BIO_new(BIO_s_mem());
3488-
if (bio_out && PEM_write_bio_X509(bio_out, ca)) {
3489-
BUF_MEM *bio_buf;
3490-
BIO_get_mem_ptr(bio_out, &bio_buf);
3478+
if (bio_out && PEM_write_bio_X509(bio_out, ca) && BIO_get_mem_ptr(bio_out, &bio_buf) > 0) {
34913479
ZVAL_STRINGL(&zcert, bio_buf->data, bio_buf->length);
34923480
add_index_zval(zout, i, &zcert);
34933481
}
@@ -3497,12 +3485,11 @@ PHP_FUNCTION(openssl_cms_read)
34973485

34983486
if (crls != NULL) {
34993487
for (i = 0; i < sk_X509_CRL_num(crls); i++) {
3488+
BUF_MEM *bio_buf;
35003489
X509_CRL* crl = sk_X509_CRL_value(crls, i);
35013490

35023491
bio_out = BIO_new(BIO_s_mem());
3503-
if (bio_out && PEM_write_bio_X509_CRL(bio_out, crl)) {
3504-
BUF_MEM *bio_buf;
3505-
BIO_get_mem_ptr(bio_out, &bio_buf);
3492+
if (bio_out && PEM_write_bio_X509_CRL(bio_out, crl) && BIO_get_mem_ptr(bio_out, &bio_buf) > 0) {
35063493
ZVAL_STRINGL(&zcert, bio_buf->data, bio_buf->length);
35073494
add_index_zval(zout, i, &zcert);
35083495
}

0 commit comments

Comments
 (0)