Commit a93e0e78 authored by J Mohan Rao Arisankala's avatar J Mohan Rao Arisankala Committed by Matt Caswell
Browse files

#4342: few missing malloc return checks and free in error paths 



ossl_hmac_cleanup, pkey_hmac_cleanup:
 - allow to invoke with NULL data
 - using EVP_PKEY_CTX_[get|set]_data

EVP_DigestInit_ex:
 - remove additional check for ‘type’ and doing clear free instead of
free

Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
Reviewed-by: default avatarMatt Caswell <matt@openssl.org>
parent 1c7bfec5

crypto/engine/eng_openssl.c

+20 −5
Original line number Diff line number Diff line
@@ -441,6 +441,10 @@ static int ossl_hmac_init(EVP_PKEY_CTX *ctx) 
        return 0;

    hctx->ktmp.type = V_ASN1_OCTET_STRING;

    hctx->ctx = HMAC_CTX_new();

    if (hctx->ctx == NULL) {

        OPENSSL_free(hctx);

        return 0;

    }

    EVP_PKEY_CTX_set_data(ctx, hctx);

    EVP_PKEY_CTX_set0_keygen_info(ctx, NULL, 0);

# ifdef TEST_ENG_OPENSSL_HMAC_INIT
@@ -449,31 +453,42 @@ static int ossl_hmac_init(EVP_PKEY_CTX *ctx) 
    return 1;

}



static void ossl_hmac_cleanup(EVP_PKEY_CTX *ctx);



static int ossl_hmac_copy(EVP_PKEY_CTX *dst, EVP_PKEY_CTX *src)

{

    OSSL_HMAC_PKEY_CTX *sctx, *dctx;



    /* allocate memory for dst->data and a new HMAC_CTX in dst->data->ctx */

    if (!ossl_hmac_init(dst))

        return 0;

    sctx = EVP_PKEY_CTX_get_data(src);

    dctx = EVP_PKEY_CTX_get_data(dst);

    dctx->md = sctx->md;

    if (!HMAC_CTX_copy(dctx->ctx, sctx->ctx))

        return 0;

        goto err;

    if (sctx->ktmp.data) {

        if (!ASN1_OCTET_STRING_set(&dctx->ktmp,

                                   sctx->ktmp.data, sctx->ktmp.length))

            return 0;

            goto err;

    }

    return 1;

err:

    /* release HMAC_CTX in dst->data->ctx and memory allocated for dst->data */

    ossl_hmac_cleanup(dst);

    return 0;

}



static void ossl_hmac_cleanup(EVP_PKEY_CTX *ctx)

{

    OSSL_HMAC_PKEY_CTX *hctx = EVP_PKEY_CTX_get_data(ctx);



    if (hctx) {

        HMAC_CTX_free(hctx->ctx);

        OPENSSL_clear_free(hctx->ktmp.data, hctx->ktmp.length);

        OPENSSL_free(hctx);

        EVP_PKEY_CTX_set_data(ctx, NULL);

    }

}



static int ossl_hmac_keygen(EVP_PKEY_CTX *ctx, EVP_PKEY *pkey)

crypto/evp/digest.c

+3 −5
Original line number Diff line number Diff line
@@ -68,10 +68,8 @@ int EVP_DigestInit_ex(EVP_MD_CTX *ctx, const EVP_MD *type, ENGINE *impl) 
     * previous handle, re-querying for an ENGINE, and having a

     * reinitialisation, when it may all be unnecessary.

     */

    if (ctx->engine && ctx->digest && (!type ||

                                       (type

                                        && (type->type ==

                                            ctx->digest->type))))

    if (ctx->engine && ctx->digest &&

        (type == NULL || (type->type == ctx->digest->type)))

        goto skip_to_init;

    if (type) {

        /*
@@ -117,7 +115,7 @@ int EVP_DigestInit_ex(EVP_MD_CTX *ctx, const EVP_MD *type, ENGINE *impl) 
#endif

    if (ctx->digest != type) {

        if (ctx->digest && ctx->digest->ctx_size) {

            OPENSSL_free(ctx->md_data);

            OPENSSL_clear_free(ctx->md_data, ctx->digest->ctx_size);

            ctx->md_data = NULL;

        }

        ctx->digest = type;

crypto/hmac/hm_pmeth.c

+18 −6
Original line number Diff line number Diff line
@@ -32,6 +32,10 @@ static int pkey_hmac_init(EVP_PKEY_CTX *ctx) 
        return 0;

    hctx->ktmp.type = V_ASN1_OCTET_STRING;

    hctx->ctx = HMAC_CTX_new();

    if (hctx->ctx == NULL) {

        OPENSSL_free(hctx);

        return 0;

    }



    ctx->data = hctx;

    ctx->keygen_info_count = 0;
@@ -39,33 +43,41 @@ static int pkey_hmac_init(EVP_PKEY_CTX *ctx) 
    return 1;

}



static void pkey_hmac_cleanup(EVP_PKEY_CTX *ctx);



static int pkey_hmac_copy(EVP_PKEY_CTX *dst, EVP_PKEY_CTX *src)

{

    HMAC_PKEY_CTX *sctx, *dctx;



    /* allocate memory for dst->data and a new HMAC_CTX in dst->data->ctx */

    if (!pkey_hmac_init(dst))

        return 0;

    sctx = src->data;

    dctx = dst->data;

    sctx = EVP_PKEY_CTX_get_data(src);

    dctx = EVP_PKEY_CTX_get_data(dst);

    dctx->md = sctx->md;

    if (!HMAC_CTX_copy(dctx->ctx, sctx->ctx))

        return 0;

        goto err;

    if (sctx->ktmp.data) {

        if (!ASN1_OCTET_STRING_set(&dctx->ktmp,

                                   sctx->ktmp.data, sctx->ktmp.length))

            return 0;

            goto err;

    }

    return 1;

err:

    /* release HMAC_CTX in dst->data->ctx and memory allocated for dst->data */

    pkey_hmac_cleanup (dst);

    return 0;

}



static void pkey_hmac_cleanup(EVP_PKEY_CTX *ctx)

{

    HMAC_PKEY_CTX *hctx = ctx->data;

    HMAC_PKEY_CTX *hctx = EVP_PKEY_CTX_get_data(ctx);



    if (hctx != NULL) {

        HMAC_CTX_free(hctx->ctx);

        OPENSSL_clear_free(hctx->ktmp.data, hctx->ktmp.length);

        OPENSSL_free(hctx);

        ctx->data = NULL;

        EVP_PKEY_CTX_set_data(ctx, NULL);

    }

}