Commit 69664d6a authored by Viktor Dukhovni's avatar Viktor Dukhovni
Browse files

Future proof build_chain() in x509_vfy.c 



Coverity reports a potential NULL deref when "2 0 0" DANE trust-anchors
from DNS are configured via SSL_dane_tlsa_add() and X509_STORE_CTX_init()
is called with a NULL stack of untrusted certificates.

Since ssl_verify_cert_chain() always provideds a non-NULL stack of
untrusted certs, and no other code path enables DANE, the problem
can only happen in applications that use SSL_CTX_set_cert_verify_callback()
to implement their own wrappers around X509_verify_cert() passing
only the leaf certificate to the latter.

Regardless of the "improbability" of the problem, we do need to
ensure that build_chain() handles this case correctly.

Reviewed-by: default avatarMatt Caswell <matt@openssl.org>
parent 4c5e6b2c

crypto/x509/x509_vfy.c

+14 −1
Original line number Diff line number Diff line
@@ -2789,8 +2789,21 @@ static int build_chain(X509_STORE_CTX *ctx) 
        return 0;

    }



    /* Include any untrusted full certificates from DNS */

    /*

     * If we got any "DANE-TA(2) Cert(0) Full(0)" trust-anchors from DNS, add

     * them to our working copy of the untrusted certificate stack.  Since the

     * caller of X509_STORE_CTX_init() may have provided only a leaf cert with

     * no corresponding stack of untrusted certificates, we may need to create

     * an empty stack first.  [ At present only the ssl library provides DANE

     * support, and ssl_verify_cert_chain() always provides a non-null stack

     * containing at least the leaf certificate, but we must be prepared for

     * this to change. ]

     */

    if (DANETLS_ENABLED(dane) && dane->certs != NULL) {

        if (sktmp == NULL && (sktmp = sk_X509_new_null()) == NULL) {

            X509err(X509_F_BUILD_CHAIN, ERR_R_MALLOC_FAILURE);

            return 0;

        }

        for (i = 0; i < sk_X509_num(dane->certs); ++i) {

            if (!sk_X509_push(sktmp, sk_X509_value(dane->certs, i))) {

                sk_X509_free(sktmp);