Commit 55500ea7 authored by Alessandro Ghedini's avatar Alessandro Ghedini Committed by Rich Salz
Browse files

GH354: Memory leak fixes 



Fix more potential leaks in X509_verify_cert()
Fix memory leak in ClientHello test
Fix memory leak in gost2814789 test
Fix potential memory leak in PKCS7_verify()
Fix potential memory leaks in X509_add1_reject_object()
Refactor to use "goto err" in cleanup.

Signed-off-by: default avatarRich Salz <rsalz@akamai.com>
Reviewed-by: default avatarEmilia Käsper <emilia@openssl.org>
parent f00a10b8

crypto/asn1/x_x509a.c

+5 −2
Original line number Original line Diff line number Diff line
@@ -172,11 +172,14 @@ int X509_add1_reject_object(X509 *x, ASN1_OBJECT *obj) 
    if ((objtmp = OBJ_dup(obj)) == NULL)

    if ((objtmp = OBJ_dup(obj)) == NULL)

        return 0;

        return 0;

    if ((aux = aux_get(x)) == NULL)

    if ((aux = aux_get(x)) == NULL)

        return 0;

        goto err;

    if (aux->reject == NULL

    if (aux->reject == NULL

        && (aux->reject = sk_ASN1_OBJECT_new_null()) == NULL)

        && (aux->reject = sk_ASN1_OBJECT_new_null()) == NULL)

        return 0;

        goto err;

    return sk_ASN1_OBJECT_push(aux->reject, objtmp);

    return sk_ASN1_OBJECT_push(aux->reject, objtmp);

 err:

    ASN1_OBJECT_free(objtmp);

    return 0;

}

}





void X509_trust_clear(X509 *x)

void X509_trust_clear(X509 *x)

crypto/pkcs7/pk7_smime.c

+6 −20
Original line number Original line Diff line number Diff line
@@ -255,8 +255,8 @@ int PKCS7_verify(PKCS7 *p7, STACK_OF(X509) *certs, X509_STORE *store, 
    X509_STORE_CTX cert_ctx;

    X509_STORE_CTX cert_ctx;

    char buf[4096];

    char buf[4096];

    int i, j = 0, k, ret = 0;

    int i, j = 0, k, ret = 0;

    BIO *p7bio;

    BIO *p7bio = NULL;

    BIO *tmpin, *tmpout;

    BIO *tmpin = NULL, *tmpout = NULL;





    if (!p7) {

    if (!p7) {

        PKCS7err(PKCS7_F_PKCS7_VERIFY, PKCS7_R_INVALID_NULL_POINTER);

        PKCS7err(PKCS7_F_PKCS7_VERIFY, PKCS7_R_INVALID_NULL_POINTER);
@@ -274,18 +274,11 @@ int PKCS7_verify(PKCS7 *p7, STACK_OF(X509) *certs, X509_STORE *store, 
        return 0;

        return 0;

    }

    }





    /*

     * Very old Netscape illegally included empty content with

     * a detached signature. To not support that, enable the

     * following flag.

     */

#ifdef OPENSSL_DONT_SUPPORT_OLD_NETSCAPE

    /* Check for data and content: two sets of data */

    /* Check for data and content: two sets of data */

    if (!PKCS7_get_detached(p7) && indata) {

    if (!PKCS7_get_detached(p7) && indata) {

        PKCS7err(PKCS7_F_PKCS7_VERIFY, PKCS7_R_CONTENT_AND_DATA_PRESENT);

        PKCS7err(PKCS7_F_PKCS7_VERIFY, PKCS7_R_CONTENT_AND_DATA_PRESENT);

        return 0;

        return 0;

    }

    }

#endif





    sinfos = PKCS7_get_signer_info(p7);

    sinfos = PKCS7_get_signer_info(p7);
@@ -295,7 +288,6 @@ int PKCS7_verify(PKCS7 *p7, STACK_OF(X509) *certs, X509_STORE *store, 
    }

    }





    signers = PKCS7_get0_signers(p7, certs, flags);

    signers = PKCS7_get0_signers(p7, certs, flags);



    if (!signers)

    if (!signers)

        return 0;

        return 0;
@@ -308,14 +300,12 @@ int PKCS7_verify(PKCS7 *p7, STACK_OF(X509) *certs, X509_STORE *store, 
                if (!X509_STORE_CTX_init(&cert_ctx, store, signer,

                if (!X509_STORE_CTX_init(&cert_ctx, store, signer,

                                         p7->d.sign->cert)) {

                                         p7->d.sign->cert)) {

                    PKCS7err(PKCS7_F_PKCS7_VERIFY, ERR_R_X509_LIB);

                    PKCS7err(PKCS7_F_PKCS7_VERIFY, ERR_R_X509_LIB);

                    sk_X509_free(signers);

                    goto err;

                    return 0;

                }

                }

                X509_STORE_CTX_set_default(&cert_ctx, "smime_sign");

                X509_STORE_CTX_set_default(&cert_ctx, "smime_sign");

            } else if (!X509_STORE_CTX_init(&cert_ctx, store, signer, NULL)) {

            } else if (!X509_STORE_CTX_init(&cert_ctx, store, signer, NULL)) {

                PKCS7err(PKCS7_F_PKCS7_VERIFY, ERR_R_X509_LIB);

                PKCS7err(PKCS7_F_PKCS7_VERIFY, ERR_R_X509_LIB);

                sk_X509_free(signers);

                goto err;

                return 0;

            }

            }

            if (!(flags & PKCS7_NOCRL))

            if (!(flags & PKCS7_NOCRL))

                X509_STORE_CTX_set0_crls(&cert_ctx, p7->d.sign->crl);

                X509_STORE_CTX_set0_crls(&cert_ctx, p7->d.sign->crl);
@@ -328,8 +318,7 @@ int PKCS7_verify(PKCS7 *p7, STACK_OF(X509) *certs, X509_STORE *store, 
                         PKCS7_R_CERTIFICATE_VERIFY_ERROR);

                         PKCS7_R_CERTIFICATE_VERIFY_ERROR);

                ERR_add_error_data(2, "Verify error:",

                ERR_add_error_data(2, "Verify error:",

                                   X509_verify_cert_error_string(j));

                                   X509_verify_cert_error_string(j));

                sk_X509_free(signers);

                goto err;

                return 0;

            }

            }

            /* Check for revocation status here */

            /* Check for revocation status here */

        }

        }
@@ -348,7 +337,7 @@ int PKCS7_verify(PKCS7 *p7, STACK_OF(X509) *certs, X509_STORE *store, 
        tmpin = BIO_new_mem_buf(ptr, len);

        tmpin = BIO_new_mem_buf(ptr, len);

        if (tmpin == NULL) {

        if (tmpin == NULL) {

            PKCS7err(PKCS7_F_PKCS7_VERIFY, ERR_R_MALLOC_FAILURE);

            PKCS7err(PKCS7_F_PKCS7_VERIFY, ERR_R_MALLOC_FAILURE);

            return 0;

            goto err;

        }

        }

    } else

    } else

        tmpin = indata;

        tmpin = indata;
@@ -398,15 +387,12 @@ int PKCS7_verify(PKCS7 *p7, STACK_OF(X509) *certs, X509_STORE *store, 
    ret = 1;

    ret = 1;





 err:

 err:



    if (tmpin == indata) {

    if (tmpin == indata) {

        if (indata)

        if (indata)

            BIO_pop(p7bio);

            BIO_pop(p7bio);

    }

    }

    BIO_free_all(p7bio);

    BIO_free_all(p7bio);



    sk_X509_free(signers);

    sk_X509_free(signers);



    return ret;

    return ret;

}

}

crypto/x509/x509_vfy.c

+2 −2
Original line number Original line Diff line number Diff line
@@ -243,7 +243,7 @@ int X509_verify_cert(X509_STORE_CTX *ctx) 
        if (ctx->param->flags & X509_V_FLAG_TRUSTED_FIRST) {

        if (ctx->param->flags & X509_V_FLAG_TRUSTED_FIRST) {

            ok = ctx->get_issuer(&xtmp, ctx, x);

            ok = ctx->get_issuer(&xtmp, ctx, x);

            if (ok < 0)

            if (ok < 0)

                return ok;

                goto end;

            /*

            /*

             * If successful for now free up cert so it will be picked up

             * If successful for now free up cert so it will be picked up

             * again later.

             * again later.
@@ -341,7 +341,7 @@ int X509_verify_cert(X509_STORE_CTX *ctx) 
            ok = ctx->get_issuer(&xtmp, ctx, x);

            ok = ctx->get_issuer(&xtmp, ctx, x);





            if (ok < 0)

            if (ok < 0)

                return ok;

                goto end;

            if (ok == 0)

            if (ok == 0)

                break;

                break;

            x = xtmp;

            x = xtmp;

test/clienthellotest.c

+1 −0
Original line number Original line Diff line number Diff line
@@ -213,6 +213,7 @@ int main(int argc, char *argv[]) 
    EVP_cleanup();

    EVP_cleanup();

    CRYPTO_cleanup_all_ex_data();

    CRYPTO_cleanup_all_ex_data();

    CRYPTO_mem_leaks(err);

    CRYPTO_mem_leaks(err);

    BIO_free(err);





    return testresult?0:1;

    return testresult?0:1;

}
 
}

test/gost2814789test.c

+3 −0
Original line number Original line Diff line number Diff line
@@ -1411,6 +1411,7 @@ int main(int argc, char *argv[]) 
            }

            }

            siglen = 4;

            siglen = 4;

            OPENSSL_assert(EVP_DigestSignFinal(&mctx, bTest, &siglen));

            OPENSSL_assert(EVP_DigestSignFinal(&mctx, bTest, &siglen));

            EVP_PKEY_free(mac_key);

            EVP_MD_CTX_cleanup(&mctx);

            EVP_MD_CTX_cleanup(&mctx);

            enlu = (int)tcs[t].ullLen;

            enlu = (int)tcs[t].ullLen;

            enlf = 0;

            enlf = 0;
@@ -1434,6 +1435,8 @@ int main(int argc, char *argv[]) 
    printf(" passed\n");

    printf(" passed\n");

    fflush(NULL);

    fflush(NULL);





    NCONF_free(pConfig);



    return EXIT_SUCCESS;

    return EXIT_SUCCESS;

}

}

#endif
 
#endif