From 8fe377e93b4b56060e5bbfb6f3142ceaeca744fa Mon Sep 17 00:00:00 2001 From: Frank Morgner Date: Fri, 25 May 2018 14:54:47 +0200 Subject: [PATCH 01/16] fixed out of bounds reads Thanks to Eric Sesterhenn from X41 D-SEC GmbH for reporting and suggesting security fixes. --- src/libopensc/asn1.c | 1 + src/libopensc/card-asepcos.c | 2 +- src/libopensc/card-authentic.c | 5 +++- src/libopensc/card-cac.c | 9 +++++-- src/libopensc/card-coolkey.c | 2 +- src/libopensc/card-entersafe.c | 8 +++--- src/libopensc/card-epass2003.c | 47 ++++++++++++++++++---------------- src/libopensc/card-gpk.c | 3 +++ src/libopensc/card-iasecc.c | 2 +- src/libopensc/card-oberthur.c | 7 +++++ src/libopensc/card-openpgp.c | 6 +++++ src/libopensc/card-piv.c | 9 ++++--- src/libopensc/card-rtecp.c | 2 +- src/libopensc/card-setcos.c | 18 +++++++++++-- src/libopensc/ef-gdo.c | 2 +- src/libopensc/pkcs15-itacns.c | 1 + src/libopensc/pkcs15-tcos.c | 8 +++--- src/tools/opensc-tool.c | 19 +++++++------- 18 files changed, 100 insertions(+), 51 deletions(-) diff --git a/src/libopensc/asn1.c b/src/libopensc/asn1.c index b1be88dc..e43c0f13 100644 --- a/src/libopensc/asn1.c +++ b/src/libopensc/asn1.c @@ -103,6 +103,7 @@ int sc_asn1_read_tag(const u8 ** buf, size_t buflen, unsigned int *cla_out, len = *p & 0x7f; if (*p++ & 0x80) { unsigned int a = 0; + left--; if (len > 4 || len > left) return SC_ERROR_INVALID_ASN1_OBJECT; left -= len; diff --git a/src/libopensc/card-asepcos.c b/src/libopensc/card-asepcos.c index 8055073c..5459d98f 100644 --- a/src/libopensc/card-asepcos.c +++ b/src/libopensc/card-asepcos.c @@ -169,7 +169,7 @@ static int asepcos_parse_sec_attr(sc_card_t *card, sc_file_t *file, const u8 *bu while (len != 0) { unsigned int amode, tlen = 3; - if (len < 5 && p[0] != 0x80 && p[1] != 0x01) { + if (len < 5 || p[0] != 0x80 || p[1] != 0x01) { sc_debug(card->ctx, SC_LOG_DEBUG_NORMAL, "invalid access mode encoding"); return SC_ERROR_INTERNAL; } diff --git a/src/libopensc/card-authentic.c b/src/libopensc/card-authentic.c index f72c2d20..9fbda94a 100644 --- a/src/libopensc/card-authentic.c +++ b/src/libopensc/card-authentic.c @@ -560,6 +560,9 @@ authentic_set_current_files(struct sc_card *card, struct sc_path *path, sc_file_dup(&card->cache.current_df, file); if (cur_df_path.len) { + if (cur_df_path.len + card->cache.current_df->path.len > sizeof card->cache.current_df->path.value + || cur_df_path.len > sizeof card->cache.current_df->path.value) + LOG_FUNC_RETURN(ctx, SC_ERROR_UNKNOWN_DATA_RECEIVED); memcpy(card->cache.current_df->path.value + cur_df_path.len, card->cache.current_df->path.value, card->cache.current_df->path.len); @@ -988,7 +991,7 @@ authentic_process_fci(struct sc_card *card, struct sc_file *file, } sc_log_hex(ctx, "ACL data", file->sec_attr, file->sec_attr_len); - for (ii = 0; ii < file->sec_attr_len / 2; ii++) { + for (ii = 0; ii < file->sec_attr_len / 2 && ii < sizeof ops_DF; ii++) { unsigned char op = file->type == SC_FILE_TYPE_DF ? ops_DF[ii] : ops_EF[ii]; unsigned char acl = *(file->sec_attr + ii*2); unsigned char cred_id = *(file->sec_attr + ii*2 + 1); diff --git a/src/libopensc/card-cac.c b/src/libopensc/card-cac.c index 4e853e5c..a30a0438 100644 --- a/src/libopensc/card-cac.c +++ b/src/libopensc/card-cac.c @@ -664,12 +664,17 @@ static int cac_read_binary(sc_card_t *card, unsigned int idx, cert_len = 0; cert_ptr = NULL; cert_type = 0; - for (tl_ptr = tl, val_ptr=val; tl_len >= 2; - val_len -= len, val_ptr += len, tl_len -= tl_head_len) { + for (tl_ptr = tl, val_ptr = val; tl_len >= 2; + val_len -= len, val_ptr += len, tl_len -= tl_head_len) { tl_start = tl_ptr; if (sc_simpletlv_read_tag(&tl_ptr, tl_len, &tag, &len) != SC_SUCCESS) break; tl_head_len = tl_ptr - tl_start; + + /* incomplete value */ + if (val_len < len) + break; + if (tag == CAC_TAG_CERTIFICATE) { cert_len = len; cert_ptr = val_ptr; diff --git a/src/libopensc/card-coolkey.c b/src/libopensc/card-coolkey.c index 4c966f64..3c57628b 100644 --- a/src/libopensc/card-coolkey.c +++ b/src/libopensc/card-coolkey.c @@ -1467,7 +1467,7 @@ coolkey_find_attribute(sc_card_t *card, sc_cardctl_coolkey_attribute_t *attribut for (i=0; i < attribute_count; i++) { size_t record_len = coolkey_get_attribute_record_len(attr, object_record_type, buf_len); /* make sure we have the complete record */ - if (buf_len < record_len) { + if (buf_len < record_len || record_len < 4) { return SC_ERROR_CORRUPTED_DATA; } /* does the attribute match the one we are looking for */ diff --git a/src/libopensc/card-entersafe.c b/src/libopensc/card-entersafe.c index d5264349..828b0305 100644 --- a/src/libopensc/card-entersafe.c +++ b/src/libopensc/card-entersafe.c @@ -1408,13 +1408,15 @@ static int entersafe_gen_key(sc_card_t *card, sc_entersafe_gen_key_data *data) data->modulus = malloc(len); if (!data->modulus) - SC_FUNC_RETURN(card->ctx, SC_LOG_DEBUG_VERBOSE,SC_ERROR_OUT_OF_MEMORY); + SC_FUNC_RETURN(card->ctx, SC_LOG_DEBUG_VERBOSE, SC_ERROR_OUT_OF_MEMORY); p=rbuf; - assert(*p=='E'); + if (*p!='E') + SC_FUNC_RETURN(card->ctx, SC_LOG_DEBUG_VERBOSE, SC_ERROR_INVALID_DATA); p+=2+p[1]; /* N */ - assert(*p=='N'); + if (*p!='N') + SC_FUNC_RETURN(card->ctx, SC_LOG_DEBUG_VERBOSE, SC_ERROR_INVALID_DATA); ++p; if(*p++>0x80) { diff --git a/src/libopensc/card-epass2003.c b/src/libopensc/card-epass2003.c index 88fe8275..cbe417cf 100644 --- a/src/libopensc/card-epass2003.c +++ b/src/libopensc/card-epass2003.c @@ -740,11 +740,11 @@ construct_mac_tlv(struct sc_card *card, unsigned char *apdu_buf, size_t data_tlv memcpy(mac_tlv + 2, &mac[mac_len - 16], 8); } else { - unsigned char iv[8] = { 0 }; + unsigned char iv[EVP_MAX_IV_LENGTH] = { 0 }; unsigned char tmp[8] = { 0 }; des_encrypt_cbc(exdata->sk_mac, 8, icv, apdu_buf, mac_len, mac); des_decrypt_cbc(&exdata->sk_mac[8], 8, iv, &mac[mac_len - 8], 8, tmp); - memset(iv, 0x00, 8); + memset(iv, 0x00, sizeof iv); des_encrypt_cbc(exdata->sk_mac, 8, iv, tmp, 8, mac_tlv + 2); } @@ -903,9 +903,9 @@ epass2003_sm_wrap_apdu(struct sc_card *card, struct sc_apdu *plain, struct sc_ap * SW12(TLV)=0x99|0x02|SW1+SW2 * MAC(TLV)=0x8e|0x08|MAC */ static int -decrypt_response(struct sc_card *card, unsigned char *in, unsigned char *out, size_t * out_len) +decrypt_response(struct sc_card *card, unsigned char *in, size_t inlen, unsigned char *out, size_t * out_len) { - size_t in_len; + size_t cipher_len; size_t i; unsigned char iv[16] = { 0 }; unsigned char plaintext[4096] = { 0 }; @@ -922,37 +922,40 @@ decrypt_response(struct sc_card *card, unsigned char *in, unsigned char *out, si /* parse cipher length */ if (0x01 == in[2] && 0x82 != in[1]) { - in_len = in[1]; + cipher_len = in[1]; i = 3; } else if (0x01 == in[3] && 0x81 == in[1]) { - in_len = in[2]; + cipher_len = in[2]; i = 4; } else if (0x01 == in[4] && 0x82 == in[1]) { - in_len = in[2] * 0x100; - in_len += in[3]; + cipher_len = in[2] * 0x100; + cipher_len += in[3]; i = 5; } else { return -1; } - /* decrypt */ - if (KEY_TYPE_AES == exdata->smtype) - aes128_decrypt_cbc(exdata->sk_enc, 16, iv, &in[i], in_len - 1, plaintext); - else - des3_decrypt_cbc(exdata->sk_enc, 16, iv, &in[i], in_len - 1, plaintext); - - /* unpadding */ - while (0x80 != plaintext[in_len - 2] && (in_len - 2 > 0)) - in_len--; - - if (2 == in_len) + if (cipher_len < 2 || i+cipher_len > inlen || cipher_len > sizeof plaintext) return -1; - memcpy(out, plaintext, in_len - 2); - *out_len = in_len - 2; + /* decrypt */ + if (KEY_TYPE_AES == exdata->smtype) + aes128_decrypt_cbc(exdata->sk_enc, 16, iv, &in[i], cipher_len - 1, plaintext); + else + des3_decrypt_cbc(exdata->sk_enc, 16, iv, &in[i], cipher_len - 1, plaintext); + + /* unpadding */ + while (0x80 != plaintext[cipher_len - 2] && (cipher_len - 2 > 0)) + cipher_len--; + + if (2 == cipher_len) + return -1; + + memcpy(out, plaintext, cipher_len - 2); + *out_len = cipher_len - 2; return 0; } @@ -974,7 +977,7 @@ epass2003_sm_unwrap_apdu(struct sc_card *card, struct sc_apdu *sm, struct sc_apd r = sc_check_sw(card, sm->sw1, sm->sw2); if (r == SC_SUCCESS) { if (exdata->sm) { - if (0 != decrypt_response(card, sm->resp, plain->resp, &len)) + if (0 != decrypt_response(card, sm->resp, sm->resplen, plain->resp, &len)) return SC_ERROR_CARD_CMD_FAILED; } else { diff --git a/src/libopensc/card-gpk.c b/src/libopensc/card-gpk.c index b3dd848c..c8d44691 100644 --- a/src/libopensc/card-gpk.c +++ b/src/libopensc/card-gpk.c @@ -409,6 +409,9 @@ gpk_parse_fileinfo(sc_card_t *card, if (sp[0] == 0x85) { unsigned int ac[3], n; + if (sp + 11 + 2*3 >= end) + break; + file->id = (sp[4] << 8) | sp[5]; file->size = (sp[8] << 8) | sp[9]; file->record_length = sp[7]; diff --git a/src/libopensc/card-iasecc.c b/src/libopensc/card-iasecc.c index 3e0d3acd..48d4cea5 100644 --- a/src/libopensc/card-iasecc.c +++ b/src/libopensc/card-iasecc.c @@ -2548,7 +2548,7 @@ iasecc_get_serialnr(struct sc_card *card, struct sc_serial_number *serial) if (card->type == SC_CARD_TYPE_IASECC_SAGEM) { /* 5A 0A 92 50 00 20 10 10 25 00 01 3F */ /* 00 02 01 01 02 50 00 13 */ - for (ii=0; ii < rbuf[1] - offs; ii++) + for (ii=0; (ii < rbuf[1] - offs) && (ii + offs + 2 < sizeof(rbuf)); ii++) *(card->serialnr.value + ii) = ((rbuf[ii + offs + 1] & 0x0F) << 4) + ((rbuf[ii + offs + 2] & 0xF0) >> 4) ; card->serialnr.len = ii; diff --git a/src/libopensc/card-oberthur.c b/src/libopensc/card-oberthur.c index 818a6a6c..112367b5 100644 --- a/src/libopensc/card-oberthur.c +++ b/src/libopensc/card-oberthur.c @@ -476,6 +476,9 @@ auth_select_file(struct sc_card *card, const struct sc_path *in_path, memcpy(&path, in_path, sizeof(struct sc_path)); + if (!auth_current_df) + return SC_ERROR_OBJECT_NOT_FOUND; + sc_log(card->ctx, "in_path; type=%d, path=%s, out %p", in_path->type, sc_print_path(in_path), file_out); sc_log(card->ctx, "current path; type=%d, path=%s", @@ -2113,6 +2116,10 @@ auth_read_binary(struct sc_card *card, unsigned int offset, bn[1].data = NULL; LOG_FUNC_CALLED(card->ctx); + + if (!auth_current_ef) + LOG_TEST_RET(card->ctx, SC_ERROR_INVALID_ARGUMENTS, "Invalid auth_current_ef"); + sc_log(card->ctx, "offset %i; size %"SC_FORMAT_LEN_SIZE_T"u; flags 0x%lX", offset, count, flags); diff --git a/src/libopensc/card-openpgp.c b/src/libopensc/card-openpgp.c index 594ff052..5fa38992 100644 --- a/src/libopensc/card-openpgp.c +++ b/src/libopensc/card-openpgp.c @@ -1061,6 +1061,9 @@ pgp_enumerate_blob(sc_card_t *card, pgp_blob_t *blob) const u8 *data = in; pgp_blob_t *new; + if (!in) + return SC_ERROR_OBJECT_NOT_VALID; + r = sc_asn1_read_tag(&data, blob->len - (in - blob->data), &cla, &tag, &len); if (r < 0 || data == NULL) { @@ -1069,6 +1072,9 @@ pgp_enumerate_blob(sc_card_t *card, pgp_blob_t *blob) return SC_ERROR_OBJECT_NOT_VALID; } + if (data + len > blob->data + blob->len) + return SC_ERROR_OBJECT_NOT_VALID; + /* undo ASN1's split of tag & class */ for (tmptag = tag; tmptag > 0x0FF; tmptag >>= 8) { cla <<= 8; diff --git a/src/libopensc/card-piv.c b/src/libopensc/card-piv.c index 13b0cc21..0cd3f5ad 100644 --- a/src/libopensc/card-piv.c +++ b/src/libopensc/card-piv.c @@ -573,7 +573,7 @@ static int piv_general_io(sc_card_t *card, int ins, int p1, int p2, * the buffer is bigger, so it will not produce "ASN1.tag too long!" */ body = rbuf; - if (sc_asn1_read_tag(&body, 0xffff, &cla_out, &tag_out, &bodylen) != SC_SUCCESS + if (sc_asn1_read_tag(&body, rbuflen, &cla_out, &tag_out, &bodylen) != SC_SUCCESS || body == NULL) { /* only early beta cards had this problem */ sc_log(card->ctx, "***** received buffer tag MISSING "); @@ -3033,12 +3033,13 @@ static int piv_match_card_continued(sc_card_t *card) * 73 66 74 65 20 63 64 31 34 34 * will check for 73 66 74 65 */ - else if (card->reader->atr_info.hist_bytes_len >= 4 && - !(memcmp(card->reader->atr_info.hist_bytes, "sfte", 4))) { + else if (card->reader->atr_info.hist_bytes_len >= 4 + && !(memcmp(card->reader->atr_info.hist_bytes, "sfte", 4))) { type = SC_CARD_TYPE_PIV_II_GI_DE; } - else if (card->reader->atr_info.hist_bytes[0] == 0x80u) { /* compact TLV */ + else if (card->reader->atr_info.hist_bytes_len > 0 + && card->reader->atr_info.hist_bytes[0] == 0x80u) { /* compact TLV */ size_t datalen; const u8 *data = sc_compacttlv_find_tag(card->reader->atr_info.hist_bytes + 1, card->reader->atr_info.hist_bytes_len - 1, diff --git a/src/libopensc/card-rtecp.c b/src/libopensc/card-rtecp.c index a1efe22e..32932ae7 100644 --- a/src/libopensc/card-rtecp.c +++ b/src/libopensc/card-rtecp.c @@ -275,7 +275,7 @@ static int rtecp_select_file(sc_card_t *card, set_acl_from_sec_attr(card, file); else r = SC_ERROR_UNKNOWN_DATA_RECEIVED; - if (r) + if (r && !file_out) sc_file_free(file); else { diff --git a/src/libopensc/card-setcos.c b/src/libopensc/card-setcos.c index f6dc0c61..98464c2e 100644 --- a/src/libopensc/card-setcos.c +++ b/src/libopensc/card-setcos.c @@ -789,6 +789,8 @@ static void parse_sec_attr_44(sc_file_t *file, const u8 *buf, size_t len) /* Check all sub-AC definitions within the total AC */ while (len > 1) { /* minimum length = 2 */ int iACLen = buf[iOffset] & 0x0F; + if ((size_t) iACLen > len) + break; iPinCount = -1; /* default no pin required */ iMethod = SC_AC_NONE; /* default no authentication required */ @@ -806,7 +808,10 @@ static void parse_sec_attr_44(sc_file_t *file, const u8 *buf, size_t len) /* Get KeyNumber if available */ if(iKeyLen) { - int iSC = buf[iOffset+iACLen]; + int iSC; + if (len < 1+iACLen) + break; + iSC = buf[iOffset+iACLen]; switch( (iSC>>5) & 0x03 ){ case 0: @@ -825,11 +830,15 @@ static void parse_sec_attr_44(sc_file_t *file, const u8 *buf, size_t len) /* Get PinNumber if available */ if (iACLen > (1+iParmLen+iKeyLen)) { /* check via total length if pin is present */ + if (len < 1+1+1+iParmLen) + break; iKeyRef = buf[iOffset+1+1+iParmLen]; /* PTL + AM-header + parameter-bytes */ iMethod = SC_AC_CHV; } /* Convert SETCOS command to OpenSC command group */ + if (len < 1+2) + break; switch(buf[iOffset+2]){ case 0x2A: /* crypto operation */ iOperation = SC_AC_OP_CRYPTO; @@ -863,7 +872,10 @@ static void parse_sec_attr_44(sc_file_t *file, const u8 *buf, size_t len) iPinCount = iACLen - 1; if (buf[iOffset] & 0x20) { - int iSC = buf[iOffset + iACLen]; + int iSC; + if (len < 1 + iACLen) + break; + iSC = buf[iOffset + iACLen]; switch( (iSC>>5) & 0x03 ) { case 0: @@ -884,6 +896,8 @@ static void parse_sec_attr_44(sc_file_t *file, const u8 *buf, size_t len) /* Pin present ? */ if ( iPinCount > 0 ) { + if (len < 1 + 2) + break; iKeyRef = buf[iOffset + 2]; /* pin ref */ iMethod = SC_AC_CHV; } diff --git a/src/libopensc/ef-gdo.c b/src/libopensc/ef-gdo.c index 39dea2e5..7888fd49 100644 --- a/src/libopensc/ef-gdo.c +++ b/src/libopensc/ef-gdo.c @@ -72,7 +72,7 @@ sc_parse_ef_gdo_content(const unsigned char *gdo, size_t gdo_len, } p += tag_len; - left -= (p - gdo); + left = gdo_len - (p - gdo); } if (!iccsn_found && iccsn_len) diff --git a/src/libopensc/pkcs15-itacns.c b/src/libopensc/pkcs15-itacns.c index eef8ac1f..f0f39590 100644 --- a/src/libopensc/pkcs15-itacns.c +++ b/src/libopensc/pkcs15-itacns.c @@ -550,6 +550,7 @@ static int itacns_add_data_files(sc_pkcs15_card_t *p15card) sc_debug(p15card->card->ctx, SC_LOG_DEBUG_NORMAL, "Could not read EF_DatiPersonali: " "keeping generic card name"); + return SC_SUCCESS; } { diff --git a/src/libopensc/pkcs15-tcos.c b/src/libopensc/pkcs15-tcos.c index 74bb32a6..7732ad08 100644 --- a/src/libopensc/pkcs15-tcos.c +++ b/src/libopensc/pkcs15-tcos.c @@ -132,7 +132,7 @@ static int insert_key( int i, rec_no=0; if(prkey_info.path.len>=2) prkey_info.path.len-=2; sc_append_file_id(&prkey_info.path, 0x5349); - if(sc_select_file(card, &prkey_info.path, NULL)!=SC_SUCCESS){ + if(sc_select_file(card, &prkey_info.path, NULL)!=SC_SUCCESS || !f->prop_attr){ sc_debug(ctx, SC_LOG_DEBUG_NORMAL, "Select(%s) failed\n", sc_print_path(&prkey_info.path)); @@ -157,7 +157,8 @@ static int insert_key( if(buf[i]==0xB8) can_crypt++; } } else { - if(sc_select_file(card, &prkey_info.path, &f)!=SC_SUCCESS){ + if(sc_select_file(card, &prkey_info.path, &f)!=SC_SUCCESS + || !f->prop_attr || f->prop_attr_len < 2){ sc_debug(ctx, SC_LOG_DEBUG_NORMAL, "Select(%s) failed\n", sc_print_path(&prkey_info.path)); @@ -245,7 +246,8 @@ static int insert_pin( return 1; } } else { - if(sc_select_file(card, &pin_info.path, &f)!=SC_SUCCESS){ + if(sc_select_file(card, &pin_info.path, &f)!=SC_SUCCESS + || !f->prop_attr || f->prop_attr_len < 4){ sc_debug(ctx, SC_LOG_DEBUG_NORMAL,"Select(%s) failed\n", path); return 1; } diff --git a/src/tools/opensc-tool.c b/src/tools/opensc-tool.c index 68c72805..2f121acf 100644 --- a/src/tools/opensc-tool.c +++ b/src/tools/opensc-tool.c @@ -457,7 +457,7 @@ static int enum_dir(sc_path_t path, int depth) { sc_file_t *file; int r, file_type; - u8 files[SC_MAX_APDU_BUFFER_SIZE]; + u8 files[SC_MAX_EXT_APDU_BUFFER_SIZE]; r = sc_lock(card); if (r == SC_SUCCESS) @@ -483,15 +483,16 @@ static int enum_dir(sc_path_t path, int depth) } if (r == 0) { printf("Empty directory\n"); - } else - for (i = 0; i < r/2; i++) { - sc_path_t tmppath; + } else { + for (i = 0; i < r/2; i++) { + sc_path_t tmppath; - memset(&tmppath, 0, sizeof(tmppath)); - memcpy(&tmppath, &path, sizeof(path)); - memcpy(tmppath.value + tmppath.len, files + 2*i, 2); - tmppath.len += 2; - enum_dir(tmppath, depth + 1); + memset(&tmppath, 0, sizeof(tmppath)); + memcpy(&tmppath, &path, sizeof(path)); + memcpy(tmppath.value + tmppath.len, files + 2*i, 2); + tmppath.len += 2; + enum_dir(tmppath, depth + 1); + } } } return 0; From 360e95d45ac4123255a4c796db96337f332160ad Mon Sep 17 00:00:00 2001 From: Frank Morgner Date: Fri, 25 May 2018 23:34:14 +0200 Subject: [PATCH 02/16] fixed out of bounds writes Thanks to Eric Sesterhenn from X41 D-SEC GmbH for reporting the problems. --- src/libopensc/card-cac.c | 2 +- src/libopensc/card-epass2003.c | 3 ++- src/libopensc/card-muscle.c | 7 +++++-- src/libopensc/card-tcos.c | 6 +++--- src/libopensc/pkcs15-esteid.c | 2 +- src/libopensc/pkcs15-gemsafeV1.c | 2 +- src/libopensc/pkcs15-sc-hsm.c | 14 ++++++++------ src/libopensc/sc.c | 2 +- src/tools/cryptoflex-tool.c | 5 +++-- src/tools/egk-tool.c | 2 +- src/tools/util.c | 5 +++-- 11 files changed, 29 insertions(+), 21 deletions(-) diff --git a/src/libopensc/card-cac.c b/src/libopensc/card-cac.c index a30a0438..756788a0 100644 --- a/src/libopensc/card-cac.c +++ b/src/libopensc/card-cac.c @@ -794,7 +794,7 @@ static int cac_get_serial_nr_from_CUID(sc_card_t* card, sc_serial_number_t* seri } if (priv->cac_id_len) { serial->len = MIN(priv->cac_id_len, SC_MAX_SERIALNR); - memcpy(serial->value, priv->cac_id, priv->cac_id_len); + memcpy(serial->value, priv->cac_id, serial->len); SC_FUNC_RETURN(card->ctx, SC_LOG_DEBUG_NORMAL, SC_SUCCESS); } SC_FUNC_RETURN(card->ctx, SC_LOG_DEBUG_NORMAL, SC_ERROR_FILE_NOT_FOUND); diff --git a/src/libopensc/card-epass2003.c b/src/libopensc/card-epass2003.c index cbe417cf..65a58f67 100644 --- a/src/libopensc/card-epass2003.c +++ b/src/libopensc/card-epass2003.c @@ -951,7 +951,7 @@ decrypt_response(struct sc_card *card, unsigned char *in, size_t inlen, unsigned while (0x80 != plaintext[cipher_len - 2] && (cipher_len - 2 > 0)) cipher_len--; - if (2 == cipher_len) + if (2 == cipher_len || *out_len < cipher_len - 2) return -1; memcpy(out, plaintext, cipher_len - 2); @@ -977,6 +977,7 @@ epass2003_sm_unwrap_apdu(struct sc_card *card, struct sc_apdu *sm, struct sc_apd r = sc_check_sw(card, sm->sw1, sm->sw2); if (r == SC_SUCCESS) { if (exdata->sm) { + len = plain->resplen; if (0 != decrypt_response(card, sm->resp, sm->resplen, plain->resp, &len)) return SC_ERROR_CARD_CMD_FAILED; } diff --git a/src/libopensc/card-muscle.c b/src/libopensc/card-muscle.c index fd148e30..d247efab 100644 --- a/src/libopensc/card-muscle.c +++ b/src/libopensc/card-muscle.c @@ -518,7 +518,9 @@ static int muscle_list_files(sc_card_t *card, u8 *buf, size_t bufLen) mscfs_check_cache(priv->fs); for(x = 0; x < fs->cache.size; x++) { - u8* oid= fs->cache.array[x].objectId.id; + u8* oid = fs->cache.array[x].objectId.id; + if (bufLen < 2) + break; sc_debug(card->ctx, SC_LOG_DEBUG_NORMAL, "FILE: %02X%02X%02X%02X\n", oid[0],oid[1],oid[2],oid[3]); @@ -527,7 +529,8 @@ static int muscle_list_files(sc_card_t *card, u8 *buf, size_t bufLen) buf[1] = oid[3]; if(buf[0] == 0x00 && buf[1] == 0x00) continue; /* No directories/null names outside of root */ buf += 2; - count+=2; + count += 2; + bufLen -= 2; } } return count; diff --git a/src/libopensc/card-tcos.c b/src/libopensc/card-tcos.c index 40eac877..3c86b1f4 100644 --- a/src/libopensc/card-tcos.c +++ b/src/libopensc/card-tcos.c @@ -408,7 +408,7 @@ static int tcos_select_file(sc_card_t *card, file->path = *in_path; for(i=2; i+1id = (d[0]<<8) | d[1]; break; case 0x84: - memcpy(file->name, d, len); - file->namelen = len; + file->namelen = MIN(sizeof file->name, len); + memcpy(file->name, d, file->namelen); break; case 0x86: sc_file_set_sec_attr(file, d, len); diff --git a/src/libopensc/pkcs15-esteid.c b/src/libopensc/pkcs15-esteid.c index b3cf5178..2b8d66b9 100644 --- a/src/libopensc/pkcs15-esteid.c +++ b/src/libopensc/pkcs15-esteid.c @@ -79,7 +79,7 @@ sc_pkcs15emu_esteid_init (sc_pkcs15_card_t * p15card) /* read the serial (document number) */ r = sc_read_record (card, SC_ESTEID_PD_DOCUMENT_NR, buff, sizeof(buff), SC_RECORD_BY_REC_NR); SC_TEST_RET(card->ctx, SC_LOG_DEBUG_NORMAL, r, "read document number failed"); - buff[r] = '\0'; + buff[MIN((size_t) r, (sizeof buff)-1)] = '\0'; set_string (&p15card->tokeninfo->serial_number, (const char *) buff); p15card->tokeninfo->flags = SC_PKCS15_TOKEN_PRN_GENERATION diff --git a/src/libopensc/pkcs15-gemsafeV1.c b/src/libopensc/pkcs15-gemsafeV1.c index a8162b9d..3b220f7a 100644 --- a/src/libopensc/pkcs15-gemsafeV1.c +++ b/src/libopensc/pkcs15-gemsafeV1.c @@ -208,7 +208,7 @@ static int gemsafe_get_cert_len(sc_card_t *card) * the private key. */ ind = 2; /* skip length */ - while (ibuf[ind] == 0x01) { + while (ibuf[ind] == 0x01 && i < gemsafe_cert_max) { if (ibuf[ind+1] == 0xFE) { gemsafe_prkeys[i].ref = ibuf[ind+4]; sc_log(card->ctx, "Key container %d is allocated and uses key_ref %d", diff --git a/src/libopensc/pkcs15-sc-hsm.c b/src/libopensc/pkcs15-sc-hsm.c index 305572cb..1391bea1 100644 --- a/src/libopensc/pkcs15-sc-hsm.c +++ b/src/libopensc/pkcs15-sc-hsm.c @@ -837,12 +837,14 @@ static int sc_pkcs15emu_sc_hsm_init (sc_pkcs15_card_t * p15card) r = read_file(p15card, (u8 *) "\x2F\x02", efbin, &len, 1); LOG_TEST_RET(card->ctx, r, "Skipping optional EF.C_DevAut"); - /* save EF_C_DevAut for further use */ - ptr = realloc(priv->EF_C_DevAut, len); - if (ptr) { - memcpy(ptr, efbin, len); - priv->EF_C_DevAut = ptr; - priv->EF_C_DevAut_len = len; + if (len > 0) { + /* save EF_C_DevAut for further use */ + ptr = realloc(priv->EF_C_DevAut, len); + if (ptr) { + memcpy(ptr, efbin, len); + priv->EF_C_DevAut = ptr; + priv->EF_C_DevAut_len = len; + } } ptr = efbin; diff --git a/src/libopensc/sc.c b/src/libopensc/sc.c index af71cafa..53fefb3e 100644 --- a/src/libopensc/sc.c +++ b/src/libopensc/sc.c @@ -628,7 +628,7 @@ int sc_file_set_sec_attr(sc_file_t *file, const u8 *sec_attr, return SC_ERROR_INVALID_ARGUMENTS; } - if (sec_attr == NULL) { + if (sec_attr == NULL || sec_attr_len) { if (file->sec_attr != NULL) free(file->sec_attr); file->sec_attr = NULL; diff --git a/src/tools/cryptoflex-tool.c b/src/tools/cryptoflex-tool.c index 21b0baa1..662a0282 100644 --- a/src/tools/cryptoflex-tool.c +++ b/src/tools/cryptoflex-tool.c @@ -21,6 +21,7 @@ #include "config.h" #include "libopensc/sc-ossl-compat.h" +#include "libopensc/internal.h" #include #include #include @@ -331,7 +332,7 @@ static int read_public_key(RSA *rsa) fprintf(stderr, "Unable to select public key file: %s\n", sc_strerror(r)); return 2; } - bufsize = file->size; + bufsize = MIN(file->size, sizeof buf); sc_file_free(file); r = sc_read_binary(card, 0, buf, bufsize, 0); if (r < 0) { @@ -382,7 +383,7 @@ static int read_private_key(RSA *rsa) e = sc_file_get_acl_entry(file, SC_AC_OP_READ); if (e == NULL || e->method == SC_AC_NEVER) return 10; - bufsize = file->size; + bufsize = MIN(file->size, sizeof buf); sc_file_free(file); r = sc_read_binary(card, 0, buf, bufsize, 0); if (r < 0) { diff --git a/src/tools/egk-tool.c b/src/tools/egk-tool.c index 0a8834b6..31360833 100644 --- a/src/tools/egk-tool.c +++ b/src/tools/egk-tool.c @@ -149,7 +149,7 @@ int read_file(struct sc_card *card, char *str_path, unsigned char **data, size_t goto err; } - len = file ? file->size : 4096; + len = file && file->size > 0 ? file->size : 4096; p = realloc(*data, len); if (!p) { goto err; diff --git a/src/tools/util.c b/src/tools/util.c index 6b8743e2..e49647a4 100644 --- a/src/tools/util.c +++ b/src/tools/util.c @@ -339,10 +339,11 @@ const char * util_acl_to_str(const sc_acl_entry_t *e) strcpy(buf, "????"); break; } - strcat(line, buf); - strcat(line, " "); + strncat(line, buf, sizeof line); + strncat(line, " ", sizeof line); e = e->next; } + line[(sizeof line)-1] = '\0'; /* make sure it's NUL terminated */ line[strlen(line)-1] = 0; /* get rid of trailing space */ return line; } From ffe38fd87fc06879924759ca2e25eabb47ed6f0d Mon Sep 17 00:00:00 2001 From: Frank Morgner Date: Sun, 27 May 2018 00:14:01 +0200 Subject: [PATCH 03/16] sc_asn1_read_tag: fixed tracking of consumed bytes fixes return buffers that are outside the allocated memory space --- src/libopensc/asn1.c | 36 +++++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/src/libopensc/asn1.c b/src/libopensc/asn1.c index e43c0f13..3262ed80 100644 --- a/src/libopensc/asn1.c +++ b/src/libopensc/asn1.c @@ -65,15 +65,17 @@ int sc_asn1_read_tag(const u8 ** buf, size_t buflen, unsigned int *cla_out, size_t left = buflen, len; unsigned int cla, tag, i; - if (left < 2) - return SC_ERROR_INVALID_ASN1_OBJECT; *buf = NULL; + + if (left == 0) + return SC_ERROR_INVALID_ASN1_OBJECT; if (*p == 0xff || *p == 0) { /* end of data reached */ *taglen = 0; *tag_out = SC_ASN1_TAG_EOC; return SC_SUCCESS; } + /* parse tag byte(s) * Resulted tag is presented by integer that has not to be * confused with the 'tag number' part of ASN.1 tag. @@ -86,31 +88,35 @@ int sc_asn1_read_tag(const u8 ** buf, size_t buflen, unsigned int *cla_out, /* high tag number */ size_t n = SC_ASN1_TAGNUM_SIZE - 1; /* search the last tag octet */ - while (left-- != 0 && n != 0) { + do { + if (left == 0 || n == 0) + /* either an invalid tag or it doesn't fit in + * unsigned int */ + return SC_ERROR_INVALID_ASN1_OBJECT; tag <<= 8; tag |= *p; - if ((*p++ & 0x80) == 0) - break; + p++; + left--; n--; - } - if (left == 0 || n == 0) - /* either an invalid tag or it doesn't fit in - * unsigned int */ - return SC_ERROR_INVALID_ASN1_OBJECT; + } while (tag & 0x80); } /* parse length byte(s) */ - len = *p & 0x7f; - if (*p++ & 0x80) { + if (left == 0) + return SC_ERROR_INVALID_ASN1_OBJECT; + len = *p; + p++; + left--; + if (len & 0x80) { + len &= 0x7f; unsigned int a = 0; - left--; - if (len > 4 || len > left) + if (len > sizeof a || len > left) return SC_ERROR_INVALID_ASN1_OBJECT; - left -= len; for (i = 0; i < len; i++) { a <<= 8; a |= *p; p++; + left--; } len = a; } From 83f45cda2af16b65264103fbe0394fd422f0120d Mon Sep 17 00:00:00 2001 From: Frank Morgner Date: Sun, 27 May 2018 00:38:37 +0200 Subject: [PATCH 04/16] Added bounds checking to sc_simpletlv_read_tag() - Logic is identical to sc_asn1_read_tag() - Fixes out of bounds access e.g. in cac_parse_CCC --- src/libopensc/card-setcos.c | 14 +++++++------- src/libopensc/errors.c | 2 ++ src/libopensc/errors.h | 2 ++ src/libopensc/simpletlv.c | 33 ++++++++++++++++++++++----------- src/tools/util.c | 5 +++-- 5 files changed, 36 insertions(+), 20 deletions(-) diff --git a/src/libopensc/card-setcos.c b/src/libopensc/card-setcos.c index 98464c2e..a8ca012f 100644 --- a/src/libopensc/card-setcos.c +++ b/src/libopensc/card-setcos.c @@ -788,8 +788,8 @@ static void parse_sec_attr_44(sc_file_t *file, const u8 *buf, size_t len) /* Check all sub-AC definitions within the total AC */ while (len > 1) { /* minimum length = 2 */ - int iACLen = buf[iOffset] & 0x0F; - if ((size_t) iACLen > len) + size_t iACLen = buf[iOffset] & 0x0F; + if (iACLen > len) break; iPinCount = -1; /* default no pin required */ @@ -797,8 +797,8 @@ static void parse_sec_attr_44(sc_file_t *file, const u8 *buf, size_t len) if (buf[iOffset] & 0X80) { /* AC in adaptive coding */ /* Evaluates only the command-byte, not the optional P1/P2/Option bytes */ - int iParmLen = 1; /* command-byte is always present */ - int iKeyLen = 0; /* Encryption key is optional */ + size_t iParmLen = 1; /* command-byte is always present */ + size_t iKeyLen = 0; /* Encryption key is optional */ if (buf[iOffset] & 0x20) iKeyLen++; if (buf[iOffset+1] & 0x40) iParmLen++; @@ -809,7 +809,7 @@ static void parse_sec_attr_44(sc_file_t *file, const u8 *buf, size_t len) /* Get KeyNumber if available */ if(iKeyLen) { int iSC; - if (len < 1+iACLen) + if (len < 1+(size_t)iACLen) break; iSC = buf[iOffset+iACLen]; @@ -830,7 +830,7 @@ static void parse_sec_attr_44(sc_file_t *file, const u8 *buf, size_t len) /* Get PinNumber if available */ if (iACLen > (1+iParmLen+iKeyLen)) { /* check via total length if pin is present */ - if (len < 1+1+1+iParmLen) + if (len < 1+1+1+(size_t)iParmLen) break; iKeyRef = buf[iOffset+1+1+iParmLen]; /* PTL + AM-header + parameter-bytes */ iMethod = SC_AC_CHV; @@ -873,7 +873,7 @@ static void parse_sec_attr_44(sc_file_t *file, const u8 *buf, size_t len) if (buf[iOffset] & 0x20) { int iSC; - if (len < 1 + iACLen) + if (len < 1 + (size_t)iACLen) break; iSC = buf[iOffset + iACLen]; diff --git a/src/libopensc/errors.c b/src/libopensc/errors.c index b6c6eb48..354e0a6e 100644 --- a/src/libopensc/errors.c +++ b/src/libopensc/errors.c @@ -104,6 +104,8 @@ const char *sc_strerror(int error) "Unable to load external module", "EF offset too large", "Not implemented" + "Invalid Simple TLV object", + "Premature end of Simple TLV stream", }; const int int_base = -SC_ERROR_INTERNAL; diff --git a/src/libopensc/errors.h b/src/libopensc/errors.h index 183b0758..e0330a93 100644 --- a/src/libopensc/errors.h +++ b/src/libopensc/errors.h @@ -95,6 +95,8 @@ extern "C" { #define SC_ERROR_CANNOT_LOAD_MODULE -1414 #define SC_ERROR_OFFSET_TOO_LARGE -1415 #define SC_ERROR_NOT_IMPLEMENTED -1416 +#define SC_ERROR_INVALID_TLV_OBJECT -1417 +#define SC_ERROR_TLV_END_OF_CONTENTS -1418 /* Relating to PKCS #15 init stuff */ #define SC_ERROR_PKCS15INIT -1500 diff --git a/src/libopensc/simpletlv.c b/src/libopensc/simpletlv.c index ab0401b5..67a08da2 100644 --- a/src/libopensc/simpletlv.c +++ b/src/libopensc/simpletlv.c @@ -74,27 +74,38 @@ sc_simpletlv_put_tag(u8 tag, size_t datalen, u8 *out, size_t outlen, u8 **ptr) int sc_simpletlv_read_tag(u8 **buf, size_t buflen, u8 *tag_out, size_t *taglen) { - size_t len; + u8 tag; + size_t left = buflen, len; u8 *p = *buf; - if (buflen < 2) { - *buf = p+buflen; - return SC_ERROR_INVALID_ARGUMENTS; - } + *buf = NULL; + + if (left < 2) { + return SC_ERROR_INVALID_TLV_OBJECT; + } + tag = *p; + p++; + len = *p; + p++; + left -= 2; - *tag_out = *p++; - len = *p++; if (len == 0xff) { /* don't crash on bad data */ - if (buflen < 4) { - *taglen = 0; - return SC_ERROR_INVALID_ARGUMENTS; + if (left < 2) { + return SC_ERROR_INVALID_TLV_OBJECT; } /* skip two bytes (the size) */ len = lebytes2ushort(p); - p+=2; + p += 2; + left -= 2; } + + *tag_out = tag; *taglen = len; *buf = p; + + if (len > left) + return SC_ERROR_TLV_END_OF_CONTENTS; + return SC_SUCCESS; } diff --git a/src/tools/util.c b/src/tools/util.c index e49647a4..a3cbff7a 100644 --- a/src/tools/util.c +++ b/src/tools/util.c @@ -31,6 +31,7 @@ #include #include "util.h" #include "ui/notify.h" +#include "common/compat_strlcat.h" int is_string_valid_atr(const char *atr_str) @@ -339,8 +340,8 @@ const char * util_acl_to_str(const sc_acl_entry_t *e) strcpy(buf, "????"); break; } - strncat(line, buf, sizeof line); - strncat(line, " ", sizeof line); + strlcat(line, buf, sizeof line); + strlcat(line, " ", sizeof line); e = e->next; } line[(sizeof line)-1] = '\0'; /* make sure it's NUL terminated */ From d5d15105dd4cc91e54c86efbd75f07c6f21bb281 Mon Sep 17 00:00:00 2001 From: Jakub Jelen Date: Tue, 19 Jun 2018 14:54:31 +0200 Subject: [PATCH 05/16] cac: Ignore end of content errors (#7) The CAC buffers are split to separate TL and V buffers so we need to ignore this error --- src/libopensc/card-cac.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/libopensc/card-cac.c b/src/libopensc/card-cac.c index 756788a0..a42df928 100644 --- a/src/libopensc/card-cac.c +++ b/src/libopensc/card-cac.c @@ -637,7 +637,8 @@ static int cac_read_binary(sc_card_t *card, unsigned int idx, val_len -= len, tlv_len -= len, val_ptr += len, tlv_ptr += len) { /* get the tag and the length */ tl_start = tl_ptr; - if (sc_simpletlv_read_tag(&tl_ptr, tl_len, &tag, &len) != SC_SUCCESS) + r = sc_simpletlv_read_tag(&tl_ptr, tl_len, &tag, &len); + if (r != SC_SUCCESS && r != SC_ERROR_TLV_END_OF_CONTENTS) break; tl_head_len = (tl_ptr - tl_start); sc_simpletlv_put_tag(tag, len, tlv_ptr, tlv_len, &tlv_ptr); @@ -646,6 +647,8 @@ static int cac_read_binary(sc_card_t *card, unsigned int idx, /* don't crash on bad data */ if (val_len < len) { + sc_log(card->ctx, "Received too long value %"SC_FORMAT_LEN_SIZE_T"u, " + "while only %"SC_FORMAT_LEN_SIZE_T"u left. Truncating", len, val_len); len = val_len; } /* if we run out of return space, truncate */ @@ -667,13 +670,17 @@ static int cac_read_binary(sc_card_t *card, unsigned int idx, for (tl_ptr = tl, val_ptr = val; tl_len >= 2; val_len -= len, val_ptr += len, tl_len -= tl_head_len) { tl_start = tl_ptr; - if (sc_simpletlv_read_tag(&tl_ptr, tl_len, &tag, &len) != SC_SUCCESS) + r = sc_simpletlv_read_tag(&tl_ptr, tl_len, &tag, &len); + if (r != SC_SUCCESS && r != SC_ERROR_TLV_END_OF_CONTENTS) break; tl_head_len = tl_ptr - tl_start; /* incomplete value */ - if (val_len < len) + if (val_len < len) { + sc_log(card->ctx, "Read incomplete value %"SC_FORMAT_LEN_SIZE_T"u, " + "while only %"SC_FORMAT_LEN_SIZE_T"u left", len, val_len); break; + } if (tag == CAC_TAG_CERTIFICATE) { cert_len = len; @@ -687,9 +694,6 @@ static int cac_read_binary(sc_card_t *card, unsigned int idx, if (tag == CAC_TAG_MSCUID) { sc_log_hex(card->ctx, "MSCUID", val_ptr, len); } - if ((val_len < len) || (tl_len < tl_head_len)) { - break; - } } /* if the info byte is 1, then the cert is compressed, decompress it */ if ((cert_type & 0x3) == 1) { From 3e5a9a42c355f4ef64c40347d9046d0e063e589b Mon Sep 17 00:00:00 2001 From: Doug Engert Date: Tue, 12 Jun 2018 19:50:31 -0500 Subject: [PATCH 06/16] Remove in PIV driver need for aid_file Remove aid_file and aidfile variables in card-piv.c. These are not needed as piv_select_aid parses the returned data from a SELECT AID command. In response to e-mail from X41 group on 6/11/2018. On branch x41-piv-2 Changes to be committed: modified: card-piv.c --- src/libopensc/card-piv.c | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/src/libopensc/card-piv.c b/src/libopensc/card-piv.c index 0cd3f5ad..bfc59596 100644 --- a/src/libopensc/card-piv.c +++ b/src/libopensc/card-piv.c @@ -147,7 +147,6 @@ enum { }; typedef struct piv_private_data { - sc_file_t *aid_file; int enumtag; int selected_obj; /* The index into the piv_objects last selected */ int return_only_cert; /* return the cert from the object */ @@ -758,10 +757,9 @@ static int piv_select_aid(sc_card_t* card, u8* aid, size_t aidlen, u8* response, /* find the PIV AID on the card. If card->type already filled in, * then look for specific AID only - * Assumes that priv may not be present */ -static int piv_find_aid(sc_card_t * card, sc_file_t *aid_file) +static int piv_find_aid(sc_card_t * card) { sc_apdu_t apdu; u8 rbuf[SC_MAX_APDU_BUFFER_SIZE]; @@ -842,8 +840,6 @@ static int piv_find_aid(sc_card_t * card, sc_file_t *aid_file) if (apdu.resp[0] != 0x6f || apdu.resp[1] > apdu.resplen - 2 ) 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]); - LOG_FUNC_RETURN(card->ctx, i); } @@ -2924,7 +2920,6 @@ piv_finish(sc_card_t *card) SC_FUNC_CALLED(card->ctx, SC_LOG_DEBUG_VERBOSE); if (priv) { - sc_file_free(priv->aid_file); if (priv->w_buf) free(priv->w_buf); if (priv->offCardCertURL) @@ -3073,7 +3068,6 @@ static int piv_match_card_continued(sc_card_t *card) card->type = type; card->drv_data = priv; /* will free if no match, or pass on to piv_init */ - priv->aid_file = sc_file_new(); priv->selected_obj = -1; priv->pin_preference = 0x80; /* 800-73-3 part 1, table 3 */ priv->logged_in = SC_PIN_STATE_UNKNOWN; @@ -3100,9 +3094,7 @@ static int piv_match_card_continued(sc_card_t *card) if (i < 0) { /* Detect by selecting applet */ - sc_file_t aidfile; - - i = piv_find_aid(card, &aidfile); + i = piv_find_aid(card); } if (i >= 0) { @@ -3488,7 +3480,7 @@ piv_pin_cmd(sc_card_t *card, struct sc_pin_cmd_data *data, int *tries_left) /* if access to applet is know to be reset by other driver we select_aid and try again */ if ( priv->card_issues & CI_OTHER_AID_LOSE_STATE && priv->pin_cmd_verify_sw1 == 0x6DU) { sc_log(card->ctx, "AID may be lost doing piv_find_aid and retry pin_cmd"); - piv_find_aid(card, priv->aid_file); /* return not tested */ + piv_find_aid(card); priv->pin_cmd_verify = 1; /* tell piv_check_sw its a verify to save sw1, sw2 */ r = iso_drv->ops->pin_cmd(card, data, tries_left); From 384626533e3596f796f860a37ebb72e093939c41 Mon Sep 17 00:00:00 2001 From: Doug Engert Date: Sun, 3 Jun 2018 14:39:40 -0500 Subject: [PATCH 07/16] PIV Security Changes Add return code if "out" is smaller then received data. Remove extra blanks. --- src/libopensc/card-piv.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libopensc/card-piv.c b/src/libopensc/card-piv.c index bfc59596..324f3aec 100644 --- a/src/libopensc/card-piv.c +++ b/src/libopensc/card-piv.c @@ -2369,15 +2369,15 @@ static int piv_validate_general_authentication(sc_card_t *card, r = piv_general_io(card, 0x87, real_alg_id, priv->key_ref, sbuf, p - sbuf, &rbuf, &rbuflen); - if ( r >= 0) { - body = sc_asn1_find_tag(card->ctx, rbuf, rbuflen, 0x7c, &bodylen); - + if (r >= 0) { + body = sc_asn1_find_tag(card->ctx, rbuf, rbuflen, 0x7c, &bodylen); if (body) { tag = sc_asn1_find_tag(card->ctx, body, bodylen, 0x82, &taglen); if (tag) { memcpy(out, tag, taglen); r = taglen; - } + } else + r = SC_ERROR_INVALID_DATA; } else r = SC_ERROR_INVALID_DATA; } From 5807368ed44a6feb78e6107eaa2f337d474a8d36 Mon Sep 17 00:00:00 2001 From: Frank Morgner Date: Mon, 18 Jun 2018 15:23:46 +0200 Subject: [PATCH 08/16] fixed bad memory access --- src/libopensc/card-cac.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libopensc/card-cac.c b/src/libopensc/card-cac.c index a42df928..4e971eed 100644 --- a/src/libopensc/card-cac.c +++ b/src/libopensc/card-cac.c @@ -1390,9 +1390,9 @@ static int cac_path_from_cardurl(sc_card_t *card, sc_path_t *path, cac_card_url_ } sc_mem_clear(path, sizeof(sc_path_t)); memcpy(path->aid.value, &val->rid, sizeof(val->rid)); - memcpy(&path->aid.value[5], &val->applicationID, sizeof(val->applicationID)); + memcpy(&path->aid.value[5], val->applicationID, sizeof(val->applicationID)); path->aid.len = sizeof(val->rid) + sizeof(val->applicationID); - memcpy(path->value, &val->objectID, sizeof(val->objectID)); + memcpy(path->value, val->objectID, sizeof(val->objectID)); path->len = sizeof(val->objectID); path->type = SC_PATH_TYPE_FILE_ID; sc_debug(card->ctx, SC_LOG_DEBUG_VERBOSE, From 03628449b75a93787eb2359412a3980365dda49b Mon Sep 17 00:00:00 2001 From: Frank Morgner Date: Fri, 29 Jun 2018 15:58:00 +0200 Subject: [PATCH 09/16] iasecc: fixed unbound recursion --- src/libopensc/card-iasecc.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libopensc/card-iasecc.c b/src/libopensc/card-iasecc.c index 48d4cea5..b37b833b 100644 --- a/src/libopensc/card-iasecc.c +++ b/src/libopensc/card-iasecc.c @@ -827,16 +827,16 @@ iasecc_select_file(struct sc_card *card, const struct sc_path *path, sc_log(ctx, "iasecc_select_file() path:%s", sc_print_path(path)); sc_print_cache(card); - if (lpath.len >= 2 && lpath.value[0] == 0x3F && lpath.value[1] == 0x00) { + if (path->type != SC_PATH_TYPE_DF_NAME + && lpath.len >= 2 + && lpath.value[0] == 0x3F && lpath.value[1] == 0x00) { sc_log(ctx, "EF.ATR(aid:'%s')", card->ef_atr ? sc_dump_hex(card->ef_atr->aid.value, card->ef_atr->aid.len) : ""); rv = iasecc_select_mf(card, file_out); LOG_TEST_RET(ctx, rv, "MF selection error"); - if (lpath.len >= 2 && lpath.value[0] == 0x3F && lpath.value[1] == 0x00) { - memmove(&lpath.value[0], &lpath.value[2], lpath.len - 2); - lpath.len -= 2; - } + memmove(&lpath.value[0], &lpath.value[2], lpath.len - 2); + lpath.len -= 2; } if (lpath.aid.len) { From 78f005533818e5920a563bb5949c1a647d786090 Mon Sep 17 00:00:00 2001 From: Frank Morgner Date: Mon, 2 Jul 2018 09:53:35 +0200 Subject: [PATCH 10/16] fixed uninitialized use of variable --- src/libopensc/pkcs15-tcos.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libopensc/pkcs15-tcos.c b/src/libopensc/pkcs15-tcos.c index 7732ad08..547de12a 100644 --- a/src/libopensc/pkcs15-tcos.c +++ b/src/libopensc/pkcs15-tcos.c @@ -132,7 +132,7 @@ static int insert_key( int i, rec_no=0; if(prkey_info.path.len>=2) prkey_info.path.len-=2; sc_append_file_id(&prkey_info.path, 0x5349); - if(sc_select_file(card, &prkey_info.path, NULL)!=SC_SUCCESS || !f->prop_attr){ + if(sc_select_file(card, &prkey_info.path, NULL)!=SC_SUCCESS){ sc_debug(ctx, SC_LOG_DEBUG_NORMAL, "Select(%s) failed\n", sc_print_path(&prkey_info.path)); From 92a98cb3bb1719b0100b033aae1c3c1f2679e7c8 Mon Sep 17 00:00:00 2001 From: Frank Morgner Date: Mon, 2 Jul 2018 10:07:53 +0200 Subject: [PATCH 11/16] mcrd: converted assert to proper error handling --- src/libopensc/card-mcrd.c | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/src/libopensc/card-mcrd.c b/src/libopensc/card-mcrd.c index 16cb9992..52dd457d 100644 --- a/src/libopensc/card-mcrd.c +++ b/src/libopensc/card-mcrd.c @@ -149,7 +149,8 @@ static struct df_info_s *get_df_info(sc_card_t * card) struct mcrd_priv_data *priv = DRVDATA(card); struct df_info_s *dfi; - assert(!priv->is_ef); + if(!(!priv->is_ef)) + return NULL; if (!priv->curpathlen) { sc_log(ctx, "no current path to find the df_info\n"); @@ -202,7 +203,8 @@ static int mcrd_delete_ref_to_authkey(sc_card_t * card) int r; u8 sbuf[SC_MAX_APDU_BUFFER_SIZE]; - assert(card != NULL); + if(!(card != NULL)) + return SC_ERROR_INTERNAL; sc_format_apdu(card, &apdu, SC_APDU_CASE_3_SHORT, 0x22, 0x41, 0xA4); sbuf[0] = 0x83; @@ -220,7 +222,8 @@ static int mcrd_delete_ref_to_signkey(sc_card_t * card) sc_apdu_t apdu; int r; u8 sbuf[SC_MAX_APDU_BUFFER_SIZE]; - assert(card != NULL); + if(!(card != NULL)) + return SC_ERROR_INTERNAL; sc_format_apdu(card, &apdu, SC_APDU_CASE_3_SHORT, 0x22, 0x41, 0xB6); @@ -242,7 +245,8 @@ static int mcrd_set_decipher_key_ref(sc_card_t * card, int key_reference) int r; u8 sbuf[SC_MAX_APDU_BUFFER_SIZE]; u8 keyref_data[SC_ESTEID_KEYREF_FILE_RECLEN]; - assert(card != NULL); + if(!(card != NULL)) + return SC_ERROR_INTERNAL; sc_format_apdu(card, &apdu, SC_APDU_CASE_3_SHORT, 0x22, 0x41, 0xB8); /* track the active keypair */ @@ -956,7 +960,8 @@ select_file_by_path(sc_card_t * card, unsigned short *pathptr, SC_FUNC_CALLED(card->ctx, SC_LOG_DEBUG_VERBOSE); - assert(!priv->curpathlen || priv->curpath[0] == MFID); + if (!(!priv->curpathlen || priv->curpath[0] == MFID)) + return SC_ERROR_INTERNAL; if (pathlen && *pathptr == 0x3FFF) { pathlen--; @@ -997,7 +1002,8 @@ select_file_by_path(sc_card_t * card, unsigned short *pathptr, /* This EF or DF was already selected, but we need to get the FCI, so we have to select again. */ - assert(priv->curpathlen > 1); + if (!(priv->curpathlen > 1)) + return SC_ERROR_INTERNAL; priv->curpathlen--; priv->is_ef = 0; r = select_down(card, pathptr + pathlen - 1, 1, @@ -1022,7 +1028,8 @@ select_file_by_path(sc_card_t * card, unsigned short *pathptr, priv->is_ef = 0; } if (priv->is_ef) { - assert(priv->curpathlen > 1); + if(!(priv->curpathlen > 1)) + return SC_ERROR_INTERNAL; priv->curpathlen--; priv->is_ef = 0; } @@ -1040,7 +1047,8 @@ select_file_by_fid(sc_card_t * card, unsigned short *pathptr, SC_FUNC_CALLED(card->ctx, SC_LOG_DEBUG_VERBOSE); - assert(!priv->curpathlen || priv->curpath[0] == MFID); + if (!(!priv->curpathlen || priv->curpath[0] == MFID)) + return SC_ERROR_INTERNAL; if (pathlen > 1) return SC_ERROR_INVALID_ARGUMENTS; @@ -1056,7 +1064,8 @@ select_file_by_fid(sc_card_t * card, unsigned short *pathptr, /* There is no current file. */ r = SC_ERROR_INTERNAL; } else { - assert(priv->curpathlen > 1); + if (!(priv->curpathlen > 1)) + return SC_ERROR_INTERNAL; priv->curpathlen--; priv->is_ef = 0; r = select_down(card, pathptr, 1, 0, file); @@ -1081,7 +1090,8 @@ select_file_by_fid(sc_card_t * card, unsigned short *pathptr, priv->is_ef = 0; } if (priv->is_ef) { - assert(priv->curpathlen > 1); + if (!(priv->curpathlen > 1)) + return SC_ERROR_INTERNAL; priv->curpathlen--; priv->is_ef = 0; } @@ -1209,7 +1219,8 @@ static int mcrd_set_security_env(sc_card_t * card, u8 *p; int r, locked = 0; - assert(card != NULL && env != NULL); + if (!(card != NULL && env != NULL)) + return SC_ERROR_INTERNAL; SC_FUNC_CALLED(card->ctx, SC_LOG_DEBUG_NORMAL); /* special environment handling for esteid, stolen from openpgp */ From 30fe0ad453caa34d5851cf36d3e459133dcadd3b Mon Sep 17 00:00:00 2001 From: Frank Morgner Date: Tue, 3 Jul 2018 09:36:21 +0200 Subject: [PATCH 12/16] pgp: fixed integer underflow --- src/libopensc/card-openpgp.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/libopensc/card-openpgp.c b/src/libopensc/card-openpgp.c index 5fa38992..59cf41e7 100644 --- a/src/libopensc/card-openpgp.c +++ b/src/libopensc/card-openpgp.c @@ -623,13 +623,19 @@ pgp_get_card_features(sc_card_t *card) /* category indicator 0x00, 0x10 or 0x80 => compact TLV (ISO) */ switch (hist_bytes[0]) { case 0x00: - pgp_parse_hist_bytes(card, hist_bytes+1, hist_bytes_len-4); + if (hist_bytes_len > 4) { + pgp_parse_hist_bytes(card, hist_bytes+1, hist_bytes_len-4); + } break; case 0x80: - pgp_parse_hist_bytes(card, hist_bytes+1, hist_bytes_len-1); + if (hist_bytes_len > 1) { + pgp_parse_hist_bytes(card, hist_bytes+1, hist_bytes_len-1); + } break; case 0x10: - pgp_parse_hist_bytes(card, hist_bytes+2, hist_bytes_len-2); + if (hist_bytes_len > 2) { + pgp_parse_hist_bytes(card, hist_bytes+2, hist_bytes_len-2); + } break; } } @@ -642,7 +648,9 @@ pgp_get_card_features(sc_card_t *card) if ((pgp_get_blob(card, priv->mf, 0x5f52, &blob) >= 0) && (blob->data != NULL) && (blob->data[0] == 0x00)) { - pgp_parse_hist_bytes(card, hist_bytes+1, hist_bytes_len-4); + if (hist_bytes_len > 4) { + pgp_parse_hist_bytes(card, hist_bytes+1, hist_bytes_len-4); + } /* get card status from historical bytes status indicator */ if ((blob->data[0] == 0x00) && (blob->len >= 4)) { From 0b44793900ee9ad72909242da761702e23b12167 Mon Sep 17 00:00:00 2001 From: Frank Morgner Date: Wed, 4 Jul 2018 17:51:35 +0200 Subject: [PATCH 13/16] tcos: use ISO7816 fci parser --- src/libopensc/card-tcos.c | 38 +------------------------------------- 1 file changed, 1 insertion(+), 37 deletions(-) diff --git a/src/libopensc/card-tcos.c b/src/libopensc/card-tcos.c index 3c86b1f4..0fad1e15 100644 --- a/src/libopensc/card-tcos.c +++ b/src/libopensc/card-tcos.c @@ -343,7 +343,6 @@ static int tcos_select_file(sc_card_t *card, sc_apdu_t apdu; sc_file_t *file=NULL; u8 buf[SC_MAX_APDU_BUFFER_SIZE], pathbuf[SC_MAX_PATH_SIZE], *path = pathbuf; - unsigned int i; int r, pathlen; assert(card != NULL && in_path != NULL); @@ -407,42 +406,7 @@ static int tcos_select_file(sc_card_t *card, *file_out = file; file->path = *in_path; - for(i=2; i+1size=0; - for(j=0; jsize = (file->size<<8) | d[j]; - break; - case 0x82: - file->shareable = (d[0] & 0x40) ? 1 : 0; - file->ef_structure = d[0] & 7; - switch ((d[0]>>3) & 7) { - case 0: file->type = SC_FILE_TYPE_WORKING_EF; break; - case 7: file->type = SC_FILE_TYPE_DF; break; - default: - sc_debug(ctx, SC_LOG_DEBUG_NORMAL, "invalid file type %02X in file descriptor\n", d[0]); - SC_FUNC_RETURN(ctx, SC_LOG_DEBUG_VERBOSE, SC_ERROR_UNKNOWN_DATA_RECEIVED); - } - break; - case 0x83: - file->id = (d[0]<<8) | d[1]; - break; - case 0x84: - file->namelen = MIN(sizeof file->name, len); - memcpy(file->name, d, file->namelen); - break; - case 0x86: - sc_file_set_sec_attr(file, d, len); - break; - default: - if (len>0) sc_file_set_prop_attr(file, d, len); - } - } - file->magic = SC_FILE_MAGIC; + iso_ops->process_fci(card, file, apdu.resp, apdu.resplen); parse_sec_attr(card, file, file->sec_attr, file->sec_attr_len); From 50b000047c44b1f2c6f0134094c19519d512e5b6 Mon Sep 17 00:00:00 2001 From: Frank Morgner Date: Wed, 4 Jul 2018 17:51:53 +0200 Subject: [PATCH 14/16] ias/ecc: disable iccsn parsing if someone wants to implement this with memory bounds checking, please raise your hands --- src/libopensc/card-iasecc.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/libopensc/card-iasecc.c b/src/libopensc/card-iasecc.c index b37b833b..a817c99e 100644 --- a/src/libopensc/card-iasecc.c +++ b/src/libopensc/card-iasecc.c @@ -2504,6 +2504,12 @@ iasecc_pin_cmd(struct sc_card *card, struct sc_pin_cmd_data *data, int *tries_le static int iasecc_get_serialnr(struct sc_card *card, struct sc_serial_number *serial) { +#if 1 + /* the current implementation doesn't perform any bounds check when parsing + * the serial number. Hence, we disable this code until someone has time to + * fix this. */ + LOG_FUNC_RETURN(card->ctx, SC_ERROR_NOT_SUPPORTED); +#else struct sc_context *ctx = card->ctx; struct sc_iin *iin = &card->serialnr.iin; struct sc_apdu apdu; @@ -2573,6 +2579,7 @@ end: memcpy(serial, &card->serialnr, sizeof(*serial)); LOG_FUNC_RETURN(ctx, SC_SUCCESS); +#endif } From 79c0dbaa4e47ad8d9d43b327f5cc34c2f31dd873 Mon Sep 17 00:00:00 2001 From: Jakub Jelen Date: Mon, 9 Jul 2018 14:13:41 +0200 Subject: [PATCH 15/16] cac: Avoid OOB reads for inconsistent TLV structures --- src/libopensc/card-cac.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/libopensc/card-cac.c b/src/libopensc/card-cac.c index 4e971eed..eeab07e4 100644 --- a/src/libopensc/card-cac.c +++ b/src/libopensc/card-cac.c @@ -1555,8 +1555,15 @@ static int cac_parse_CCC(sc_card_t *card, cac_private_data_t *priv, u8 *tl, for (; (tl < tl_end) && (val< val_end); val += len) { /* get the tag and the length */ u8 tag; - if (sc_simpletlv_read_tag(&tl, tl_end - tl, &tag, &len) != SC_SUCCESS) + r = sc_simpletlv_read_tag(&tl, tl_end - tl, &tag, &len); + if (r != SC_SUCCESS && r != SC_ERROR_TLV_END_OF_CONTENTS) { + sc_log(card->ctx, "Failed to parse tag from buffer"); break; + } + if (val + len > val_end) { + sc_log(card->ctx, "Invalid length %"SC_FORMAT_LEN_SIZE_T"u", len); + break; + } switch (tag) { case CAC_TAG_CUID: sc_debug(card->ctx, SC_LOG_DEBUG_VERBOSE,"TAG:CUID"); From 5ec26573da7e6f21364d40c5c74962ef60a07000 Mon Sep 17 00:00:00 2001 From: Jakub Jelen Date: Tue, 14 Aug 2018 16:12:38 +0200 Subject: [PATCH 16/16] coolkey: Do not overflow allocated buffer --- src/libopensc/card-coolkey.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libopensc/card-coolkey.c b/src/libopensc/card-coolkey.c index 3c57628b..b97559cc 100644 --- a/src/libopensc/card-coolkey.c +++ b/src/libopensc/card-coolkey.c @@ -1129,7 +1129,7 @@ static int coolkey_read_object(sc_card_t *card, unsigned long object_id, size_t do { ulong2bebytes(¶ms.offset[0], offset); params.length = MIN(left, COOLKEY_MAX_CHUNK_SIZE); - len = left+2; + len = left; r = coolkey_apdu_io(card, COOLKEY_CLASS, COOLKEY_INS_READ_OBJECT, 0, 0, (u8 *)¶ms, sizeof(params), &out_ptr, &len, nonce, nonce_size); if (r < 0) {