Commit 9f084451 authored by FdaSilvaYY's avatar FdaSilvaYY Committed by Pauli
Browse files

OCSP: fix memory leak in OCSP_url_svcloc_new method. 


Add a few coverage test case.

Fixes #8949

[extended tests]

Reviewed-by: default avatarMatt Caswell <matt@openssl.org>
Reviewed-by: default avatarPaul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/8959)

(cherry picked from commit 5b3accde606ffe01466426bd59407ffca0690d23)
parent 22598dab

crypto/ocsp/ocsp_ext.c

+2 −2
Original line number Diff line number Diff line
@@ -439,6 +439,7 @@ X509_EXTENSION *OCSP_url_svcloc_new(X509_NAME *issuer, const char **urls) 


    if ((sloc = OCSP_SERVICELOC_new()) == NULL)

        goto err;

    X509_NAME_free(sloc->issuer);

    if ((sloc->issuer = X509_NAME_dup(issuer)) == NULL)

        goto err;

    if (urls && *urls
@@ -449,12 +450,11 @@ X509_EXTENSION *OCSP_url_svcloc_new(X509_NAME *issuer, const char **urls) 
            goto err;

        if ((ad->method = OBJ_nid2obj(NID_ad_OCSP)) == NULL)

            goto err;

        if ((ad->location = GENERAL_NAME_new()) == NULL)

            goto err;

        if ((ia5 = ASN1_IA5STRING_new()) == NULL)

            goto err;

        if (!ASN1_STRING_set((ASN1_STRING *)ia5, *urls, -1))

            goto err;

        /* ad->location is allocated inside ACCESS_DESCRIPTION_new */

        ad->location->type = GEN_URI;

        ad->location->d.ia5 = ia5;

        ia5 = NULL;

test/ocspapitest.c

+81 −1
Original line number Diff line number Diff line
@@ -47,6 +47,24 @@ static int get_cert_and_key(X509 **cert_out, EVP_PKEY **key_out) 
    return 0;

}



static int get_cert(X509 **cert_out)

{

    BIO *certbio;

    X509 *cert = NULL;



    if (!TEST_ptr(certbio = BIO_new_file(certstr, "r")))

        return 0;

    cert = PEM_read_bio_X509(certbio, NULL, NULL, NULL);

    BIO_free(certbio);

    if (!TEST_ptr(cert))

        goto end;

    *cert_out = cert;

    return 1;

 end:

    X509_free(cert);

    return 0;

}



static OCSP_BASICRESP *make_dummy_resp(void)

{

    const unsigned char namestr[] = "openssl.example.com";
@@ -131,7 +149,67 @@ static int test_resp_signer(void) 
    EVP_PKEY_free(key);

    return ret;

}

#endif



static int test_access_description(int testcase)

{

    ACCESS_DESCRIPTION *ad = ACCESS_DESCRIPTION_new();

    int ret = 0;



    if (!TEST_ptr(ad))

        goto err;



    switch (testcase) {

    case 0:     /* no change */

        break;

    case 1:     /* check and release current location */

        if (!TEST_ptr(ad->location))

            goto err;

        GENERAL_NAME_free(ad->location);

        ad->location = NULL;

        break;

    case 2:     /* replace current location */

        GENERAL_NAME_free(ad->location);

        ad->location = GENERAL_NAME_new();

        if (!TEST_ptr(ad->location))

            goto err;

        break;

    }

    ACCESS_DESCRIPTION_free(ad);

    ret = 1;

err:

    return ret;

}



static int test_ocsp_url_svcloc_new(void)

{

    static const char *  urls[] = {

        "www.openssl.org",

        "www.openssl.net",

        NULL

    };



    X509 *issuer = NULL;

    X509_EXTENSION * ext = NULL;

    int ret = 0;



    if (!TEST_true(get_cert(&issuer)))

        goto err;



    /*

     * Test calling this ocsp method to catch any memory leak

     */

    ext = OCSP_url_svcloc_new(X509_get_issuer_name(issuer), urls);

    if (!TEST_ptr(ext))

        goto err;



    X509_EXTENSION_free(ext);

    ret = 1;

err:

    X509_free(issuer);

    return ret;

}



#endif /* OPENSSL_NO_OCSP */



int setup_tests(void)

{
@@ -140,6 +218,8 @@ int setup_tests(void) 
        return 0;

#ifndef OPENSSL_NO_OCSP

    ADD_TEST(test_resp_signer);

    ADD_ALL_TESTS(test_access_description, 3);

    ADD_TEST(test_ocsp_url_svcloc_new);

#endif

    return 1;

}