From 7a34c204c1904b789a1842003071b4da3d7c0932 Mon Sep 17 00:00:00 2001 From: Frank Morgner Date: Thu, 22 Jan 2015 20:29:33 +0100 Subject: [PATCH 01/30] fixed dereference before null check silence warnings reported by coverity-scan --- src/common/compat_getopt_main.c | 2 +- src/libopensc/card-authentic.c | 2 +- src/libopensc/card-iasecc.c | 18 +++++++---- src/libopensc/card-oberthur.c | 7 +++-- src/libopensc/card-westcos.c | 17 ++++++----- src/libopensc/card.c | 11 ++++--- src/libopensc/cwa14890.c | 8 ++--- src/libopensc/sc.c | 21 ++++++------- src/pkcs11/framework-pkcs15.c | 10 ++++--- src/pkcs11/pkcs11-global.c | 2 +- src/pkcs15init/pkcs15-entersafe.c | 7 ++--- src/pkcs15init/pkcs15-epass2003.c | 3 +- src/pkcs15init/pkcs15-gpk.c | 3 +- src/pkcs15init/pkcs15-jcop.c | 13 ++++---- src/pkcs15init/pkcs15-lib.c | 45 ++++++++++++++-------------- src/pkcs15init/pkcs15-myeid.c | 5 ++-- src/pkcs15init/pkcs15-oberthur-awp.c | 11 +++---- src/pkcs15init/pkcs15-oberthur.c | 6 ++-- src/pkcs15init/pkcs15-setcos.c | 3 +- src/pkcs15init/profile.c | 2 +- src/scconf/parse.c | 37 +++++++++++++---------- src/scconf/sclex.c | 6 ++-- src/tests/base64.c | 3 +- src/tools/dnie-tool.c | 2 +- src/tools/piv-tool.c | 2 +- src/tools/pkcs15-init.c | 3 +- src/tools/pkcs15-tool.c | 26 ++++++++-------- src/tools/westcos-tool.c | 10 +++---- 28 files changed, 150 insertions(+), 135 deletions(-) diff --git a/src/common/compat_getopt_main.c b/src/common/compat_getopt_main.c index b5215118..827ba019 100644 --- a/src/common/compat_getopt_main.c +++ b/src/common/compat_getopt_main.c @@ -234,7 +234,7 @@ main(int argc, char * argv[]) case 'o': /* -output=FILE */ outfilename = optarg; /* we allow "-" as a synonym for stdout here */ - if (! strcmp(optarg, "-")) + if (optarg && !strcmp(optarg, "-")) { outfilename = 0; } diff --git a/src/libopensc/card-authentic.c b/src/libopensc/card-authentic.c index 744c534b..737574cd 100644 --- a/src/libopensc/card-authentic.c +++ b/src/libopensc/card-authentic.c @@ -650,7 +650,7 @@ authentic_reduce_path(struct sc_card *card, struct sc_path *path) LOG_FUNC_CALLED(ctx); - if (path->len <= 2 || path->type == SC_PATH_TYPE_DF_NAME || !path) + if (!path || path->len <= 2 || path->type == SC_PATH_TYPE_DF_NAME) LOG_FUNC_RETURN(ctx, SC_SUCCESS); if (!card->cache.valid || !card->cache.current_df) diff --git a/src/libopensc/card-iasecc.c b/src/libopensc/card-iasecc.c index 6e26c87d..318ff548 100644 --- a/src/libopensc/card-iasecc.c +++ b/src/libopensc/card-iasecc.c @@ -2565,7 +2565,8 @@ iasecc_sdo_create(struct sc_card *card, struct iasecc_sdo *sdo) rv = iasecc_sdo_put_data(card, &update); LOG_TEST_RET(ctx, rv, "failed to update 'Compulsory usage' data"); - field->on_card = 1; + if (field) + field->on_card = 1; } free(data); @@ -3252,14 +3253,19 @@ static int iasecc_compute_signature(struct sc_card *card, const unsigned char *in, size_t in_len, unsigned char *out, size_t out_len) { - struct sc_context *ctx = card->ctx; - struct iasecc_private_data *prv = (struct iasecc_private_data *) card->drv_data; - struct sc_security_env *env = &prv->security_env; + struct sc_context *ctx; + struct iasecc_private_data *prv; + struct sc_security_env *env; + + if (!card || !in || !out) + return SC_ERROR_INVALID_ARGUMENTS; + + ctx = card->ctx; + prv = (struct iasecc_private_data *) card->drv_data; + env = &prv->security_env; LOG_FUNC_CALLED(ctx); sc_log(ctx, "inlen %i, outlen %i", in_len, out_len); - if (!card || !in || !out) - LOG_TEST_RET(ctx, SC_ERROR_INVALID_ARGUMENTS, "Invalid compute signature arguments"); if (env->operation == SC_SEC_OPERATION_SIGN) return iasecc_compute_signature_dst(card, in, in_len, out, out_len); diff --git a/src/libopensc/card-oberthur.c b/src/libopensc/card-oberthur.c index 2a9dbe3f..eb0820b1 100644 --- a/src/libopensc/card-oberthur.c +++ b/src/libopensc/card-oberthur.c @@ -1108,16 +1108,17 @@ auth_compute_signature(struct sc_card *card, const unsigned char *in, size_t ile unsigned char resp[SC_MAX_APDU_BUFFER_SIZE]; int rv; - SC_FUNC_CALLED(card->ctx, SC_LOG_DEBUG_VERBOSE); - sc_debug(card->ctx, SC_LOG_DEBUG_NORMAL, "inlen %i, outlen %i\n", ilen, olen); if (!card || !in || !out) { - SC_FUNC_RETURN(card->ctx, SC_LOG_DEBUG_NORMAL, SC_ERROR_INVALID_ARGUMENTS); + return SC_ERROR_INVALID_ARGUMENTS; } else if (ilen > 96) { sc_debug(card->ctx, SC_LOG_DEBUG_NORMAL, "Illegal input length %d\n", ilen); SC_TEST_RET(card->ctx, SC_LOG_DEBUG_NORMAL, SC_ERROR_INVALID_ARGUMENTS, "Illegal input length"); } + SC_FUNC_CALLED(card->ctx, SC_LOG_DEBUG_VERBOSE); + sc_debug(card->ctx, SC_LOG_DEBUG_NORMAL, "inlen %i, outlen %i\n", ilen, olen); + sc_format_apdu(card, &apdu, SC_APDU_CASE_4_SHORT, 0x2A, 0x9E, 0x9A); apdu.datalen = ilen; apdu.data = in; diff --git a/src/libopensc/card-westcos.c b/src/libopensc/card-westcos.c index 1642eb7d..60bcb999 100644 --- a/src/libopensc/card-westcos.c +++ b/src/libopensc/card-westcos.c @@ -1109,6 +1109,15 @@ static int westcos_sign_decipher(int mode, sc_card_t *card, return SC_ERROR_INVALID_ARGUMENTS; sc_debug(card->ctx, SC_LOG_DEBUG_NORMAL, "westcos_sign_decipher outlen=%d\n", outlen); + +#ifndef ENABLE_OPENSSL + r = SC_ERROR_NOT_SUPPORTED; +#else + if (keyfile == NULL || mem == NULL || card->drv_data == NULL) { + r = SC_ERROR_OUT_OF_MEMORY; + goto out; + } + priv_data = (priv_data_t *) card->drv_data; if(priv_data->flags & RSA_CRYPTO_COMPONENT) @@ -1134,14 +1143,6 @@ static int westcos_sign_decipher(int mode, sc_card_t *card, r = apdu.resplen; goto out2; } - -#ifndef ENABLE_OPENSSL - r = SC_ERROR_NOT_SUPPORTED; -#else - if (keyfile == NULL || mem == NULL || priv_data == NULL) { - r = SC_ERROR_OUT_OF_MEMORY; - goto out; - } if ((priv_data->env.flags) & SC_ALGORITHM_RSA_PAD_PKCS1) pad = RSA_PKCS1_PADDING; diff --git a/src/libopensc/card.c b/src/libopensc/card.c index 9a331728..5142d0c0 100644 --- a/src/libopensc/card.c +++ b/src/libopensc/card.c @@ -403,10 +403,11 @@ int sc_create_file(sc_card_t *card, sc_file_t *file) { int r; char pbuf[SC_MAX_PATH_STRING_SIZE]; - const sc_path_t *in_path = &file->path; + const sc_path_t *in_path; - assert(card != NULL); + assert(card != NULL && file != NULL); + in_path = &file->path; r = sc_path_print(pbuf, sizeof(pbuf), in_path); if (r != SC_SUCCESS) pbuf[0] = '\0'; @@ -864,14 +865,16 @@ sc_algorithm_info_t * sc_card_find_gostr3410_alg(sc_card_t *card, static int match_atr_table(sc_context_t *ctx, struct sc_atr_table *table, struct sc_atr *atr) { - u8 *card_atr_bin = atr->value; - size_t card_atr_bin_len = atr->len; + u8 *card_atr_bin; + size_t card_atr_bin_len; char card_atr_hex[3 * SC_MAX_ATR_SIZE]; size_t card_atr_hex_len; unsigned int i = 0; if (ctx == NULL || table == NULL || atr == NULL) return -1; + card_atr_bin = atr->value; + card_atr_bin_len = atr->len; sc_bin_to_hex(card_atr_bin, card_atr_bin_len, card_atr_hex, sizeof(card_atr_hex), ':'); card_atr_hex_len = strlen(card_atr_hex); diff --git a/src/libopensc/cwa14890.c b/src/libopensc/cwa14890.c index dbbd599b..c9515275 100644 --- a/src/libopensc/cwa14890.c +++ b/src/libopensc/cwa14890.c @@ -1434,10 +1434,6 @@ int cwa_encode_apdu(sc_card_t * card, u8 *msgbuf = NULL; /* to encrypt apdu data */ u8 *cryptbuf = NULL; - /* reserve extra bytes for padding and tlv header */ - msgbuf = calloc(12 + from->lc, sizeof(u8)); /* to encrypt apdu data */ - cryptbuf = calloc(12 + from->lc, sizeof(u8)); - /* mandatory check */ if (!card || !card->ctx || !provider) return SC_ERROR_INVALID_ARGUMENTS; @@ -1450,6 +1446,10 @@ int cwa_encode_apdu(sc_card_t * card, LOG_FUNC_RETURN(ctx, SC_ERROR_SM_NOT_INITIALIZED); if (sm_session->state != CWA_SM_ACTIVE) LOG_FUNC_RETURN(ctx, SC_ERROR_SM_INVALID_LEVEL); + + /* reserve extra bytes for padding and tlv header */ + msgbuf = calloc(12 + from->lc, sizeof(u8)); /* to encrypt apdu data */ + cryptbuf = calloc(12 + from->lc, sizeof(u8)); if (!msgbuf || !cryptbuf) LOG_FUNC_RETURN(ctx, SC_ERROR_OUT_OF_MEMORY); diff --git a/src/libopensc/sc.c b/src/libopensc/sc.c index a2be9b34..08375f0c 100644 --- a/src/libopensc/sc.c +++ b/src/libopensc/sc.c @@ -274,17 +274,18 @@ void sc_format_path(const char *str, sc_path_t *path) { int type = SC_PATH_TYPE_PATH; - memset(path, 0, sizeof(*path)); - if (*str == 'i' || *str == 'I') { - type = SC_PATH_TYPE_FILE_ID; - str++; + if (path) { + memset(path, 0, sizeof(*path)); + if (*str == 'i' || *str == 'I') { + type = SC_PATH_TYPE_FILE_ID; + str++; + } + path->len = sizeof(path->value); + if (sc_hex_to_bin(str, path->value, &path->len) >= 0) { + path->type = type; + } + path->count = -1; } - path->len = sizeof(path->value); - if (sc_hex_to_bin(str, path->value, &path->len) >= 0) { - path->type = type; - } - path->count = -1; - return; } int sc_append_path(sc_path_t *dest, const sc_path_t *src) diff --git a/src/pkcs11/framework-pkcs15.c b/src/pkcs11/framework-pkcs15.c index c30167e5..2bf63774 100644 --- a/src/pkcs11/framework-pkcs15.c +++ b/src/pkcs11/framework-pkcs15.c @@ -3446,10 +3446,12 @@ pkcs15_prkey_get_attribute(struct sc_pkcs11_session *session, check_attribute_buffer(attr, sizeof(CK_ULONG)); switch (prkey->prv_p15obj->type) { case SC_PKCS15_TYPE_PRKEY_EC: - if (key && key->u.ec.params.field_length > 0) - *(CK_ULONG *) attr->pValue = key->u.ec.params.field_length; - else - *(CK_ULONG *) attr->pValue = (key->u.ec.ecpointQ.len - 1) / 2 *8; + if (key) { + if (key->u.ec.params.field_length > 0) + *(CK_ULONG *) attr->pValue = key->u.ec.params.field_length; + else + *(CK_ULONG *) attr->pValue = (key->u.ec.ecpointQ.len - 1) / 2 *8; + } return CKR_OK; default: *(CK_ULONG *) attr->pValue = prkey->prv_info->modulus_length; diff --git a/src/pkcs11/pkcs11-global.c b/src/pkcs11/pkcs11-global.c index 64abd1a7..aa507589 100644 --- a/src/pkcs11/pkcs11-global.c +++ b/src/pkcs11/pkcs11-global.c @@ -688,7 +688,7 @@ out: sc_wait_for_event(context, 0, NULL, NULL, -1, &reader_states); } - sc_log(context, "C_WaitForSlotEvent() = %s, event in 0x%lx", lookup_enum (RV_T, rv), *pSlot); + sc_log(context, "C_WaitForSlotEvent() = %s", lookup_enum (RV_T, rv)); sc_pkcs11_unlock(); return rv; } diff --git a/src/pkcs15init/pkcs15-entersafe.c b/src/pkcs15init/pkcs15-entersafe.c index 80c66698..12611179 100644 --- a/src/pkcs15init/pkcs15-entersafe.c +++ b/src/pkcs15init/pkcs15-entersafe.c @@ -292,10 +292,9 @@ static int entersafe_create_pin(sc_profile_t *profile, sc_pkcs15_card_t *p15card data.key_data.symmetric.key_len=16; r = sc_card_ctl(card, SC_CARDCTL_ENTERSAFE_WRITE_KEY, &data); - if (pin_obj) { - /* Cache new PIN value. */ - sc_pkcs15_pincache_add(p15card, pin_obj, pin, pin_len); - } + + /* Cache new PIN value. */ + sc_pkcs15_pincache_add(p15card, pin_obj, pin, pin_len); } {/*puk*/ diff --git a/src/pkcs15init/pkcs15-epass2003.c b/src/pkcs15init/pkcs15-epass2003.c index a457ec93..d35cad63 100644 --- a/src/pkcs15init/pkcs15-epass2003.c +++ b/src/pkcs15init/pkcs15-epass2003.c @@ -470,8 +470,7 @@ static int epass2003_pkcs15_store_key(struct sc_profile *profile, SC_TEST_RET(card->ctx, SC_LOG_DEBUG_NORMAL, r, "store key: cannot update private key"); - if (file) - sc_file_free(file); + sc_file_free(file); SC_FUNC_RETURN(card->ctx, SC_LOG_DEBUG_VERBOSE, r); } diff --git a/src/pkcs15init/pkcs15-gpk.c b/src/pkcs15init/pkcs15-gpk.c index fdcf1871..e6502d96 100644 --- a/src/pkcs15init/pkcs15-gpk.c +++ b/src/pkcs15init/pkcs15-gpk.c @@ -458,8 +458,7 @@ gpk_create_key(sc_profile_t *profile, sc_pkcs15_card_t *p15card, sc_pkcs15_objec #endif done: - if (keyfile) - sc_file_free(keyfile); + sc_file_free(keyfile); return r; } diff --git a/src/pkcs15init/pkcs15-jcop.c b/src/pkcs15init/pkcs15-jcop.c index b20e09f2..31854529 100644 --- a/src/pkcs15init/pkcs15-jcop.c +++ b/src/pkcs15init/pkcs15-jcop.c @@ -140,10 +140,10 @@ jcop_create_key(sc_profile_t *profile, sc_pkcs15_card_t *p15card, sc_pkcs15_obje size_t bytes, mod_len, prv_len; int r; - if (obj->type != SC_PKCS15_TYPE_PRKEY_RSA) { - sc_debug(p15card->card->ctx, SC_LOG_DEBUG_NORMAL, "JCOP supports only RSA keys."); - return SC_ERROR_NOT_SUPPORTED; - } + if (obj->type != SC_PKCS15_TYPE_PRKEY_RSA) { + sc_debug(p15card->card->ctx, SC_LOG_DEBUG_NORMAL, "JCOP supports only RSA keys."); + return SC_ERROR_NOT_SUPPORTED; + } /* The caller is supposed to have chosen a key file path for us */ if (key_info->path.len == 0 || key_info->modulus_length == 0) return SC_ERROR_INVALID_ARGUMENTS; @@ -155,7 +155,7 @@ jcop_create_key(sc_profile_t *profile, sc_pkcs15_card_t *p15card, sc_pkcs15_obje mod_len = key_info->modulus_length / 8; bytes = mod_len / 2; - prv_len = 2 + 5 * bytes; + prv_len = 2 + 5 * bytes; keyfile->size = prv_len; /* Fix up PIN references in file ACL */ @@ -164,8 +164,7 @@ jcop_create_key(sc_profile_t *profile, sc_pkcs15_card_t *p15card, sc_pkcs15_obje if (r >= 0) r = sc_pkcs15init_create_file(profile, p15card, keyfile); - if (keyfile) - sc_file_free(keyfile); + sc_file_free(keyfile); return r; } diff --git a/src/pkcs15init/pkcs15-lib.c b/src/pkcs15init/pkcs15-lib.c index 913450a2..02422c73 100644 --- a/src/pkcs15init/pkcs15-lib.c +++ b/src/pkcs15init/pkcs15-lib.c @@ -1888,8 +1888,7 @@ sc_pkcs15init_store_data(struct sc_pkcs15_card *p15card, struct sc_profile *prof *path = file->path; - if (file) - sc_file_free(file); + sc_file_free(file); LOG_FUNC_RETURN(ctx, r); } @@ -2197,7 +2196,7 @@ sc_pkcs15init_select_intrinsic_id(struct sc_pkcs15_card *p15card, struct sc_prof { struct sc_context *ctx = p15card->card->ctx; struct sc_pkcs15_pubkey *pubkey = NULL; - unsigned id_style = profile->id_style; + unsigned id_style; struct sc_pkcs15_id id; unsigned char *id_data = NULL; size_t id_data_len = 0; @@ -2207,15 +2206,17 @@ sc_pkcs15init_select_intrinsic_id(struct sc_pkcs15_card *p15card, struct sc_prof #ifndef ENABLE_OPENSSL LOG_FUNC_RETURN(ctx, SC_SUCCESS); #else - if (!id_out) + if (!id_out || !profile) LOG_FUNC_RETURN(ctx, SC_ERROR_INVALID_ARGUMENTS); + id_style = profile->id_style; + /* ID already exists */ if (id_out->len) LOG_FUNC_RETURN(ctx, SC_SUCCESS); /* Native ID style is not intrisic one */ - if (profile->id_style == SC_PKCS15INIT_ID_STYLE_NATIVE) + if (id_style == SC_PKCS15INIT_ID_STYLE_NATIVE) LOG_FUNC_RETURN(ctx, SC_SUCCESS); memset(&id, 0, sizeof(id)); @@ -2283,7 +2284,7 @@ sc_pkcs15init_select_intrinsic_id(struct sc_pkcs15_card *p15card, struct sc_prof break; default: - sc_log(ctx, "Unsupported ID style: %i", profile->id_style); + sc_log(ctx, "Unsupported ID style: %i", id_style); LOG_TEST_RET(ctx, SC_ERROR_NOT_SUPPORTED, "Non supported ID style"); } @@ -3238,25 +3239,23 @@ sc_pkcs15init_verify_secret(struct sc_profile *profile, struct sc_pkcs15_card *p sc_log(ctx, "Symbolic PIN resolved to PIN(type:CHV,reference:%i)", type, reference); } - if (p15card) { - if (path && path->len) { - struct sc_path tmp_path = *path; - int iter; + if (path && path->len) { + struct sc_path tmp_path = *path; + int iter; - r = SC_ERROR_OBJECT_NOT_FOUND; - for (iter = tmp_path.len/2; iter >= 0 && r == SC_ERROR_OBJECT_NOT_FOUND; iter--, tmp_path.len -= 2) - r = sc_pkcs15_find_pin_by_type_and_reference(p15card, - tmp_path.len ? &tmp_path : NULL, - type, reference, &pin_obj); - } - else { - r = sc_pkcs15_find_pin_by_type_and_reference(p15card, NULL, type, reference, &pin_obj); - } + r = SC_ERROR_OBJECT_NOT_FOUND; + for (iter = tmp_path.len/2; iter >= 0 && r == SC_ERROR_OBJECT_NOT_FOUND; iter--, tmp_path.len -= 2) + r = sc_pkcs15_find_pin_by_type_and_reference(p15card, + tmp_path.len ? &tmp_path : NULL, + type, reference, &pin_obj); + } + else { + r = sc_pkcs15_find_pin_by_type_and_reference(p15card, NULL, type, reference, &pin_obj); + } - if (!r && pin_obj) { - memcpy(&auth_info, pin_obj->data, sizeof(auth_info)); - sc_log(ctx, "found PIN object '%s'", pin_obj->label); - } + if (!r && pin_obj) { + memcpy(&auth_info, pin_obj->data, sizeof(auth_info)); + sc_log(ctx, "found PIN object '%s'", pin_obj->label); } if (pin_obj) { diff --git a/src/pkcs15init/pkcs15-myeid.c b/src/pkcs15init/pkcs15-myeid.c index 4ade6974..1da4515a 100644 --- a/src/pkcs15init/pkcs15-myeid.c +++ b/src/pkcs15init/pkcs15-myeid.c @@ -189,7 +189,7 @@ myeid_init_card(sc_profile_t *profile, */ static int myeid_create_dir(sc_profile_t *profile, sc_pkcs15_card_t *p15card, sc_file_t *df) { - struct sc_context *ctx = p15card->card->ctx; + struct sc_context *ctx; struct sc_file *file = NULL; int r = 0, ii; static const char *create_dfs[] = { @@ -209,9 +209,10 @@ myeid_create_dir(sc_profile_t *profile, sc_pkcs15_card_t *p15card, sc_file_t *df SC_PKCS15_DODF }; - if (!profile || !p15card || !df) + if (!profile || !p15card || !p15card->card || !df) return SC_ERROR_INVALID_ARGUMENTS; + ctx = p15card->card->ctx; LOG_FUNC_CALLED(ctx); sc_log(ctx, "id (%x)", df->id); diff --git a/src/pkcs15init/pkcs15-oberthur-awp.c b/src/pkcs15init/pkcs15-oberthur-awp.c index 18032fba..aa4e20a1 100644 --- a/src/pkcs15init/pkcs15-oberthur-awp.c +++ b/src/pkcs15init/pkcs15-oberthur-awp.c @@ -330,12 +330,10 @@ awp_create_container(struct sc_pkcs15_card *p15card, struct sc_profile *profile, rv = awp_create_container_record(p15card, profile, file, acc); - if (clist) - sc_file_free(clist); - if (file) - sc_file_free(file); if (list) free(list); + sc_file_free(file); + sc_file_free(clist); SC_FUNC_RETURN(ctx, SC_LOG_DEBUG_NORMAL, rv); } @@ -1669,8 +1667,8 @@ awp_delete_from_container(struct sc_pkcs15_card *p15card, rv = 0; if (buff) free(buff); - if (file) sc_file_free(file); if (clist) sc_file_free(clist); + sc_file_free(file); SC_FUNC_RETURN(ctx, SC_LOG_DEBUG_NORMAL, rv); } @@ -1744,8 +1742,7 @@ awp_remove_from_object_list( struct sc_pkcs15_card *p15card, struct sc_profile * done: if (buff) free(buff); - if (lst) - sc_file_free(lst); + sc_file_free(lst); if (lst_file) sc_file_free(lst_file); diff --git a/src/pkcs15init/pkcs15-oberthur.c b/src/pkcs15init/pkcs15-oberthur.c index 34185987..15661e73 100644 --- a/src/pkcs15init/pkcs15-oberthur.c +++ b/src/pkcs15init/pkcs15-oberthur.c @@ -63,15 +63,17 @@ static int cosm_write_tokeninfo (struct sc_pkcs15_card *p15card, struct sc_profile *profile, char *label, unsigned flags) { - struct sc_context *ctx = p15card->card->ctx; + struct sc_context *ctx; struct sc_file *file = NULL; int rv; size_t sz; char *buffer = NULL; - if (!p15card || !profile) + if (!p15card || !p15card->card || !profile) return SC_ERROR_INVALID_ARGUMENTS; + ctx = p15card->card->ctx; + SC_FUNC_CALLED(ctx, SC_LOG_DEBUG_VERBOSE); sc_debug(ctx, SC_LOG_DEBUG_NORMAL, "cosm_write_tokeninfo() label '%s'; flags 0x%X", label, flags); if (sc_profile_get_file(profile, COSM_TITLE"-token-info", &file)) diff --git a/src/pkcs15init/pkcs15-setcos.c b/src/pkcs15init/pkcs15-setcos.c index fdd185ce..d2e85f17 100644 --- a/src/pkcs15init/pkcs15-setcos.c +++ b/src/pkcs15init/pkcs15-setcos.c @@ -236,8 +236,7 @@ setcos_create_pin(sc_profile_t *profile, sc_pkcs15_card_t *p15card, SC_TEST_RET(ctx, SC_LOG_DEBUG_NORMAL, r, "Cannot set MF into the activated state"); } - if(pinfile) - sc_file_free(pinfile); + sc_file_free(pinfile); SC_FUNC_RETURN(ctx, SC_LOG_DEBUG_NORMAL, r); } diff --git a/src/pkcs15init/profile.c b/src/pkcs15init/profile.c index 1c560f89..433c7ae4 100644 --- a/src/pkcs15init/profile.c +++ b/src/pkcs15init/profile.c @@ -2059,7 +2059,7 @@ sc_profile_find_file_by_path(struct sc_profile *pro, const sc_path_t *path) sc_log(ctx, "find profile file by path:%s", sc_print_path(path)); #endif - if (!path->len && !path->aid.len) + if (!path || (!path->len && !path->aid.len)) return NULL; for (fi = pro->ef_list; fi; fi = fi->next) { diff --git a/src/scconf/parse.c b/src/scconf/parse.c index 2a9aed3f..043ab794 100644 --- a/src/scconf/parse.c +++ b/src/scconf/parse.c @@ -82,8 +82,9 @@ static scconf_item *scconf_item_find(scconf_parser * parser) scconf_item *item; for (item = parser->block->items; item; item = item->next) { - if (item->type == SCCONF_ITEM_TYPE_VALUE && - strcasecmp(item->key, parser->key) == 0) { + if (item && item->type == SCCONF_ITEM_TYPE_VALUE + && item->key && parser->key + && strcasecmp(item->key, parser->key) == 0) { return item; } } @@ -148,20 +149,24 @@ scconf_item *scconf_item_add(scconf_context * config, scconf_block * block, scco scconf_list_copy(dst->name, &parser.name); } scconf_item_add_internal(&parser, type); - switch (parser.current_item->type) { - case SCCONF_ITEM_TYPE_COMMENT: - parser.current_item->value.comment = strdup((const char *) data); - break; - case SCCONF_ITEM_TYPE_BLOCK: - if (!dst) - return NULL; - dst->parent = parser.block; - parser.current_item->value.block = dst; - scconf_list_destroy(parser.name); - break; - case SCCONF_ITEM_TYPE_VALUE: - scconf_list_copy((const scconf_list *) data, &parser.current_item->value.list); - break; + if (parser.current_item) { + switch (parser.current_item->type) { + case SCCONF_ITEM_TYPE_COMMENT: + parser.current_item->value.comment = strdup((const char *) data); + break; + case SCCONF_ITEM_TYPE_BLOCK: + if (!dst) + return NULL; + dst->parent = parser.block; + parser.current_item->value.block = dst; + scconf_list_destroy(parser.name); + break; + case SCCONF_ITEM_TYPE_VALUE: + scconf_list_copy((const scconf_list *) data, &parser.current_item->value.list); + break; + } + } else { + /* FIXME is it an error if item is NULL? */ } return parser.current_item; } diff --git a/src/scconf/sclex.c b/src/scconf/sclex.c index b16db4b7..c5a6758f 100644 --- a/src/scconf/sclex.c +++ b/src/scconf/sclex.c @@ -57,8 +57,10 @@ static void buf_addch(BUFHAN * bp, char ch) bp->bufmax += 256; bp->buf = (char *) realloc(bp->buf, bp->bufmax); } - bp->buf[bp->bufcur++] = ch; - bp->buf[bp->bufcur] = '\0'; + if (bp->buf) { + bp->buf[bp->bufcur++] = ch; + bp->buf[bp->bufcur] = '\0'; + } } static int buf_nextch(BUFHAN * bp) diff --git a/src/tests/base64.c b/src/tests/base64.c index ec9583dc..f868d12d 100644 --- a/src/tests/base64.c +++ b/src/tests/base64.c @@ -38,6 +38,7 @@ int main(int argc, char *argv[]) fwrite(outbuf, len, 1, stdout); r = 0; err: - fclose(inf); + if (inf) + fclose(inf); return r; } diff --git a/src/tools/dnie-tool.c b/src/tools/dnie-tool.c index 93f7c108..c856a7ba 100644 --- a/src/tools/dnie-tool.c +++ b/src/tools/dnie-tool.c @@ -146,7 +146,7 @@ int main(int argc, char* argv[]) if (r) { fprintf(stderr, "Error: Failed to establish context: %s\n", sc_strerror(r)); - return -1; + goto dnie_tool_end; } if (verbose > 1) { diff --git a/src/tools/piv-tool.c b/src/tools/piv-tool.c index c8455317..596b5a44 100644 --- a/src/tools/piv-tool.c +++ b/src/tools/piv-tool.c @@ -113,7 +113,7 @@ static int load_object(const char * object_id, const char * object_file) int r; struct stat stat_buf; - if((fp=fopen(object_file, "r"))==NULL){ + if(!object_file || (fp=fopen(object_file, "r")) == NULL){ printf("Cannot open object file, %s %s\n", (object_file)?object_file:"", strerror(errno)); return -1; diff --git a/src/tools/pkcs15-init.c b/src/tools/pkcs15-init.c index aa3474a6..21abbbf4 100644 --- a/src/tools/pkcs15-init.c +++ b/src/tools/pkcs15-init.c @@ -1296,8 +1296,7 @@ static int get_cert_info(sc_pkcs15_card_t *myp15card, sc_pkcs15_object_t *certob } done: - if (cert) - sc_pkcs15_free_certificate(cert); + sc_pkcs15_free_certificate(cert); if (othercert) sc_pkcs15_free_certificate(othercert); diff --git a/src/tools/pkcs15-tool.c b/src/tools/pkcs15-tool.c index 603a7534..b357319a 100644 --- a/src/tools/pkcs15-tool.c +++ b/src/tools/pkcs15-tool.c @@ -783,17 +783,17 @@ static int read_ssh_key(void) struct sc_pkcs15_object *obj = NULL; sc_pkcs15_pubkey_t *pubkey = NULL; sc_pkcs15_cert_t *cert = NULL; - FILE *outf = NULL; + FILE *outf = NULL; - if (opt_outfile != NULL) { - outf = fopen(opt_outfile, "w"); - if (outf == NULL) { - fprintf(stderr, "Error opening file '%s': %s\n", opt_outfile, strerror(errno)); - goto fail2; - } - } + if (opt_outfile != NULL) { + outf = fopen(opt_outfile, "w"); + if (outf == NULL) { + fprintf(stderr, "Error opening file '%s': %s\n", opt_outfile, strerror(errno)); + goto fail2; + } + } else { - outf = stdout; + outf = stdout; } id.len = SC_PKCS15_MAX_ID_SIZE; @@ -988,8 +988,8 @@ static int read_ssh_key(void) free(uu); } - if (outf != stdout) - fclose(outf); + if (outf != stdout) + fclose(outf); if (cert) sc_pkcs15_free_certificate(cert); else if (pubkey) @@ -999,8 +999,8 @@ static int read_ssh_key(void) fail: printf("can't convert key: buffer too small\n"); fail2: - if (outf != stdout) - fclose(outf); + if (outf && outf != stdout) + fclose(outf); if (cert) sc_pkcs15_free_certificate(cert); else if (pubkey) diff --git a/src/tools/westcos-tool.c b/src/tools/westcos-tool.c index 54a5d65d..24136fbb 100644 --- a/src/tools/westcos-tool.c +++ b/src/tools/westcos-tool.c @@ -305,7 +305,7 @@ static int cert2der(X509 *cert, u8 **value) static int create_file_cert(sc_card_t *card) { int r; - int size; + int size = 0; sc_path_t path; sc_file_t *file = NULL; @@ -313,12 +313,13 @@ static int create_file_cert(sc_card_t *card) r = sc_select_file(card, &path, &file); if(r) goto out; - size = (file->size) - 32; - if(file) { + size = (file->size) - 32; sc_file_free(file); file = NULL; + } else { + size = 2048; } sc_format_path("0002", &path); @@ -903,8 +904,7 @@ out: sc_disconnect_card(card); } - if (ctx) - sc_release_context(ctx); + sc_release_context(ctx); return EXIT_SUCCESS; } From 1b53b59ed393057cc7840b916d4081ef64a1da74 Mon Sep 17 00:00:00 2001 From: Frank Morgner Date: Sat, 24 Jan 2015 19:22:39 +0100 Subject: [PATCH 02/30] fixed potential use after free reported by coverity scan --- src/tools/sc-hsm-tool.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/tools/sc-hsm-tool.c b/src/tools/sc-hsm-tool.c index ac94aa90..7e1eb707 100644 --- a/src/tools/sc-hsm-tool.c +++ b/src/tools/sc-hsm-tool.c @@ -1169,7 +1169,10 @@ static int wrap_key(sc_card_t *card, int keyid, const char *outf, const char *pi memcpy(ptr, key, key_len); ptr += key_len; + free(key); + key = NULL; + key_len = 0; // Add private key description if (ef_prkd_len > 0) { @@ -1184,7 +1187,6 @@ static int wrap_key(sc_card_t *card, int keyid, const char *outf, const char *pi } // Encode key, key decription and certificate object in sequence - key_len = 0; wrap_with_tag(0x30, keyblob, ptr - keyblob, &key, &key_len); out = fopen(outf, "wb"); From 8df9896204f4d776bd312f725436f2f791abd377 Mon Sep 17 00:00:00 2001 From: Frank Morgner Date: Sat, 24 Jan 2015 19:25:11 +0100 Subject: [PATCH 03/30] pass big parameter by reference reported by coverity scan --- src/tools/pkcs15-init.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/tools/pkcs15-init.c b/src/tools/pkcs15-init.c index 21abbbf4..81d883c8 100644 --- a/src/tools/pkcs15-init.c +++ b/src/tools/pkcs15-init.c @@ -1311,7 +1311,7 @@ done: */ static int do_delete_crypto_objects(sc_pkcs15_card_t *myp15card, struct sc_profile *profile, - const sc_pkcs15_id_t id, + const sc_pkcs15_id_t *id, unsigned int which) { sc_pkcs15_object_t *objs[10]; /* 1 priv + 1 pub + chain of at most 8 certs, should be enough */ @@ -1327,23 +1327,23 @@ static int do_delete_crypto_objects(sc_pkcs15_card_t *myp15card, } for (i = 0; i< r; i++) - if (sc_pkcs15_compare_id(&id, &((struct sc_pkcs15_prkey_info *)key_objs[i]->data)->id)) + if (sc_pkcs15_compare_id(id, &((struct sc_pkcs15_prkey_info *)key_objs[i]->data)->id)) objs[count++] = key_objs[i]; if (!count) - fprintf(stderr, "NOTE: couldn't find privkey %s to delete\n", sc_pkcs15_print_id(&id)); + fprintf(stderr, "NOTE: couldn't find privkey %s to delete\n", sc_pkcs15_print_id(id)); } if (which & SC_PKCS15INIT_TYPE_PUBKEY) { - if (sc_pkcs15_find_pubkey_by_id(myp15card, &id, &objs[count]) != 0) - fprintf(stderr, "NOTE: couldn't find pubkey %s to delete\n", sc_pkcs15_print_id(&id)); + if (sc_pkcs15_find_pubkey_by_id(myp15card, id, &objs[count]) != 0) + fprintf(stderr, "NOTE: couldn't find pubkey %s to delete\n", sc_pkcs15_print_id(id)); else count++; } if (which & SC_PKCS15INIT_TYPE_CERT) { - if (sc_pkcs15_find_cert_by_id(myp15card, &id, &objs[count]) != 0) - fprintf(stderr, "NOTE: couldn't find cert %s to delete\n", sc_pkcs15_print_id(&id)); + if (sc_pkcs15_find_cert_by_id(myp15card, id, &objs[count]) != 0) + fprintf(stderr, "NOTE: couldn't find cert %s to delete\n", sc_pkcs15_print_id(id)); else { count++; del_cert = 1; @@ -1416,7 +1416,7 @@ do_delete_objects(struct sc_profile *profile, unsigned int myopt_delete_flags) util_fatal("Specify the --id for key(s) or cert(s) to be deleted\n"); sc_pkcs15_format_id(opt_objectid, &id); - r = do_delete_crypto_objects(p15card, profile, id, myopt_delete_flags); + r = do_delete_crypto_objects(p15card, profile, &id, myopt_delete_flags); if (r >= 0) count += r; } From fca3a37097a7653dfc318ca8e99450cbd2747eea Mon Sep 17 00:00:00 2001 From: Frank Morgner Date: Sat, 24 Jan 2015 19:47:01 +0100 Subject: [PATCH 04/30] fixed truncated stdio return value --- src/tools/sc-hsm-tool.c | 2 +- src/tools/util.c | 17 +++++++++-------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/tools/sc-hsm-tool.c b/src/tools/sc-hsm-tool.c index 7e1eb707..e9d7f474 100644 --- a/src/tools/sc-hsm-tool.c +++ b/src/tools/sc-hsm-tool.c @@ -424,7 +424,7 @@ void clearScreen() void waitForEnterKeyPressed() { - char c; + int c; fflush(stdout); while ((c = getchar()) != '\n' && c != EOF) { diff --git a/src/tools/util.c b/src/tools/util.c index 0b0cebd6..e11ffb02 100644 --- a/src/tools/util.c +++ b/src/tools/util.c @@ -376,7 +376,8 @@ util_getpass (char **lineptr, size_t *len, FILE *stream) { #define MAX_PASS_SIZE 128 char *buf; - unsigned int i; + size_t i; + int ch = 0; #ifndef _WIN32 struct termios old, new; @@ -395,26 +396,26 @@ util_getpass (char **lineptr, size_t *len, FILE *stream) for (i = 0; i < MAX_PASS_SIZE - 1; i++) { #ifndef _WIN32 - buf[i] = getchar(); + ch = getchar(); #else - buf[i] = _getch(); + ch = _getch(); #endif - if (buf[i] == 0 || buf[i] == 3) + if (ch == 0 || ch == 3) break; - if (buf[i] == '\n' || buf[i] == '\r') + if (ch == '\n' || ch == '\r') break; + + buf[i] = (char) ch; } #ifndef _WIN32 tcsetattr (fileno (stdout), TCSAFLUSH, &old); fputs("\n", stdout); #endif - if (buf[i] == 0 || buf[i] == 3) { + if (ch == 0 || ch == 3) { free(buf); return -1; } - buf[i] = 0; - if (*lineptr && (!len || *len < i+1)) { free(*lineptr); *lineptr = NULL; From 9a4b58800b8c1fc89f598d52b28994be2fbf8bd7 Mon Sep 17 00:00:00 2001 From: Frank Morgner Date: Sat, 24 Jan 2015 20:00:03 +0100 Subject: [PATCH 05/30] fixed Printf arg type mismatch --- src/tools/cardos-tool.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/tools/cardos-tool.c b/src/tools/cardos-tool.c index f171220c..e271fed6 100644 --- a/src/tools/cardos-tool.c +++ b/src/tools/cardos-tool.c @@ -593,7 +593,7 @@ static int cardos_format(const char *opt_startkey) if (check_apdu(&apdu)) return 1; if (apdu.resplen < 0x04) { - printf("expected 4-6 bytes form GET DATA for startkey data, but got only %u\n", apdu.resplen); + printf("expected 4-6 bytes form GET DATA for startkey data, but got only %zu\n", apdu.resplen); printf("aborting\n"); return 1; } @@ -914,7 +914,7 @@ static int cardos_change_startkey(const char *change_startkey_apdu) if (check_apdu(&apdu)) return 1; if (apdu.resplen < 0x04) { - printf("expected 4-6 bytes form GET DATA for startkey data, but got only %u\n", apdu.resplen); + printf("expected 4-6 bytes form GET DATA for startkey data, but got only %zu\n", apdu.resplen); printf("aborting\n"); return 1; } @@ -999,7 +999,7 @@ change_startkey: if (check_apdu(&apdu)) return 1; if (apdu.resplen < 0x04) { - printf("expected 4-6 bytes form GET DATA for startkey data, but got only %u\n", apdu.resplen); + printf("expected 4-6 bytes form GET DATA for startkey data, but got only %zu\n", apdu.resplen); printf("aborting\n"); return 1; } From 3f64d3a805b93f1c436c321c3fb832757d4c07b4 Mon Sep 17 00:00:00 2001 From: Frank Morgner Date: Sat, 24 Jan 2015 20:11:16 +0100 Subject: [PATCH 06/30] fixed bad memory allocation --- src/tools/pkcs11-tool.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/pkcs11-tool.c b/src/tools/pkcs11-tool.c index c07e61d9..55b3996d 100644 --- a/src/tools/pkcs11-tool.c +++ b/src/tools/pkcs11-tool.c @@ -2836,7 +2836,7 @@ get_mechanisms(CK_SLOT_ID slot, CK_MECHANISM_TYPE_PTR *pList, CK_FLAGS flags) CK_RV rv; rv = p11->C_GetMechanismList(slot, *pList, &ulCount); - *pList = calloc(ulCount, sizeof(*pList)); + *pList = calloc(ulCount, sizeof(**pList)); if (*pList == NULL) util_fatal("calloc failed: %m"); From 6641cbf45559050ebcc059ed3df33e26ea185c15 Mon Sep 17 00:00:00 2001 From: Frank Morgner Date: Sat, 24 Jan 2015 20:17:26 +0100 Subject: [PATCH 07/30] fixed potential string overflow --- src/libopensc/pkcs15-tcos.c | 4 +++- src/tools/cryptoflex-tool.c | 5 +++-- src/tools/pkcs11-tool.c | 10 ++++++---- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/libopensc/pkcs15-tcos.c b/src/libopensc/pkcs15-tcos.c index 4c98518e..c848fb83 100644 --- a/src/libopensc/pkcs15-tcos.c +++ b/src/libopensc/pkcs15-tcos.c @@ -25,6 +25,7 @@ #include #include "common/compat_strlcpy.h" +#include "common/compat_strlcat.h" #include "internal.h" #include "pkcs15.h" #include "cardctl.h" @@ -261,7 +262,8 @@ static char *dirpath(char *dir, const char *path){ static char buf[SC_MAX_PATH_STRING_SIZE]; strcpy(buf,dir); - return strcat(buf,path); + strlcat(buf,path,sizeof buf); + return buf; } static int detect_netkey( diff --git a/src/tools/cryptoflex-tool.c b/src/tools/cryptoflex-tool.c index 56113a83..fbc83b77 100644 --- a/src/tools/cryptoflex-tool.c +++ b/src/tools/cryptoflex-tool.c @@ -28,6 +28,7 @@ #include "libopensc/pkcs15.h" #include "common/compat_strlcpy.h" +#include "common/compat_strlcat.h" #include "util.h" static const char *app_name = "cryptoflex-tool"; @@ -145,7 +146,7 @@ static int select_app_df(void) strcpy(str, "3F00"); if (opt_appdf != NULL) - strcat(str, opt_appdf); + strlcat(str, opt_appdf, sizeof str); sc_format_path(str, &path); r = sc_select_file(card, &path, &file); if (r) { @@ -945,7 +946,7 @@ static int create_pin(void) } strcpy(buf, "3F00"); if (opt_appdf != NULL) - strcat(buf, opt_appdf); + strlcat(buf, opt_appdf, sizeof buf); sc_format_path(buf, &path); return create_pin_file(&path, opt_pin_num, ""); diff --git a/src/tools/pkcs11-tool.c b/src/tools/pkcs11-tool.c index 55b3996d..0d00c4a4 100644 --- a/src/tools/pkcs11-tool.c +++ b/src/tools/pkcs11-tool.c @@ -47,6 +47,8 @@ #include "pkcs11/pkcs11.h" #include "pkcs11/pkcs11-opensc.h" #include "libopensc/asn1.h" +#include "common/compat_strlcat.h" +#include "common/compat_strlcpy.h" #include "util.h" extern void *C_LoadModule(const char *name, CK_FUNCTION_LIST_PTR_PTR); @@ -1145,7 +1147,7 @@ static void init_token(CK_SLOT_ID slot) util_fatal("No PIN entered, exiting\n"); if (!new_pin || !*new_pin || strlen(new_pin) > 20) util_fatal("Invalid SO PIN\n"); - strcpy(new_buf, new_pin); + strlcpy(new_buf, new_pin, sizeof new_buf); free(new_pin); new_pin = NULL; printf("Please enter the new SO PIN (again): "); r = util_getpass(&new_pin, &len, stdin); @@ -1318,7 +1320,7 @@ static int unlock_pin(CK_SLOT_ID slot, CK_SESSION_HANDLE sess, int login_type) r = util_getpass(&new_pin, &len, stdin); if (r < 0) return 1; - strcpy(new_buf, new_pin); + strlcpy(new_buf, new_pin, sizeof new_buf); printf("Please enter the new PIN again: "); r = util_getpass(&new_pin, &len, stdin); @@ -4434,8 +4436,8 @@ static const char *p11_flag_names(struct flag_info *list, CK_FLAGS value) buffer[0] = '\0'; while (list->value) { if (list->value & value) { - strcat(buffer, sepa); - strcat(buffer, list->name); + strlcat(buffer, sepa, sizeof buffer); + strlcat(buffer, list->name, sizeof buffer); value &= ~list->value; sepa = ", "; } From 00330b2c7937bbceae3e3ebfae112c626c7ae231 Mon Sep 17 00:00:00 2001 From: Frank Morgner Date: Sat, 24 Jan 2015 22:16:22 +0100 Subject: [PATCH 08/30] fixed resource leak --- src/libopensc/card.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/libopensc/card.c b/src/libopensc/card.c index 5142d0c0..f909bf85 100644 --- a/src/libopensc/card.c +++ b/src/libopensc/card.c @@ -222,18 +222,17 @@ int sc_connect_card(sc_reader_t *reader, sc_card_t **card_out) } if (card->name == NULL) card->name = card->driver->name; - *card_out = card; - /* Override card limitations with reader limitations. - * Note that zero means no limitations at all. + /* Override card limitations with reader limitations. + * Note that zero means no limitations at all. */ - if ((card->max_recv_size == 0) || - ((reader->driver->max_recv_size != 0) && (reader->driver->max_recv_size < card->max_recv_size))) - card->max_recv_size = reader->driver->max_recv_size; + if ((card->max_recv_size == 0) || + ((reader->driver->max_recv_size != 0) && (reader->driver->max_recv_size < card->max_recv_size))) + card->max_recv_size = reader->driver->max_recv_size; - if ((card->max_send_size == 0) || - ((reader->driver->max_send_size != 0) && (reader->driver->max_send_size < card->max_send_size))) - card->max_send_size = reader->driver->max_send_size; + if ((card->max_send_size == 0) || + ((reader->driver->max_send_size != 0) && (reader->driver->max_send_size < card->max_send_size))) + card->max_send_size = reader->driver->max_send_size; sc_log(ctx, "card info name:'%s', type:%i, flags:0x%X, max_send/recv_size:%i/%i", card->name, card->type, card->flags, card->max_send_size, card->max_recv_size); @@ -246,6 +245,7 @@ int sc_connect_card(sc_reader_t *reader, sc_card_t **card_out) goto err; } #endif + *card_out = card; LOG_FUNC_RETURN(ctx, SC_SUCCESS); err: From b6a935a261d87fa77f1682763ff7c7f55189f246 Mon Sep 17 00:00:00 2001 From: Frank Morgner Date: Sat, 24 Jan 2015 23:12:47 +0100 Subject: [PATCH 09/30] fixed memory leak --- src/libopensc/apdu.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libopensc/apdu.c b/src/libopensc/apdu.c index 55f13d08..38c75153 100644 --- a/src/libopensc/apdu.c +++ b/src/libopensc/apdu.c @@ -203,8 +203,10 @@ int sc_apdu_get_octets(sc_context_t *ctx, const sc_apdu_t *apdu, u8 **buf, if (nbuf == NULL) return SC_ERROR_OUT_OF_MEMORY; /* encode the APDU in the buffer */ - if (sc_apdu2bytes(ctx, apdu, proto, nbuf, nlen) != SC_SUCCESS) + if (sc_apdu2bytes(ctx, apdu, proto, nbuf, nlen) != SC_SUCCESS) { + free(nbuf); return SC_ERROR_INTERNAL; + } *buf = nbuf; *len = nlen; From ea40322a30a8053ee859a3a5ae9d20d17af830da Mon Sep 17 00:00:00 2001 From: Frank Morgner Date: Wed, 21 Jan 2015 16:57:02 +0100 Subject: [PATCH 10/30] added travis-ci configuration --- .travis.yml | 56 ++++++++++++++++++++++++++++++++++++++ src/libopensc/iasecc-sdo.c | 11 ++++++++ 2 files changed, 67 insertions(+) create mode 100644 .travis.yml diff --git a/.travis.yml b/.travis.yml new file mode 100644 index 00000000..ed63e92c --- /dev/null +++ b/.travis.yml @@ -0,0 +1,56 @@ +language: C + +env: + global: + # The next declaration is the encrypted COVERITY_SCAN_TOKEN, created + # via the "travis encrypt" command using the project repo's public key + - secure: "UkHn7wy4im8V1nebCWbAetnDSOLRUbOlF6++ovk/7Bnso1/lnhXHelyzgRxfD/oI68wm9nnRV+RQEZ9+72Ug1CyvHxyyxxkwal/tPeHH4B/L+aGdPi0id+5OZSKIm77VP3m5s102sJMJgH7DFd03+nUd0K26p0tk8ad4j1geV4c=" + +matrix: + include: + - compiler: clang + - compiler: gcc + - compiler: gcc + env: HOST=i686-w64-mingw32 + +before_install: + - if [ $TRAVIS_OS_NAME == linux ]; then + sudo apt-get update; + fi + +install: + - if [ $TRAVIS_OS_NAME == linux ]; then + if [ -z "$HOST" ]; then + sudo apt-get install libpcsclite-dev xsltproc docbook-xsl; + else + sudo apt-get install mingw-w64 binutils-mingw-w64-i686 gcc-mingw-w64-i686; + fi + fi + +before_script: + - ./bootstrap + - if [ -z "$HOST" ]; then + ./configure --enable-doc --enable-dnie-ui; + else + unset CC; + unset CXX; + ./configure --host=$HOST --disable-openssl; + fi + +addons: + coverity_scan: + project: + name: "OpenSC/OpenSC" + description: "Build submitted via Travis CI" + notification_email: viktor.tarasov@gmail.com + build_command: "make -j 4" + branch_pattern: coverity_scan + +script: + - if [ "${COVERITY_SCAN_BRANCH}" != 1 ]; then + make; + fi + - if [ -z "$HOST" -a "${COVERITY_SCAN_BRANCH}" != 1 ]; then + make check; + make dist; + fi diff --git a/src/libopensc/iasecc-sdo.c b/src/libopensc/iasecc-sdo.c index ea5aa971..b1d3096f 100644 --- a/src/libopensc/iasecc-sdo.c +++ b/src/libopensc/iasecc-sdo.c @@ -1287,4 +1287,15 @@ iasecc_docp_copy(struct sc_context *ctx, struct iasecc_sdo_docp *in, struct iase LOG_FUNC_RETURN(ctx, SC_SUCCESS); } +#else + +/* we need to define the functions below to export them */ +#include "errors.h" + +int +iasecc_sdo_encode_update_field() +{ + return SC_ERROR_NOT_SUPPORTED; +} + #endif /* ENABLE_OPENSSL */ From fdd38f6e0437077086a793dadc912f07c75d8774 Mon Sep 17 00:00:00 2001 From: Frank Morgner Date: Wed, 28 Jan 2015 04:30:40 +0100 Subject: [PATCH 11/30] fixed copy into fixed size buffer --- src/libopensc/pkcs15-tcos.c | 2 +- src/pkcs15init/pkcs15-lib.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libopensc/pkcs15-tcos.c b/src/libopensc/pkcs15-tcos.c index c848fb83..30762156 100644 --- a/src/libopensc/pkcs15-tcos.c +++ b/src/libopensc/pkcs15-tcos.c @@ -261,7 +261,7 @@ static int insert_pin( static char *dirpath(char *dir, const char *path){ static char buf[SC_MAX_PATH_STRING_SIZE]; - strcpy(buf,dir); + strlcpy(buf,dir,sizeof buf); strlcat(buf,path,sizeof buf); return buf; } diff --git a/src/pkcs15init/pkcs15-lib.c b/src/pkcs15init/pkcs15-lib.c index 02422c73..fb76ac98 100644 --- a/src/pkcs15init/pkcs15-lib.c +++ b/src/pkcs15init/pkcs15-lib.c @@ -350,7 +350,7 @@ sc_pkcs15init_bind(struct sc_card *card, const char *name, const char *profile_o * If none is defined, use the default profile name. */ if (!get_profile_from_config(card, card_profile, sizeof(card_profile))) - strcpy(card_profile, driver); + strclpy(card_profile, driver, sizeof card_profile); if (profile_option != NULL) strlcpy(card_profile, profile_option, sizeof(card_profile)); From 77752f442d90665b814f5a7fbbe5d3f28000860b Mon Sep 17 00:00:00 2001 From: Frank Morgner Date: Wed, 28 Jan 2015 05:47:20 +0100 Subject: [PATCH 12/30] fixed unused value --- src/libopensc/card-dnie.c | 3 +-- src/libopensc/pkcs15-itacns.c | 2 ++ src/pkcs11/framework-pkcs15.c | 5 ++++- src/pkcs11/pkcs11-object.c | 2 +- src/pkcs15init/pkcs15-lib.c | 3 ++- src/pkcs15init/pkcs15-sc-hsm.c | 3 ++- src/pkcs15init/pkcs15-westcos.c | 2 ++ src/tools/pkcs15-tool.c | 6 ++---- 8 files changed, 16 insertions(+), 10 deletions(-) diff --git a/src/libopensc/card-dnie.c b/src/libopensc/card-dnie.c index 7bd0df7f..9359de33 100644 --- a/src/libopensc/card-dnie.c +++ b/src/libopensc/card-dnie.c @@ -346,7 +346,6 @@ static int dnie_get_info(sc_card_t * card, char *data[]) } res = dnie_read_file(card, path, &file, &buffer, &bufferlen); if (res != SC_SUCCESS) { - msg = "Cannot read IDESP EF"; data[3]=NULL; goto get_info_ph3; } @@ -1195,7 +1194,7 @@ static int dnie_set_security_env(struct sc_card *card, sc_log(card->ctx, "checking key references"); if (env->key_ref_len != 1) { sc_log(card->ctx, "Null or invalid key ID reference"); - result = SC_ERROR_INVALID_ARGUMENTS; + LOG_FUNC_RETURN(card->ctx, SC_ERROR_INVALID_ARGUMENTS); } sc_log(card->ctx, "Using key reference '%s'", sc_dump_hex(env->key_ref, env->key_ref_len)); diff --git a/src/libopensc/pkcs15-itacns.c b/src/libopensc/pkcs15-itacns.c index c56f05c5..55e9f31b 100644 --- a/src/libopensc/pkcs15-itacns.c +++ b/src/libopensc/pkcs15-itacns.c @@ -499,6 +499,8 @@ static int itacns_add_data_files(sc_pkcs15_card_t *p15card) sizeof(obj.label)); data.path = path; rv = sc_pkcs15emu_add_data_object(p15card, &obj, &data); + SC_TEST_RET(p15card->card->ctx, SC_LOG_DEBUG_NORMAL, rv, + "Could not add data file"); } /* diff --git a/src/pkcs11/framework-pkcs15.c b/src/pkcs11/framework-pkcs15.c index 2bf63774..45a57d7e 100644 --- a/src/pkcs11/framework-pkcs15.c +++ b/src/pkcs11/framework-pkcs15.c @@ -4669,8 +4669,11 @@ register_mechanisms(struct sc_pkcs11_card *p11card) alg_info++; } - if (flags & SC_ALGORITHM_ECDSA_RAW) + if (flags & SC_ALGORITHM_ECDSA_RAW) { rc = register_ec_mechanisms(p11card, flags, ec_ext_flags, ec_min_key_size, ec_max_key_size); + if (rc != CKR_OK) + return rc; + } if (flags & (SC_ALGORITHM_GOSTR3410_RAW | SC_ALGORITHM_GOSTR3410_HASH_NONE diff --git a/src/pkcs11/pkcs11-object.c b/src/pkcs11/pkcs11-object.c index 90b5e1d5..70291b3a 100644 --- a/src/pkcs11/pkcs11-object.c +++ b/src/pkcs11/pkcs11-object.c @@ -394,7 +394,7 @@ C_FindObjectsInit(CK_SESSION_HANDLE hSession, /* the session's handle */ sizeof(CK_OBJECT_HANDLE) * operation->allocated_handles); if (operation->handles == NULL) { rv = CKR_HOST_MEMORY; - break; + goto out; } } operation->handles[operation->num_handles++] = object->handle; diff --git a/src/pkcs15init/pkcs15-lib.c b/src/pkcs15init/pkcs15-lib.c index fb76ac98..5e61399f 100644 --- a/src/pkcs15init/pkcs15-lib.c +++ b/src/pkcs15init/pkcs15-lib.c @@ -350,7 +350,7 @@ sc_pkcs15init_bind(struct sc_card *card, const char *name, const char *profile_o * If none is defined, use the default profile name. */ if (!get_profile_from_config(card, card_profile, sizeof(card_profile))) - strclpy(card_profile, driver, sizeof card_profile); + strlcpy(card_profile, driver, sizeof card_profile); if (profile_option != NULL) strlcpy(card_profile, profile_option, sizeof(card_profile)); @@ -3159,6 +3159,7 @@ sc_pkcs15init_get_transport_key(struct sc_profile *profile, struct sc_pkcs15_car if (callbacks.get_key) { rv = callbacks.get_key(profile, type, reference, defbuf, defsize, pinbuf, pinsize); + LOG_TEST_RET(ctx, rv, "Cannot get key"); } else if (rv >= 0) { if (*pinsize < defsize) diff --git a/src/pkcs15init/pkcs15-sc-hsm.c b/src/pkcs15init/pkcs15-sc-hsm.c index 9e7ebbc8..0f2fd8af 100644 --- a/src/pkcs15init/pkcs15-sc-hsm.c +++ b/src/pkcs15init/pkcs15-sc-hsm.c @@ -271,9 +271,10 @@ static int sc_hsm_generate_key(struct sc_profile *profile, struct sc_pkcs15_card r = sc_hsm_encode_gakp_ec(p15card, &cvc, key_info); break; default: - LOG_FUNC_RETURN(card->ctx, SC_ERROR_NOT_IMPLEMENTED); + r = SC_ERROR_NOT_IMPLEMENTED; break; } + LOG_TEST_RET(p15card->card->ctx, r, "Could not encode GAKP cdata"); r = sc_pkcs15emu_sc_hsm_encode_cvc(p15card, &cvc, &cvcbin, &cvclen); sc_pkcs15emu_sc_hsm_free_cvc(&cvc); diff --git a/src/pkcs15init/pkcs15-westcos.c b/src/pkcs15init/pkcs15-westcos.c index e75fb3ac..1fc70b4a 100644 --- a/src/pkcs15init/pkcs15-westcos.c +++ b/src/pkcs15init/pkcs15-westcos.c @@ -270,6 +270,8 @@ static int westcos_pkcs15init_generate_key(sc_profile_t *profile, pubkey->algorithm = SC_ALGORITHM_RSA; r = sc_pkcs15_decode_pubkey(p15card->card->ctx, pubkey, p, lg); + if (r < 0) + goto out; } (void) BIO_reset(mem); diff --git a/src/tools/pkcs15-tool.c b/src/tools/pkcs15-tool.c index b357319a..8c980bb6 100644 --- a/src/tools/pkcs15-tool.c +++ b/src/tools/pkcs15-tool.c @@ -306,7 +306,7 @@ print_pem_object(const char *kind, const u8*data, size_t data_len) return 0; } -static int +static void list_data_object(const char *kind, const unsigned char *data, size_t data_len) { char title[0x100]; @@ -321,8 +321,6 @@ list_data_object(const char *kind, const unsigned char *data, size_t data_len) printf("%02X", data[i]); } printf("\n"); - - return 0; } static int @@ -478,7 +476,7 @@ static int list_data_objects(void) continue; /* DEE emulation may say there is a file */ return 1; } - r = list_data_object("\tData", data_object->data, data_object->data_len); + list_data_object("\tData", data_object->data, data_object->data_len); sc_pkcs15_free_data_object(data_object); } else { From 7c497b324f8a79cec3dc374373efe6e2040bab2b Mon Sep 17 00:00:00 2001 From: Frank Morgner Date: Wed, 28 Jan 2015 05:51:00 +0100 Subject: [PATCH 13/30] fixed not null terminated buffer --- src/libopensc/card.c | 5 +++-- src/libopensc/iasecc-sm.c | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/libopensc/card.c b/src/libopensc/card.c index f909bf85..6b384069 100644 --- a/src/libopensc/card.c +++ b/src/libopensc/card.c @@ -29,6 +29,7 @@ #include "internal.h" #include "asn1.h" +#include "common/compat_strlcpy.h" /* #define INVALIDATE_CARD_CACHE_IN_UNLOCK @@ -1258,8 +1259,8 @@ sc_card_sm_check(struct sc_card *card) rv = sc_card_sm_load(card, module_path, module_name); LOG_TEST_RET(ctx, rv, "Failed to load SM module"); - strncpy(card->sm_ctx.module.filename, module_name, sizeof(card->sm_ctx.module.filename)); - strncpy(card->sm_ctx.config_section, sm, sizeof(card->sm_ctx.config_section)); + strlcpy(card->sm_ctx.module.filename, module_name, sizeof(card->sm_ctx.module.filename)); + strlcpy(card->sm_ctx.config_section, sm, sizeof(card->sm_ctx.config_section)); /* allocate resources for the external SM module */ sc_log(ctx, "'module_init' handler %p", card->sm_ctx.module.ops.module_init); diff --git a/src/libopensc/iasecc-sm.c b/src/libopensc/iasecc-sm.c index 7fdffbdc..bfa57133 100644 --- a/src/libopensc/iasecc-sm.c +++ b/src/libopensc/iasecc-sm.c @@ -25,6 +25,7 @@ #include "internal.h" #include "asn1.h" #include "cardctl.h" +#include "common/compat_strlcpy.h" #include "sm.h" #include "iasecc.h" @@ -156,7 +157,7 @@ iasecc_sm_external_authentication(struct sc_card *card, unsigned skey_ref, int * if (card->sm_ctx.sm_mode == SM_MODE_NONE) LOG_TEST_RET(ctx, SC_ERROR_NOT_SUPPORTED, "Cannot do 'External Authentication' without SM activated "); - strncpy(sm_info->config_section, card->sm_ctx.config_section, sizeof(sm_info->config_section)); + strlcpy(sm_info->config_section, card->sm_ctx.config_section, sizeof(sm_info->config_section)); sm_info->cmd = SM_CMD_EXTERNAL_AUTH; sm_info->serialnr = card->serialnr; sm_info->card_type = card->type; @@ -296,7 +297,7 @@ iasecc_sm_initialize(struct sc_card *card, unsigned se_num, unsigned cmd) LOG_FUNC_CALLED(ctx); - strncpy(sm_info->config_section, card->sm_ctx.config_section, sizeof(sm_info->config_section)); + strlcpy(sm_info->config_section, card->sm_ctx.config_section, sizeof(sm_info->config_section)); sm_info->cmd = cmd; sm_info->serialnr = card->serialnr; sm_info->card_type = card->type; From 027e4a08679c1bacb139e990ec33b8a61f3d416a Mon Sep 17 00:00:00 2001 From: Frank Morgner Date: Wed, 28 Jan 2015 05:59:41 +0100 Subject: [PATCH 14/30] fixed out of bounds read --- src/libopensc/ctx.c | 2 +- src/libopensc/reader-pcsc.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libopensc/ctx.c b/src/libopensc/ctx.c index daade137..fa94dc14 100644 --- a/src/libopensc/ctx.c +++ b/src/libopensc/ctx.c @@ -840,7 +840,7 @@ int sc_set_card_driver(sc_context_t *ctx, const char *short_name) if (short_name == NULL) { ctx->forced_driver = NULL; match = 1; - } else while (ctx->card_drivers[i] != NULL && i < SC_MAX_CARD_DRIVERS) { + } else while (i < SC_MAX_CARD_DRIVERS && ctx->card_drivers[i] != NULL) { struct sc_card_driver *drv = ctx->card_drivers[i]; if (strcmp(short_name, drv->short_name) == 0) { diff --git a/src/libopensc/reader-pcsc.c b/src/libopensc/reader-pcsc.c index b6a33c7a..da65c3fb 100644 --- a/src/libopensc/reader-pcsc.c +++ b/src/libopensc/reader-pcsc.c @@ -768,7 +768,7 @@ static unsigned long part10_detect_pace_capabilities(sc_reader_t *reader) PACE_FUNCTION_GetReaderPACECapabilities, /* idxFunction */ 0, 0, /* lengthInputData */ }; - u8 rbuf[6]; + u8 rbuf[7]; u8 *p = rbuf; size_t rcount = sizeof rbuf; struct pcsc_private_data *priv; From b1b99ce7e5973569e743c3ce285b2730d759ed72 Mon Sep 17 00:00:00 2001 From: Frank Morgner Date: Wed, 28 Jan 2015 06:03:52 +0100 Subject: [PATCH 15/30] fixed integer underflow --- src/pkcs15init/pkcs15-myeid.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/pkcs15init/pkcs15-myeid.c b/src/pkcs15init/pkcs15-myeid.c index 1da4515a..1c33a3b2 100644 --- a/src/pkcs15init/pkcs15-myeid.c +++ b/src/pkcs15init/pkcs15-myeid.c @@ -448,6 +448,9 @@ myeid_create_key(struct sc_profile *profile, struct sc_pkcs15_card *p15card, file->ef_structure = SC_CARDCTL_MYEID_KEY_EC; memcpy(&key_info->path.value, &file->path.value, file->path.len); + if (!file->path.len) + LOG_TEST_RET(ctx, SC_ERROR_INVALID_ARGUMENTS, + "Cannot determine private key file"); key_info->key_reference = file->path.value[file->path.len - 1] & 0xFF; sc_log(ctx, "Path of MyEID private key file to create %s", From 68d86644fd642f71e0295a81aa356e7fde319abd Mon Sep 17 00:00:00 2001 From: Frank Morgner Date: Wed, 28 Jan 2015 06:10:16 +0100 Subject: [PATCH 16/30] fixed use after free --- src/libopensc/cwa-dnie.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libopensc/cwa-dnie.c b/src/libopensc/cwa-dnie.c index 62060db0..73098f2d 100644 --- a/src/libopensc/cwa-dnie.c +++ b/src/libopensc/cwa-dnie.c @@ -265,8 +265,10 @@ int dnie_read_file(sc_card_t * card, res = SC_SUCCESS; goto dnie_read_file_end; dnie_read_file_err: - if (*file) + if (*file) { sc_file_free(*file); + *file = NULL; + } dnie_read_file_end: if (msg) sc_log(ctx, msg); From 87b2403673ca84870698b5d8ccf0efe831506c14 Mon Sep 17 00:00:00 2001 From: Frank Morgner Date: Wed, 28 Jan 2015 07:00:02 +0100 Subject: [PATCH 17/30] fixed out of bounds access/write --- src/libopensc/card-epass2003.c | 2 +- src/libopensc/card-myeid.c | 6 +++--- src/libopensc/card-setcos.c | 2 +- src/libopensc/ctbcs.c | 4 ++-- src/pkcs15init/pkcs15-asepcos.c | 2 +- src/pkcs15init/pkcs15-openpgp.c | 6 +++--- src/tools/cryptoflex-tool.c | 2 +- 7 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/libopensc/card-epass2003.c b/src/libopensc/card-epass2003.c index f3d79477..39e46f40 100644 --- a/src/libopensc/card-epass2003.c +++ b/src/libopensc/card-epass2003.c @@ -227,7 +227,7 @@ des3_encrypt_cbc(const unsigned char *key, int keysize, unsigned char iv[8], static int -des3_decrypt_cbc(const unsigned char *key, int keysize, unsigned char iv[8], +des3_decrypt_cbc(const unsigned char *key, int keysize, unsigned char iv[EVP_MAX_IV_LENGTH], const unsigned char *input, size_t length, unsigned char *output) { unsigned char bKey[24] = { 0 }; diff --git a/src/libopensc/card-myeid.c b/src/libopensc/card-myeid.c index 2e5f9c68..f9181ae3 100644 --- a/src/libopensc/card-myeid.c +++ b/src/libopensc/card-myeid.c @@ -305,8 +305,8 @@ static int encode_file_structure(sc_card_t *card, const sc_file_t *file, u8 *out, size_t *outlen) { const sc_acl_entry_t *read, *update, *delete, *generate; - u8 buf[40]; - int i; + u8 buf[41]; + size_t i; LOG_FUNC_CALLED(card->ctx); /* PrivateKey @@ -412,7 +412,7 @@ static int encode_file_structure(sc_card_t *card, const sc_file_t *file, buf[25] = 0x84; buf[26] = (u8)file->namelen; - for(i=0;i < (int)file->namelen;i++) + for(i=0;i < file->namelen;i++) buf[i + 26] = file->name[i]; buf[1] = 0x19 + file->namelen + 2; diff --git a/src/libopensc/card-setcos.c b/src/libopensc/card-setcos.c index eb9e936b..15da346e 100644 --- a/src/libopensc/card-setcos.c +++ b/src/libopensc/card-setcos.c @@ -438,7 +438,7 @@ static int setcos_create_file_44(sc_card_t *card, sc_file_t *file) const int* p_idx; int i; int len = 0; - u8 bBuf[32]; + u8 bBuf[64]; /* Get specific operation groups for specified file-type */ switch (file->type){ diff --git a/src/libopensc/ctbcs.c b/src/libopensc/ctbcs.c index 3dc773df..04420421 100644 --- a/src/libopensc/ctbcs.c +++ b/src/libopensc/ctbcs.c @@ -45,7 +45,7 @@ ctbcs_build_perform_verification_apdu(sc_apdu_t *apdu, struct sc_pin_cmd_data *d { const char *prompt; size_t buflen, count = 0, j = 0, len; - static u8 buf[254]; + static u8 buf[256]; u8 control; ctbcs_init_apdu(apdu, @@ -113,7 +113,7 @@ ctbcs_build_modify_verification_apdu(sc_apdu_t *apdu, struct sc_pin_cmd_data *da { const char *prompt; size_t buflen, count = 0, j = 0, len; - static u8 buf[254]; + static u8 buf[256]; u8 control; ctbcs_init_apdu(apdu, diff --git a/src/pkcs15init/pkcs15-asepcos.c b/src/pkcs15init/pkcs15-asepcos.c index 310c6f92..ddc72f52 100644 --- a/src/pkcs15init/pkcs15-asepcos.c +++ b/src/pkcs15init/pkcs15-asepcos.c @@ -510,7 +510,7 @@ static int asepcos_do_create_key(sc_card_t *card, size_t ksize, int fileid, int r; size_t len; sc_file_t *nfile = NULL; - u8 buf[512], *p = buf; + u8 buf[1024], *p = buf; if (sizeof(buf) < kdlen + 11) return SC_ERROR_BUFFER_TOO_SMALL; diff --git a/src/pkcs15init/pkcs15-openpgp.c b/src/pkcs15init/pkcs15-openpgp.c index f3a49621..4da415fe 100755 --- a/src/pkcs15init/pkcs15-openpgp.c +++ b/src/pkcs15init/pkcs15-openpgp.c @@ -187,7 +187,7 @@ static int openpgp_generate_key(sc_profile_t *profile, sc_pkcs15_card_t *p15card /* The OpenPGP supports only 32-bit exponent. */ key_info.exponent_len = 32; - key_info.exponent = calloc(4, 1); + key_info.exponent = calloc(key_info.exponent_len>>3, 1); /* 1/8 */ if (key_info.exponent == NULL) LOG_FUNC_RETURN(ctx, SC_ERROR_NOT_ENOUGH_MEMORY); @@ -204,10 +204,10 @@ static int openpgp_generate_key(sc_profile_t *profile, sc_pkcs15_card_t *p15card sc_log(ctx, "Set output exponent info"); pubkey->u.rsa.exponent.len = key_info.exponent_len; - pubkey->u.rsa.exponent.data = calloc(key_info.exponent_len, 1); + pubkey->u.rsa.exponent.data = calloc(key_info.exponent_len>>3, 1); /* 1/8 */ if (pubkey->u.rsa.exponent.data == NULL) goto out; - memcpy(pubkey->u.rsa.exponent.data, key_info.exponent, key_info.exponent_len); + memcpy(pubkey->u.rsa.exponent.data, key_info.exponent, key_info.exponent_len>>3); /* 1/8 */ out: if (key_info.modulus) diff --git a/src/tools/cryptoflex-tool.c b/src/tools/cryptoflex-tool.c index fbc83b77..ea1e6a43 100644 --- a/src/tools/cryptoflex-tool.c +++ b/src/tools/cryptoflex-tool.c @@ -716,7 +716,7 @@ static int encode_private_key(RSA *rsa, u8 *key, size_t *keysize) static int encode_public_key(RSA *rsa, u8 *key, size_t *keysize) { - u8 buf[512], *p = buf; + u8 buf[1024], *p = buf; u8 bnbuf[256]; int base = 0; int r; From 08fcfcc8f0b4b3c7a987b25d22430acbe8868975 Mon Sep 17 00:00:00 2001 From: Frank Morgner Date: Wed, 28 Jan 2015 07:04:02 +0100 Subject: [PATCH 18/30] fixed wrong sizeof argument --- src/tools/sc-hsm-tool.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/tools/sc-hsm-tool.c b/src/tools/sc-hsm-tool.c index e9d7f474..f0f56aeb 100644 --- a/src/tools/sc-hsm-tool.c +++ b/src/tools/sc-hsm-tool.c @@ -62,6 +62,8 @@ static int verbose = 0; #define MAX_KEY 1024 #define MAX_WRAPPED_KEY (MAX_CERT + MAX_PRKD + MAX_KEY) +#define SEED_LENGTH 16 + enum { OPT_SO_PIN = 0x100, OPT_PIN, @@ -139,14 +141,15 @@ static sc_card_t *card = NULL; * @param s Secret to share * @param n Maximum number of shares * @param rngSeed Seed value for CPRNG + * @param rngSeedLength Lenght of Seed value for CPRNG * */ -static void generatePrime(BIGNUM *prime, const BIGNUM *s, const unsigned int n, unsigned char *rngSeed) +static void generatePrime(BIGNUM *prime, const BIGNUM *s, const unsigned int n, unsigned char *rngSeed, const unsigned int rngSeedLength) { int bits = 0; // Seed the RNG - RAND_seed(rngSeed, sizeof(rngSeed)); + RAND_seed(rngSeed, rngSeedLength); // Determine minimum number of bits for prime >= max(2^r, n + 1) bits = BN_num_bits_word(n + 1) > BN_num_bits(s) ? (BN_num_bits_word(n + 1)) : (BN_num_bits(s)); @@ -851,7 +854,7 @@ static int generate_pwd_shares(sc_card_t *card, char **pwd, int *pwdlen, int pas /* * Generate seed and calculate a prime depending on the size of the secret */ - r = sc_get_challenge(card, rngseed, 16); + r = sc_get_challenge(card, rngseed, SEED_LENGTH); if (r < 0) { printf("Error generating random seed failed with %s", sc_strerror(r)); OPENSSL_cleanse(*pwd, *pwdlen); @@ -859,7 +862,7 @@ static int generate_pwd_shares(sc_card_t *card, char **pwd, int *pwdlen, int pas return r; } - generatePrime(&prime, &secret, password_shares_total, rngseed); + generatePrime(&prime, &secret, password_shares_total, rngseed, SEED_LENGTH); // Allocate data buffer for the generated shares shares = malloc(password_shares_total * sizeof(secret_share_t)); From b9f1fb333ccc2a3fbdc3d351c4253310e32720dc Mon Sep 17 00:00:00 2001 From: Frank Morgner Date: Wed, 28 Jan 2015 07:07:33 +0100 Subject: [PATCH 19/30] fixed bad output data length --- src/libopensc/muscle.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/libopensc/muscle.c b/src/libopensc/muscle.c index ef9322f2..29a2f562 100644 --- a/src/libopensc/muscle.c +++ b/src/libopensc/muscle.c @@ -662,8 +662,7 @@ int msc_compute_crypt_init(sc_card_t *card, SC_TEST_RET(card->ctx, SC_LOG_DEBUG_NORMAL, r, "APDU transmit failed"); if(apdu.sw1 == 0x90 && apdu.sw2 == 0x00) { short receivedData = outputBuffer[0] << 8 | outputBuffer[1]; - *outputDataLength = receivedData; - *outputDataLength = 0; + *outputDataLength = receivedData; assert(receivedData <= MSC_MAX_APDU); memcpy(outputData, outputBuffer + 2, receivedData); From ac0424e947098a65a011368f1e13242aa0416e6b Mon Sep 17 00:00:00 2001 From: Frank Morgner Date: Wed, 28 Jan 2015 07:09:02 +0100 Subject: [PATCH 20/30] fixed pkcs11spy's version number --- src/pkcs11/pkcs11-spy.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pkcs11/pkcs11-spy.c b/src/pkcs11/pkcs11-spy.c index 1dab64da..49a6116c 100644 --- a/src/pkcs11/pkcs11-spy.c +++ b/src/pkcs11/pkcs11-spy.c @@ -68,7 +68,7 @@ init_spy(void) if (pkcs11_spy) { /* with our own pkcs11.h we need to maintain this ourself */ pkcs11_spy->version.major = 2; - pkcs11_spy->version.major = 11; + pkcs11_spy->version.minor = 11; pkcs11_spy->C_Initialize = C_Initialize; pkcs11_spy->C_Finalize = C_Finalize; pkcs11_spy->C_GetInfo = C_GetInfo; From 7fb495ac314b97e4196732a5b0bc6b789afdc2c7 Mon Sep 17 00:00:00 2001 From: Frank Morgner Date: Wed, 28 Jan 2015 07:10:57 +0100 Subject: [PATCH 21/30] fixed self assignment --- src/pkcs15init/profile.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/pkcs15init/profile.c b/src/pkcs15init/profile.c index 433c7ae4..9f87fc9c 100644 --- a/src/pkcs15init/profile.c +++ b/src/pkcs15init/profile.c @@ -507,7 +507,6 @@ sc_profile_get_pin_info(struct sc_profile *profile, if (pi == NULL) return; - pi->pin.tries_left = pi->pin.tries_left; pi->pin.max_tries = pi->pin.tries_left; *info = pi->pin; } From 92ad6eb63c5d32ac12331f2f8d13780f1b8a75e5 Mon Sep 17 00:00:00 2001 From: Frank Morgner Date: Wed, 28 Jan 2015 07:21:55 +0100 Subject: [PATCH 22/30] fixed determining ef type --- src/libopensc/card-dnie.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libopensc/card-dnie.c b/src/libopensc/card-dnie.c index 9359de33..53ce7281 100644 --- a/src/libopensc/card-dnie.c +++ b/src/libopensc/card-dnie.c @@ -1746,7 +1746,7 @@ static int dnie_process_fci(struct sc_card *card, case 0x15: /* EF for keys: linear variable simple TLV */ file->type = SC_FILE_TYPE_WORKING_EF; /* pin file 3F000000 has also this EF type */ - if ( ( file->prop_attr[3] == 0x00 ) && (file->prop_attr[3] == 0x00 ) ) { + if ( ( file->prop_attr[2] == 0x00 ) && (file->prop_attr[3] == 0x00 ) ) { sc_log(ctx,"Processing pin EF"); break; } From 3a557ad0dddcd89e569ef69c048ceae8955974a7 Mon Sep 17 00:00:00 2001 From: Frank Morgner Date: Wed, 28 Jan 2015 07:22:03 +0100 Subject: [PATCH 23/30] fixed parsing pace output data --- src/libopensc/reader-pcsc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libopensc/reader-pcsc.c b/src/libopensc/reader-pcsc.c index da65c3fb..46ef81f3 100644 --- a/src/libopensc/reader-pcsc.c +++ b/src/libopensc/reader-pcsc.c @@ -1797,7 +1797,7 @@ static int transform_pace_output(u8 *rbuf, size_t rbuflen, if (parsed+2 > rbuflen) return SC_ERROR_UNKNOWN_DATA_RECEIVED; pace_output->mse_set_at_sw1 = rbuf[parsed+0]; - pace_output->mse_set_at_sw1 = rbuf[parsed+1]; + pace_output->mse_set_at_sw2 = rbuf[parsed+1]; parsed += 2; /* length_CardAccess */ From 734cb67924be16d0d03a8dadd92043e1f4396fd4 Mon Sep 17 00:00:00 2001 From: Frank Morgner Date: Wed, 28 Jan 2015 07:23:34 +0100 Subject: [PATCH 24/30] fixed algo ref --- src/libopensc/card-gemsafeV1.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libopensc/card-gemsafeV1.c b/src/libopensc/card-gemsafeV1.c index 7b6b87b8..9c6816b6 100644 --- a/src/libopensc/card-gemsafeV1.c +++ b/src/libopensc/card-gemsafeV1.c @@ -369,7 +369,7 @@ static u8 gemsafe_flags2algref(struct sc_card *card, const struct sc_security_en } else if (env->operation == SC_SEC_OPERATION_DECIPHER) { if (env->algorithm_flags & SC_ALGORITHM_RSA_PAD_PKCS1) ret = (card->type == SC_CARD_TYPE_GEMSAFEV1_PTEID || - card->type == SC_CARD_TYPE_GEMSAFEV1_PTEID) ? 0x02 : 0x12; + card->type == SC_CARD_TYPE_GEMSAFEV1_SEEID) ? 0x02 : 0x12; } return ret; From b94c16394f89b78f9d43ecd7a8fb25f3e25b842e Mon Sep 17 00:00:00 2001 From: Frank Morgner Date: Wed, 28 Jan 2015 07:25:46 +0100 Subject: [PATCH 25/30] card-asepcos: fixed puk handling --- src/libopensc/card-asepcos.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libopensc/card-asepcos.c b/src/libopensc/card-asepcos.c index 855c53a1..433c28fd 100644 --- a/src/libopensc/card-asepcos.c +++ b/src/libopensc/card-asepcos.c @@ -872,8 +872,8 @@ static int asepcos_build_pin_apdu(sc_card_t *card, sc_apdu_t *apdu, memcpy(p, data->pin1.data, data->pin1.len); p += data->pin1.len; } else { - memcpy(p, data->pin1.data, data->pin1.len); - p += data->pin1.len; + memcpy(p, data->pin2.data, data->pin2.len); + p += data->pin2.len; } apdu->lc = p - buf; apdu->datalen = p - buf; From bd3cfcf5ef9770535369f679351d256a8c1e8a6d Mon Sep 17 00:00:00 2001 From: Frank Morgner Date: Wed, 28 Jan 2015 07:26:34 +0100 Subject: [PATCH 26/30] fixed copy/paste error --- src/libopensc/iso7816.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libopensc/iso7816.c b/src/libopensc/iso7816.c index ab901fbe..85062651 100644 --- a/src/libopensc/iso7816.c +++ b/src/libopensc/iso7816.c @@ -657,7 +657,7 @@ iso7816_construct_fci(struct sc_card *card, const sc_file_t *file, p, *outlen - (p - out), &p); } if (file->sec_attr_len) { - assert(sizeof(buf) >= file->prop_attr_len); + assert(sizeof(buf) >= file->sec_attr_len); memcpy(buf, file->sec_attr, file->sec_attr_len); sc_asn1_put_tag(0x86, buf, file->sec_attr_len, p, *outlen - (p - out), &p); From 2e04fa99c11c47b2c5c7f5b4bd5474bc310e51ad Mon Sep 17 00:00:00 2001 From: Frank Morgner Date: Wed, 28 Jan 2015 07:39:35 +0100 Subject: [PATCH 27/30] fixed pointless array comparisons --- src/libopensc/card-openpgp.c | 2 +- src/libopensc/card-piv.c | 2 -- src/libopensc/card-tcos.c | 2 -- src/pkcs15init/pkcs15-oberthur-awp.c | 12 ++++-------- src/tools/pkcs11-tool.c | 4 ++-- src/tools/pkcs15-init.c | 5 ++--- src/tools/pkcs15-tool.c | 2 +- 7 files changed, 10 insertions(+), 19 deletions(-) diff --git a/src/libopensc/card-openpgp.c b/src/libopensc/card-openpgp.c index 6774fe17..1e0773aa 100644 --- a/src/libopensc/card-openpgp.c +++ b/src/libopensc/card-openpgp.c @@ -883,7 +883,7 @@ static unsigned int pgp_strip_path(sc_card_t *card, const sc_path_t *path) { unsigned int start_point = 0; /* start_point will move through the path string */ - if (path->value == NULL || path->len == 0) + if (path->len == 0) return 0; /* Ignore 3F00 (MF) at the beginning */ diff --git a/src/libopensc/card-piv.c b/src/libopensc/card-piv.c index b1996541..ca3ebe44 100644 --- a/src/libopensc/card-piv.c +++ b/src/libopensc/card-piv.c @@ -784,8 +784,6 @@ static int piv_find_aid(sc_card_t * card, sc_file_t *aid_file) SC_FUNC_RETURN(card->ctx, SC_LOG_DEBUG_VERBOSE, SC_ERROR_NO_CARD_SUPPORT); card->ops->process_fci(card, aid_file, apdu.resp+2, apdu.resp[1]); - if (aid_file->name == NULL) - SC_FUNC_RETURN(card->ctx, SC_LOG_DEBUG_NORMAL, SC_ERROR_NO_CARD_SUPPORT); SC_FUNC_RETURN(card->ctx, SC_LOG_DEBUG_NORMAL, i); } diff --git a/src/libopensc/card-tcos.c b/src/libopensc/card-tcos.c index d6362a38..e41db406 100644 --- a/src/libopensc/card-tcos.c +++ b/src/libopensc/card-tcos.c @@ -161,8 +161,6 @@ static int tcos_construct_fci(const sc_file_t *file, /* Directory name */ if (file->type == SC_FILE_TYPE_DF) { if (file->namelen) { - if (file->namelen > 16 || !file->name) - return SC_ERROR_INVALID_ARGUMENTS; sc_asn1_put_tag(0x84, file->name, file->namelen, p, 16, &p); } diff --git a/src/pkcs15init/pkcs15-oberthur-awp.c b/src/pkcs15init/pkcs15-oberthur-awp.c index aa4e20a1..eb0642e5 100644 --- a/src/pkcs15init/pkcs15-oberthur-awp.c +++ b/src/pkcs15init/pkcs15-oberthur-awp.c @@ -782,10 +782,8 @@ awp_encode_key_info(struct sc_pkcs15_card *p15card, struct sc_pkcs15_object *obj if (obj->type == COSM_TYPE_PUBKEY_RSA || obj->type == COSM_TYPE_PRKEY_RSA) ki->flags |= COSM_GENERATED; - if (obj->label) { - ki->label.value = (unsigned char *)strdup(obj->label); - ki->label.len = strlen(obj->label); - } + ki->label.value = (unsigned char *)strdup(obj->label); + ki->label.len = strlen(obj->label); sc_debug(ctx, SC_LOG_DEBUG_NORMAL, "cosm_encode_key_info() label(%i):%s",ki->label.len, ki->label.value); /* @@ -1088,10 +1086,8 @@ awp_encode_data_info(struct sc_pkcs15_card *p15card, struct sc_pkcs15_object *ob di->flags = 0x0000; - if (obj->label) { - di->label.value = (unsigned char *)strdup(obj->label); - di->label.len = strlen(obj->label); - } + di->label.value = (unsigned char *)strdup(obj->label); + di->label.len = strlen(obj->label); di->app.len = strlen(data_info->app_label); if (di->app.len) { diff --git a/src/tools/pkcs11-tool.c b/src/tools/pkcs11-tool.c index 0d00c4a4..9a445896 100644 --- a/src/tools/pkcs11-tool.c +++ b/src/tools/pkcs11-tool.c @@ -4217,7 +4217,7 @@ static void test_kpgen_certwrite(CK_SLOT_ID slot, CK_SESSION_HANDLE session) return; tmp = getID(session, priv_key, (CK_ULONG *) &opt_object_id_len); - if (opt_object_id == NULL || opt_object_id_len == 0) { + if (opt_object_id_len == 0) { printf("ERR: newly generated private key has no (or an empty) CKA_ID\n"); return; } @@ -4372,7 +4372,7 @@ static void test_ec(CK_SLOT_ID slot, CK_SESSION_HANDLE session) return; tmp = getID(session, priv_key, (CK_ULONG *) &opt_object_id_len); - if (opt_object_id == NULL || opt_object_id_len == 0) { + if (opt_object_id_len == 0) { printf("ERR: newly generated private key has no (or an empty) CKA_ID\n"); return; } diff --git a/src/tools/pkcs15-init.c b/src/tools/pkcs15-init.c index 81d883c8..c3d6818e 100644 --- a/src/tools/pkcs15-init.c +++ b/src/tools/pkcs15-init.c @@ -1010,8 +1010,7 @@ is_cacert_already_present(struct sc_pkcs15init_certargs *args) if (!cinfo->authority) continue; - if (args->label && objs[i]->label - && strcmp(args->label, objs[i]->label)) + if (strcmp(args->label, objs[i]->label)) continue; /* XXX we should also match the usage field here */ @@ -2839,7 +2838,7 @@ static int verify_pin(struct sc_pkcs15_card *p15card, char *auth_id_str) if (opt_no_prompt) return SC_ERROR_OBJECT_NOT_FOUND; - if (pin_obj->label) + if (0 < strnlen(pin_obj->label, sizeof pin_obj->label)) snprintf(pin_label, sizeof(pin_label), "User PIN [%s]", pin_obj->label); else snprintf(pin_label, sizeof(pin_label), "User PIN"); diff --git a/src/tools/pkcs15-tool.c b/src/tools/pkcs15-tool.c index 8c980bb6..3a4fe622 100644 --- a/src/tools/pkcs15-tool.c +++ b/src/tools/pkcs15-tool.c @@ -455,7 +455,7 @@ static int list_data_objects(void) int idx; struct sc_pkcs15_data_info *cinfo = (struct sc_pkcs15_data_info *) objs[i]->data; - if (objs[i]->label) + if (0 < strnlen(objs[i]->label, sizeof objs[i]->label)) printf("Data object '%s'\n", objs[i]->label); else printf("Data object <%i>\n", i); From 6759c04b26127b62fbe01f368daa764e94ffe08d Mon Sep 17 00:00:00 2001 From: Frank Morgner Date: Wed, 28 Jan 2015 04:45:08 +0100 Subject: [PATCH 28/30] don't ignore errors --- src/libopensc/card-iasecc.c | 3 ++- src/libopensc/card-mcrd.c | 8 +++++--- src/libopensc/card-muscle.c | 4 +++- src/libopensc/card-oberthur.c | 3 ++- src/libopensc/card-piv.c | 12 +++++++----- src/libopensc/pkcs15-cache.c | 6 ++++-- src/libopensc/pkcs15-dnie.c | 4 +++- src/libopensc/pkcs15-sc-hsm.c | 9 ++++++--- src/libopensc/pkcs15-syn.c | 6 +++++- src/libopensc/pkcs15.c | 3 ++- src/libopensc/reader-pcsc.c | 10 +++++++--- src/pkcs11/framework-pkcs15.c | 10 ++++++---- src/pkcs15init/pkcs15-lib.c | 3 ++- src/tests/lottery.c | 8 ++++++-- src/tests/p15dump.c | 18 ++++++++++++------ src/tests/pintest.c | 22 ++++++++++++++++------ src/tools/openpgp-tool.c | 2 ++ src/tools/piv-tool.c | 12 +++++++++--- src/tools/pkcs11-tool.c | 3 ++- 19 files changed, 101 insertions(+), 45 deletions(-) diff --git a/src/libopensc/card-iasecc.c b/src/libopensc/card-iasecc.c index 318ff548..776bcf5a 100644 --- a/src/libopensc/card-iasecc.c +++ b/src/libopensc/card-iasecc.c @@ -414,7 +414,8 @@ iasecc_init_gemalto(struct sc_card *card) card->caps |= SC_CARD_CAP_USE_FCI_AC; sc_format_path("3F00", &path); - sc_select_file(card, &path, NULL); + rv = sc_select_file(card, &path, NULL); + LOG_TEST_RET(ctx, rv, "MF selection error"); rv = iasecc_parse_ef_atr(card); sc_log(ctx, "rv %i", rv); diff --git a/src/libopensc/card-mcrd.c b/src/libopensc/card-mcrd.c index 52d710f9..1f469b06 100644 --- a/src/libopensc/card-mcrd.c +++ b/src/libopensc/card-mcrd.c @@ -379,13 +379,13 @@ static int mcrd_init(sc_card_t * card) priv->curpathlen = 1; sc_format_path ("3f00", &tmppath); - sc_select_file (card, &tmppath, NULL); + r = sc_select_file (card, &tmppath, NULL); /* Not needed for the fixed EstEID profile */ if (!is_esteid_card(card)) load_special_files(card); - return SC_SUCCESS; + return r; } static int mcrd_finish(sc_card_t * card) @@ -1188,7 +1188,9 @@ static int mcrd_set_security_env(sc_card_t * card, /* Make sure we always start from MF */ sc_format_path ("3f00", &tmppath); - sc_select_file (card, &tmppath, NULL); + r = sc_select_file (card, &tmppath, NULL); + if (r < 0) + return r; /* We now know that cache is not valid */ select_esteid_df(card); switch (env->operation) { diff --git a/src/libopensc/card-muscle.c b/src/libopensc/card-muscle.c index 31304b8f..81a482e5 100644 --- a/src/libopensc/card-muscle.c +++ b/src/libopensc/card-muscle.c @@ -472,7 +472,9 @@ static int muscle_init(sc_card_t *card) card->caps |= SC_CARD_CAP_RNG; /* Card type detection */ - _sc_match_atr(card, muscle_atrs, &card->type); + if (SC_SUCCESS != _sc_match_atr(card, muscle_atrs, &card->type)) + SC_FUNC_RETURN(card->ctx, SC_LOG_DEBUG_NORMAL, SC_ERROR_INVALID_CARD); + if(card->type == SC_CARD_TYPE_MUSCLE_ETOKEN_72K) { card->caps |= SC_CARD_CAP_APDU_EXT; } diff --git a/src/libopensc/card-oberthur.c b/src/libopensc/card-oberthur.c index eb0820b1..f6a4d335 100644 --- a/src/libopensc/card-oberthur.c +++ b/src/libopensc/card-oberthur.c @@ -397,7 +397,8 @@ auth_process_fci(struct sc_card *card, struct sc_file *file, case 0x38: file->type = SC_FILE_TYPE_DF; file->size = attr[0]; - sc_file_set_type_attr(file,attr,attr_len); + if (SC_SUCCESS != sc_file_set_type_attr(file,attr,attr_len)) + SC_FUNC_RETURN(card->ctx, SC_LOG_DEBUG_NORMAL, SC_ERROR_UNKNOWN_DATA_RECEIVED); break; default: SC_FUNC_RETURN(card->ctx, SC_LOG_DEBUG_NORMAL, SC_ERROR_UNKNOWN_DATA_RECEIVED); diff --git a/src/libopensc/card-piv.c b/src/libopensc/card-piv.c index ca3ebe44..21f83934 100644 --- a/src/libopensc/card-piv.c +++ b/src/libopensc/card-piv.c @@ -1557,7 +1557,7 @@ static int piv_general_mutual_authenticate(sc_card_t *card, r = sc_lock(card); if (r != SC_SUCCESS) - goto err; + SC_FUNC_RETURN(card->ctx, SC_LOG_DEBUG_NORMAL, r); locked = 1; p = sbuf; @@ -1827,7 +1827,7 @@ static int piv_general_external_authenticate(sc_card_t *card, r = sc_lock(card); if (r != SC_SUCCESS) - goto err; + SC_FUNC_RETURN(card->ctx, SC_LOG_DEBUG_NORMAL, r); locked = 1; p = sbuf; @@ -2141,7 +2141,9 @@ static int piv_get_challenge(sc_card_t *card, u8 *rnd, size_t len) sc_debug(card->ctx, SC_LOG_DEBUG_NORMAL,"challenge len=%d",len); - sc_lock(card); + r = sc_lock(card); + if (r != SC_SUCCESS) + SC_FUNC_RETURN(card->ctx, SC_LOG_DEBUG_NORMAL, r); p = sbuf; *p++ = 0x7c; @@ -2177,9 +2179,9 @@ static int piv_get_challenge(sc_card_t *card, u8 *rnd, size_t len) rbuf = NULL; } - sc_unlock(card); + r = sc_unlock(card); - SC_FUNC_RETURN(card->ctx, SC_LOG_DEBUG_NORMAL, 0); + SC_FUNC_RETURN(card->ctx, SC_LOG_DEBUG_NORMAL, r); } diff --git a/src/libopensc/pkcs15-cache.c b/src/libopensc/pkcs15-cache.c index 2e18bf9f..b0c8269d 100644 --- a/src/libopensc/pkcs15-cache.c +++ b/src/libopensc/pkcs15-cache.c @@ -112,8 +112,10 @@ int sc_pkcs15_read_cached_file(struct sc_pkcs15_card *p15card, free(data); return SC_ERROR_FILE_NOT_FOUND; } - if (offset) - fseek(f, (long)offset, SEEK_SET); + if (offset) { + if (0 != fseek(f, (long)offset, SEEK_SET)) + return SC_ERROR_FILE_NOT_FOUND; + } if (data) *buf = data; got = fread(*buf, 1, count, f); diff --git a/src/libopensc/pkcs15-dnie.c b/src/libopensc/pkcs15-dnie.c index 1a98260d..acfce590 100644 --- a/src/libopensc/pkcs15-dnie.c +++ b/src/libopensc/pkcs15-dnie.c @@ -41,7 +41,9 @@ int dump_ef(sc_card_t * card, const char *path, u8 * buf, size_t * buf_len) int rv; sc_file_t *file = sc_file_new(); sc_format_path(path, &file->path); - sc_select_file(card, &file->path, &file); + rv = sc_select_file(card, &file->path, &file); + if (rv < 0) + return rv; if (file->size > *buf_len) return SC_ERROR_BUFFER_TOO_SMALL; rv = sc_read_binary(card, 0, buf, file->size, 0); diff --git a/src/libopensc/pkcs15-sc-hsm.c b/src/libopensc/pkcs15-sc-hsm.c index 0266637d..5e5fa570 100644 --- a/src/libopensc/pkcs15-sc-hsm.c +++ b/src/libopensc/pkcs15-sc-hsm.c @@ -504,9 +504,12 @@ static int sc_pkcs15emu_sc_hsm_add_pubkey(sc_pkcs15_card_t *p15card, sc_pkcs15_p memset(&pubkey_info, 0, sizeof(pubkey_info)); memset(&pubkey_obj, 0, sizeof(pubkey_obj)); - sc_pkcs15_encode_pubkey(ctx, &pubkey, &pubkey_obj.content.value, &pubkey_obj.content.len); - sc_pkcs15_encode_pubkey(ctx, &pubkey, &pubkey_info.direct.raw.value, &pubkey_info.direct.raw.len); - sc_pkcs15_encode_pubkey_as_spki(ctx, &pubkey, &pubkey_info.direct.spki.value, &pubkey_info.direct.spki.len); + r = sc_pkcs15_encode_pubkey(ctx, &pubkey, &pubkey_obj.content.value, &pubkey_obj.content.len); + LOG_TEST_RET(ctx, r, "Could not encode public key"); + r = sc_pkcs15_encode_pubkey(ctx, &pubkey, &pubkey_info.direct.raw.value, &pubkey_info.direct.raw.len); + LOG_TEST_RET(ctx, r, "Could not encode public key"); + r = sc_pkcs15_encode_pubkey_as_spki(ctx, &pubkey, &pubkey_info.direct.spki.value, &pubkey_info.direct.spki.len); + LOG_TEST_RET(ctx, r, "Could not encode public key"); pubkey_info.id = key_info->id; strlcpy(pubkey_obj.label, label, sizeof(pubkey_obj.label)); diff --git a/src/libopensc/pkcs15-syn.c b/src/libopensc/pkcs15-syn.c index ffbf642d..3ebbd633 100644 --- a/src/libopensc/pkcs15-syn.c +++ b/src/libopensc/pkcs15-syn.c @@ -271,7 +271,11 @@ static int parse_emu_block(sc_pkcs15_card_t *p15card, scconf_block *conf) /* try to get version of the driver/api */ get_version = (const char *(*)(void)) sc_dlsym(handle, "sc_driver_version"); if (get_version) { - sscanf(get_version(), "%u.%u.%u", &major, &minor, &fix); + if (3 != sscanf(get_version(), "%u.%u.%u", &major, &minor, &fix)) { + sc_debug(ctx, SC_LOG_DEBUG_NORMAL, + "unable to get modules version number\n"); + return SC_ERROR_INTERNAL; + } } if (!get_version || (major == 0 && minor <= 9 && fix < 3) < 0) { diff --git a/src/libopensc/pkcs15.c b/src/libopensc/pkcs15.c index 5219ae5b..caa7a598 100644 --- a/src/libopensc/pkcs15.c +++ b/src/libopensc/pkcs15.c @@ -1311,7 +1311,8 @@ __sc_pkcs15_search_objects(struct sc_pkcs15_card *p15card, unsigned int class_ma /* Enumerate the DF's, so p15card->obj_list is * populated. */ /* FIXME dont ignore errors */ - sc_pkcs15_parse_df(p15card, df); + if (SC_SUCCESS != sc_pkcs15_parse_df(p15card, df)) + continue; } /* And now loop over all objects */ diff --git a/src/libopensc/reader-pcsc.c b/src/libopensc/reader-pcsc.c index 46ef81f3..a54e2a8f 100644 --- a/src/libopensc/reader-pcsc.c +++ b/src/libopensc/reader-pcsc.c @@ -781,9 +781,13 @@ static unsigned long part10_detect_pace_capabilities(sc_reader_t *reader) goto err; if (priv->pace_ioctl) { - pcsc_internal_transmit(reader, pace_capabilities_buf, - sizeof pace_capabilities_buf, rbuf, &rcount, - priv->pace_ioctl); + if (0 > pcsc_internal_transmit(reader, pace_capabilities_buf, + sizeof pace_capabilities_buf, rbuf, &rcount, + priv->pace_ioctl)) { + sc_debug(reader->ctx, SC_LOG_DEBUG_NORMAL, + "PC/SC v2 part 10 amd1: Get PACE properties failed!"); + goto err; + } if (rcount != 7) goto err; diff --git a/src/pkcs11/framework-pkcs15.c b/src/pkcs11/framework-pkcs15.c index 45a57d7e..ff198e64 100644 --- a/src/pkcs11/framework-pkcs15.c +++ b/src/pkcs11/framework-pkcs15.c @@ -867,7 +867,7 @@ check_cert_data_read(struct pkcs15_fw_data *fw_data, struct pkcs15_cert_object * /* now that we have the cert and pub key, lets see if we can bind anything else */ pkcs15_bind_related_objects(fw_data); - return 0; + return rv; } @@ -2087,7 +2087,9 @@ pkcs15_create_secret_key(struct sc_pkcs11_slot *slot, struct sc_profile *profile return rv; /* CKA_TOKEN defaults to false */ - attr_find(pTemplate, ulCount, CKA_TOKEN, &_token, NULL); + rv = attr_find(pTemplate, ulCount, CKA_TOKEN, &_token, NULL); + if (rv != CKR_OK) + return rv; switch (key_type) { /* Only support GENERIC_SECRET for now */ @@ -3847,8 +3849,8 @@ pkcs15_pubkey_get_attribute(struct sc_pkcs11_session *session, void *object, CK_ case CKA_EC_PARAMS: case CKA_EC_POINT: if (pubkey->pub_data == NULL) - /* FIXME: check the return value? */ - check_cert_data_read(fw_data, cert); + if (SC_SUCCESS != check_cert_data_read(fw_data, cert)) + return sc_to_cryptoki_error(SC_ERROR_INTERNAL, "check_cert_data_read"); break; } diff --git a/src/pkcs15init/pkcs15-lib.c b/src/pkcs15init/pkcs15-lib.c index 5e61399f..7f01e455 100644 --- a/src/pkcs15init/pkcs15-lib.c +++ b/src/pkcs15init/pkcs15-lib.c @@ -2673,7 +2673,8 @@ sc_pkcs15init_update_any_df(struct sc_pkcs15_card *p15card, int update_odf = is_new, r = 0; LOG_FUNC_CALLED(ctx); - sc_profile_get_file_by_path(profile, &df->path, &file); + r = sc_profile_get_file_by_path(profile, &df->path, &file); + LOG_TEST_RET(ctx, r, "Failed get file path"); if (file == NULL) sc_select_file(card, &df->path, &file); diff --git a/src/tests/lottery.c b/src/tests/lottery.c index ecbce269..cfd0cc57 100644 --- a/src/tests/lottery.c +++ b/src/tests/lottery.c @@ -32,8 +32,12 @@ int main(int argc, char *argv[]) for (i = 0; i < 39; i++) { nbuf[i] = i + 1; } - if (c == 0) - gettimeofday(&tv1, NULL); + if (c == 0) { + if (0 != gettimeofday(&tv1, NULL)) { + fprintf(stderr, "gettimeofday() failed: %s\n", sc_strerror(r)); + return 1; + } + } sc_lock(card); r = sc_get_challenge(card, buf, 14); sc_unlock(card); diff --git a/src/tests/p15dump.c b/src/tests/p15dump.c index 433f5db7..24f19fcd 100644 --- a/src/tests/p15dump.c +++ b/src/tests/p15dump.c @@ -23,18 +23,21 @@ static int dump_objects(const char *what, int type) printf("\nEnumerating %s... ", what); fflush(stdout); - sc_lock(card); + if (SC_SUCCESS != sc_lock(card)) + return 1; count = sc_pkcs15_get_objects(p15card, type, NULL, 0); if (count < 0) { printf("failed.\n"); fprintf(stderr, "Error enumerating %s: %s\n", what, sc_strerror(count)); - sc_unlock(card); + if (SC_SUCCESS != sc_unlock(card)) + return 1; return 1; } if (count == 0) { printf("none found.\n"); - sc_unlock(card); + if (SC_SUCCESS != sc_unlock(card)) + return 1; return 0; } printf("%u found.\n", count); @@ -48,7 +51,8 @@ static int dump_objects(const char *what, int type) sc_test_print_object(objs[i]); } free(objs); - sc_unlock(card); + if (SC_SUCCESS != sc_unlock(card)) + return 1; return (count < 0) ? 1 : 0; } @@ -111,7 +115,8 @@ int main(int argc, char *argv[]) return 1; printf("Looking for a PKCS#15 compatible Smart Card... "); fflush(stdout); - sc_lock(card); + if (SC_SUCCESS != sc_lock(card)) + return 1; i = sc_pkcs15_bind(card, NULL, &p15card); /* Keep card locked to prevent useless calls to sc_logout */ if (i) { @@ -129,7 +134,8 @@ int main(int argc, char *argv[]) dump_unusedspace(); sc_pkcs15_unbind(p15card); - sc_unlock(card); + if (SC_SUCCESS != sc_unlock(card)) + return 1; sc_test_cleanup(); return 0; } diff --git a/src/tests/pintest.c b/src/tests/pintest.c index d913a726..e0bb0ec3 100644 --- a/src/tests/pintest.c +++ b/src/tests/pintest.c @@ -36,6 +36,10 @@ static int enum_pins(struct sc_pkcs15_object ***ret) return 0; } objs = calloc(n, sizeof(*objs)); + if (!objs) { + fprintf(stderr, "Not enough memory!\n"); + return 1; + } sc_pkcs15_get_objects(p15card, SC_PKCS15_TYPE_AUTH_PIN, objs, n); for (i = 0; i < n; i++) { sc_test_print_object(objs[i]); @@ -59,9 +63,11 @@ static int ask_and_verify_pin(struct sc_pkcs15_object *pin_obj) sprintf(prompt, "Please enter PIN code [%s]: ", pin_obj->label); pass = (u8 *) getpass(prompt); - sc_lock(card); + if (SC_SUCCESS != sc_lock(card)) + return 1; i = sc_pkcs15_verify_pin(p15card, pin_obj, pass, strlen((char *) pass)); - sc_unlock(card); + if (SC_SUCCESS != sc_unlock(card)) + return 1; if (i) { if (i == SC_ERROR_PIN_CODE_INCORRECT) fprintf(stderr, @@ -90,9 +96,11 @@ int main(int argc, char *argv[]) printf("Slot is capable of doing pinpad operations!\n"); printf("Looking for a PKCS#15 compatible Smart Card... "); fflush(stdout); - sc_lock(card); + if (SC_SUCCESS != sc_lock(card)) + return 1; i = sc_pkcs15_bind(card, NULL, &p15card); - sc_unlock(card); + if (SC_SUCCESS != sc_unlock(card)) + return 1; if (i) { fprintf(stderr, "failed: %s\n", sc_strerror(i)); sc_test_cleanup(); @@ -100,9 +108,11 @@ int main(int argc, char *argv[]) } printf("found.\n"); printf("Enumerating PIN codes...\n"); - sc_lock(card); + if (SC_SUCCESS != sc_lock(card)) + return 1; count = enum_pins(&objs); - sc_unlock(card); + if (SC_SUCCESS != sc_unlock(card)) + return 1; if (count < 0) { sc_pkcs15_unbind(p15card); sc_test_cleanup(); diff --git a/src/tools/openpgp-tool.c b/src/tools/openpgp-tool.c index a03fd375..fd9f4b3a 100644 --- a/src/tools/openpgp-tool.c +++ b/src/tools/openpgp-tool.c @@ -370,6 +370,8 @@ static int do_dump_do(sc_card_t *card, unsigned int tag) if(opt_raw) { r = 0; tmp = dup(fileno(stdout)); + if (tmp < 0) + return EXIT_FAILURE; fp = freopen(NULL, "wb", stdout); if(fp) { r = fwrite(buffer, sizeof(char), sizeof(buffer), fp); diff --git a/src/tools/piv-tool.c b/src/tools/piv-tool.c index 596b5a44..d0a6ba8e 100644 --- a/src/tools/piv-tool.c +++ b/src/tools/piv-tool.c @@ -119,7 +119,10 @@ static int load_object(const char * object_id, const char * object_file) return -1; } - stat(object_file, &stat_buf); + if (0 != stat(object_file, &stat_buf)) { + printf("unable to read file %s\n",object_file); + return -1; + } derlen = stat_buf.st_size; der = malloc(derlen); if (der == NULL) { @@ -173,13 +176,16 @@ static int load_cert(const char * cert_id, const char * cert_file, if (compress) { /* file is gziped already */ struct stat stat_buf; - stat(cert_file, &stat_buf); + if (0 != stat(cert_file, &stat_buf)) { + printf("unable to read file %s\n",cert_file); + return -1; + } derlen = stat_buf.st_size; der = malloc(derlen); if (der == NULL) { printf("file %s is too big, %lu\n", cert_file, (unsigned long)derlen); - return-1 ; + return -1 ; } if (1 != fread(der, derlen, 1, fp)) { printf("unable to read file %s\n",cert_file); diff --git a/src/tools/pkcs11-tool.c b/src/tools/pkcs11-tool.c index 9a445896..6eabea7d 100644 --- a/src/tools/pkcs11-tool.c +++ b/src/tools/pkcs11-tool.c @@ -3846,7 +3846,8 @@ static int wrap_unwrap(CK_SESSION_HANDLE session, return 1; } - EVP_DecryptInit(&seal_ctx, algo, key, iv); + if (!EVP_DecryptInit(&seal_ctx, algo, key, iv)) + return 1; len = sizeof(cleartext); EVP_DecryptUpdate(&seal_ctx, cleartext, &len, ciphered, ciphered_len); From 8e9a2361c6819b7ac991acfe8034a2d8f14dfe70 Mon Sep 17 00:00:00 2001 From: Viktor Tarasov Date: Mon, 2 Feb 2015 16:24:16 +0100 Subject: [PATCH 29/30] pkcs15-tool: print length of EC public key when this key is read from dedicated EF --- src/tools/pkcs15-tool.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/src/tools/pkcs15-tool.c b/src/tools/pkcs15-tool.c index 603a7534..9e9bd4ab 100644 --- a/src/tools/pkcs15-tool.c +++ b/src/tools/pkcs15-tool.c @@ -589,6 +589,7 @@ static void print_pubkey_info(const struct sc_pkcs15_object *obj) "neverExtract", "local" }; const unsigned int af_count = NELEMENTS(access_flags); + int have_path = (pubkey->path.len != 0) || (pubkey->path.aid.len != 0); printf("Public %s Key [%s]\n", types[7 & obj->type], obj->label); print_common_flags(obj); @@ -607,18 +608,29 @@ static void print_pubkey_info(const struct sc_pkcs15_object *obj) print_access_rules(obj->access_rules, SC_PKCS15_MAX_ACCESS_RULES); - if (pubkey->modulus_length) + if (pubkey->modulus_length) { printf("\tModLength : %lu\n", (unsigned long)pubkey->modulus_length); - else - printf("\tFieldLength : %lu\n", (unsigned long)pubkey->field_length); + } + else if (pubkey->field_length) { + printf("\tFieldLength : %lu\n", (unsigned long)pubkey->field_length); + } + else if (obj->type == SC_PKCS15_TYPE_PUBKEY_EC && have_path) { + sc_pkcs15_pubkey_t *pkey = NULL; + if (!sc_pkcs15_read_pubkey(p15card, obj, &pkey)) { + printf("\tFieldLength : %lu\n", (unsigned long)pkey->u.ec.params.field_length); + sc_pkcs15_free_pubkey(pkey); + } + } + printf("\tKey ref : %d (0x%X)\n", pubkey->key_reference, pubkey->key_reference); printf("\tNative : %s\n", pubkey->native ? "yes" : "no"); - if (pubkey->path.len || pubkey->path.aid.len) + if (have_path) printf("\tPath : %s\n", sc_print_path(&pubkey->path)); if (obj->auth_id.len != 0) printf("\tAuth ID : %s\n", sc_pkcs15_print_id(&obj->auth_id)); printf("\tID : %s\n", sc_pkcs15_print_id(&pubkey->id)); - printf("\tDirectValue : <%s>\n", obj->content.len ? "present" : "absent"); + if (!have_path || obj->content.len) + printf("\tDirectValue : <%s>\n", obj->content.len ? "present" : "absent"); } static int list_public_keys(void) From 3047fe2c3b366c6e15f256e3a604f03c77779413 Mon Sep 17 00:00:00 2001 From: Viktor Tarasov Date: Mon, 2 Feb 2015 16:25:54 +0100 Subject: [PATCH 30/30] log: implement 'dump OID' --- src/libopensc/log.c | 14 ++++++++++++++ src/libopensc/log.h | 2 +- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/libopensc/log.c b/src/libopensc/log.c index 70b67824..32f396d8 100644 --- a/src/libopensc/log.c +++ b/src/libopensc/log.c @@ -235,3 +235,17 @@ sc_dump_hex(const u8 * in, size_t count) return dump_buf; } + +char * +sc_dump_oid(const struct sc_object_id *oid) +{ + static char dump_buf[SC_MAX_OBJECT_ID_OCTETS * 20]; + size_t ii; + + memset(dump_buf, 0, sizeof(dump_buf)); + if (oid) + for (ii=0; iivalue[ii] != -1; ii++) + snprintf(dump_buf + strlen(dump_buf), sizeof(dump_buf) - strlen(dump_buf), "%s%i", (ii ? "." : ""), oid->value[ii]); + + return dump_buf; +} diff --git a/src/libopensc/log.h b/src/libopensc/log.h index d9b9c438..ccacefd5 100644 --- a/src/libopensc/log.h +++ b/src/libopensc/log.h @@ -60,7 +60,7 @@ void _sc_log(struct sc_context *ctx, const char *format, ...); void sc_hex_dump(struct sc_context *ctx, int level, const u8 * buf, size_t len, char *out, size_t outlen); char * sc_dump_hex(const u8 * in, size_t count); - +char * sc_dump_oid(const struct sc_object_id *oid); #define SC_FUNC_CALLED(ctx, level) do { \ sc_do_log(ctx, level, __FILE__, __LINE__, __FUNCTION__, "called\n"); \ } while (0)