From bec794fbee14fb726cf1a4e572b261ed96f3bab7 Mon Sep 17 00:00:00 2001 From: Frank Morgner Date: Sat, 16 Nov 2019 22:37:39 +0100 Subject: [PATCH] fixed memory leak https://crbug.com/oss-fuzz/18953 --- src/libopensc/pkcs15-prkey.c | 52 +++++++++++++++++++++--------------- 1 file changed, 31 insertions(+), 21 deletions(-) diff --git a/src/libopensc/pkcs15-prkey.c b/src/libopensc/pkcs15-prkey.c index 5127565d..761cefd6 100644 --- a/src/libopensc/pkcs15-prkey.c +++ b/src/libopensc/pkcs15-prkey.c @@ -258,13 +258,9 @@ int sc_pkcs15_decode_prkdf_entry(struct sc_pkcs15_card *p15card, memset(gostr3410_params, 0, sizeof(gostr3410_params)); r = sc_asn1_decode_choice(ctx, asn1_prkey, *buf, *buflen, buf, buflen); - if (r < 0) { - /* This might have allocated something. If so, clear it now */ - free(info.subject.value); - } if (r == SC_ERROR_ASN1_END_OF_CONTENTS) - return r; - LOG_TEST_RET(ctx, r, "PrKey DF ASN.1 decoding failed"); + goto err; + LOG_TEST_GOTO_ERR(ctx, r, "PrKey DF ASN.1 decoding failed"); if (asn1_prkey[0].flags & SC_ASN1_PRESENT) { obj->type = SC_PKCS15_TYPE_PRKEY_RSA; } @@ -278,34 +274,39 @@ int sc_pkcs15_decode_prkdf_entry(struct sc_pkcs15_card *p15card, info.path.type = SC_PATH_TYPE_PATH_PROT; } else if (asn1_prkey[3].flags & SC_ASN1_PRESENT) { + /* FIXME proper handling of gost parameters without the need of + * allocating data here. this would also make sc_pkcs15_free_key_params + * obsolete */ obj->type = SC_PKCS15_TYPE_PRKEY_GOSTR3410; - assert(info.modulus_length == 0); + if (info.modulus_length != 0 || info.params.len != 0) { + r = SC_ERROR_INVALID_ASN1_OBJECT; + goto err; + } info.modulus_length = SC_PKCS15_GOSTR3410_KEYSIZE; - assert(info.params.len == 0); info.params.len = sizeof(struct sc_pkcs15_keyinfo_gostparams); info.params.data = malloc(info.params.len); - if (info.params.data == NULL) - LOG_FUNC_RETURN(ctx, SC_ERROR_OUT_OF_MEMORY); - assert(sizeof(*keyinfo_gostparams) == info.params.len); + if (info.params.data == NULL) { + r = SC_ERROR_OUT_OF_MEMORY; + goto err; + } keyinfo_gostparams = info.params.data; keyinfo_gostparams->gostr3410 = gostr3410_params[0]; keyinfo_gostparams->gostr3411 = gostr3410_params[1]; keyinfo_gostparams->gost28147 = gostr3410_params[2]; } else { - sc_log(ctx, "Neither RSA or DSA or GOSTR3410 or ECC key in PrKDF entry."); - LOG_FUNC_RETURN(ctx, SC_ERROR_INVALID_ASN1_OBJECT); + r = SC_ERROR_INVALID_ASN1_OBJECT; + LOG_TEST_GOTO_ERR(ctx, r, "Neither RSA or DSA or GOSTR3410 or ECC key in PrKDF entry."); } if (!p15card->app || !p15card->app->ddo.aid.len) { if (!p15card->file_app) { - sc_pkcs15_free_key_params(&info.params); - return SC_ERROR_INTERNAL; + r = SC_ERROR_INTERNAL; + goto err; } r = sc_pkcs15_make_absolute_path(&p15card->file_app->path, &info.path); if (r < 0) { - sc_pkcs15_free_key_params(&info.params); - return r; + goto err; } } else { @@ -348,14 +349,24 @@ int sc_pkcs15_decode_prkdf_entry(struct sc_pkcs15_card *p15card, obj->data = malloc(sizeof(info)); if (obj->data == NULL) { - sc_pkcs15_free_key_params(&info.params); - LOG_FUNC_RETURN(ctx, SC_ERROR_OUT_OF_MEMORY); + r = SC_ERROR_OUT_OF_MEMORY; + goto err; } memcpy(obj->data, &info, sizeof(info)); sc_log(ctx, "Key Subject %s", sc_dump_hex(info.subject.value, info.subject.len)); sc_log(ctx, "Key path %s", sc_print_path(&info.path)); - return 0; + + r = SC_SUCCESS; + +err: + if (r < 0) { + /* This might have allocated something. If so, clear it now */ + free(info.subject.value); + sc_pkcs15_free_key_params(&info.params); + } + + return r; } int sc_pkcs15_encode_prkdf_entry(sc_context_t *ctx, const struct sc_pkcs15_object *obj, @@ -588,7 +599,6 @@ sc_pkcs15_free_prkey(struct sc_pkcs15_prkey *key) free(key->u.dsa.priv.data); break; case SC_ALGORITHM_GOSTR3410: - assert(key->u.gostr3410.d.data); free(key->u.gostr3410.d.data); break; case SC_ALGORITHM_EC: