From 196c8389572b72fa1b837c209d798185cb58686f Mon Sep 17 00:00:00 2001 From: Frank Morgner Date: Fri, 23 Feb 2018 23:15:03 +0100 Subject: [PATCH] fixed use after free ICCSN and CHN pointed into EF.GDO's content, which was freed preemptively. Regression of 0502a839c654555220964e9f4d58ca508471472e --- src/libopensc/card-starcos.c | 12 ++++----- src/libopensc/card-tcos.c | 13 ++++------ src/libopensc/ef-gdo.c | 43 +++++++++++++++---------------- src/libopensc/opensc.h | 4 +-- src/libopensc/pkcs15-infocamere.c | 13 ++++++---- src/libopensc/pkcs15-tccardos.c | 10 +++---- 6 files changed, 46 insertions(+), 49 deletions(-) diff --git a/src/libopensc/card-starcos.c b/src/libopensc/card-starcos.c index 17c4578a..f31a1f00 100644 --- a/src/libopensc/card-starcos.c +++ b/src/libopensc/card-starcos.c @@ -1700,8 +1700,6 @@ static int starcos_get_serialnr(sc_card_t *card, sc_serial_number_t *serial) { int r; u8 rbuf[SC_MAX_APDU_BUFFER_SIZE]; - const unsigned char *iccsn; - size_t iccsn_len; sc_apdu_t apdu; if (!serial) @@ -1715,12 +1713,12 @@ static int starcos_get_serialnr(sc_card_t *card, sc_serial_number_t *serial) switch (card->type) { case SC_CARD_TYPE_STARCOS_V3_4: - r = sc_parse_ef_gdo(card, &iccsn, &iccsn_len, NULL, 0); - if (r < 0) + card->serialnr.len = SC_MAX_SERIALNR; + r = sc_parse_ef_gdo(card, card->serialnr.value, &card->serialnr.len, NULL, 0); + if (r < 0) { + card->serialnr.len = 0; return r; - /* cache serial number */ - memcpy(card->serialnr.value, iccsn, MIN(iccsn_len, SC_MAX_SERIALNR)); - card->serialnr.len = MIN(iccsn_len, SC_MAX_SERIALNR); + } break; default: diff --git a/src/libopensc/card-tcos.c b/src/libopensc/card-tcos.c index 3a885e94..6221017c 100644 --- a/src/libopensc/card-tcos.c +++ b/src/libopensc/card-tcos.c @@ -707,8 +707,6 @@ static int tcos_setperm(sc_card_t *card, int enable_nullpin) static int tcos_get_serialnr(sc_card_t *card, sc_serial_number_t *serial) { int r; - const unsigned char *iccsn; - size_t iccsn_len; if (!serial) return SC_ERROR_INVALID_ARGUMENTS; @@ -719,13 +717,12 @@ static int tcos_get_serialnr(sc_card_t *card, sc_serial_number_t *serial) return SC_SUCCESS; } - r = sc_parse_ef_gdo(card, &iccsn, &iccsn_len, NULL, 0); - if (r < 0) + card->serialnr.len = sizeof card->serialnr.value; + r = sc_parse_ef_gdo(card, card->serialnr.value, &card->serialnr.len, NULL, 0); + if (r < 0) { + card->serialnr.len = 0; return r; - - /* cache serial number */ - memcpy(card->serialnr.value, iccsn, MIN(iccsn_len, SC_MAX_SERIALNR)); - card->serialnr.len = MIN(iccsn_len, SC_MAX_SERIALNR); + } /* copy and return serial number */ memcpy(serial, &card->serialnr, sizeof(*serial)); diff --git a/src/libopensc/ef-gdo.c b/src/libopensc/ef-gdo.c index d1017f75..bda51f22 100644 --- a/src/libopensc/ef-gdo.c +++ b/src/libopensc/ef-gdo.c @@ -24,25 +24,17 @@ #include "asn1.h" #include "internal.h" #include +#include static int sc_parse_ef_gdo_content(const unsigned char *gdo, size_t gdo_len, - const unsigned char **iccsn, size_t *iccsn_len, - const unsigned char **chn, size_t *chn_len) + unsigned char *iccsn, size_t *iccsn_len, + unsigned char *chn, size_t *chn_len) { - int r = SC_SUCCESS; + int r = SC_SUCCESS, iccsn_found = 0, chn_found = 0; const unsigned char *p = gdo; size_t left = gdo_len; - if (iccsn) - *iccsn = NULL; - if (iccsn_len) - *iccsn_len = 0; - if (chn) - *chn = NULL; - if (chn_len) - *chn_len = 0; - while (left >= 2) { unsigned int cla, tag; size_t tag_len; @@ -63,16 +55,18 @@ sc_parse_ef_gdo_content(const unsigned char *gdo, size_t gdo_len, if (cla == SC_ASN1_TAG_APPLICATION) { switch (tag) { case 0x1A: - if (iccsn) - *iccsn = p; - if (iccsn_len) - *iccsn_len = tag_len; + iccsn_found = 1; + if (iccsn && iccsn_len) { + memcpy(iccsn, p, MIN(tag_len, *iccsn_len)); + *iccsn_len = MIN(tag_len, *iccsn_len); + } break; case 0x1F20: - if (chn) - *chn = p; - if (chn_len) - *chn_len = tag_len; + chn_found = 1; + if (chn && chn_len) { + memcpy(chn, p, MIN(tag_len, *chn_len)); + *chn_len = MIN(tag_len, *chn_len); + } break; } } @@ -81,6 +75,11 @@ sc_parse_ef_gdo_content(const unsigned char *gdo, size_t gdo_len, left -= (p - gdo); } + if (!iccsn_found && iccsn_len) + *iccsn_len = 0; + if (!chn_found && chn_len) + *chn_len = 0; + return r; } @@ -88,8 +87,8 @@ sc_parse_ef_gdo_content(const unsigned char *gdo, size_t gdo_len, int sc_parse_ef_gdo(struct sc_card *card, - const unsigned char **iccsn, size_t *iccsn_len, - const unsigned char **chn, size_t *chn_len) + unsigned char *iccsn, size_t *iccsn_len, + unsigned char *chn, size_t *chn_len) { struct sc_context *ctx; struct sc_path path; diff --git a/src/libopensc/opensc.h b/src/libopensc/opensc.h index 6e121a1d..be4f9055 100644 --- a/src/libopensc/opensc.h +++ b/src/libopensc/opensc.h @@ -1336,8 +1336,8 @@ void sc_free_apps(struct sc_card *card); int sc_parse_ef_atr(struct sc_card *card); void sc_free_ef_atr(struct sc_card *card); int sc_parse_ef_gdo(struct sc_card *card, - const unsigned char **iccsn, size_t *iccsn_len, - const unsigned char **chn, size_t *chn_len); + unsigned char *iccsn, size_t *iccsn_len, + unsigned char *chn, size_t *chn_len); int sc_update_dir(struct sc_card *card, sc_app_info_t *app); void sc_print_cache(struct sc_card *card); diff --git a/src/libopensc/pkcs15-infocamere.c b/src/libopensc/pkcs15-infocamere.c index 02c55ee3..6532ffc5 100644 --- a/src/libopensc/pkcs15-infocamere.c +++ b/src/libopensc/pkcs15-infocamere.c @@ -234,18 +234,21 @@ static int infocamere_1200_init(sc_pkcs15_card_t * p15card) int r; - const unsigned char *iccsn, *chn; - size_t iccsn_len, chn_len; + unsigned char chn[8]; + size_t chn_len = sizeof chn; + sc_serial_number_t iccsn; + iccsn.len = sizeof iccsn.value; - r = sc_parse_ef_gdo(card, &iccsn, &iccsn_len, &chn, &chn_len); + + r = sc_parse_ef_gdo(card, iccsn.value, &iccsn.len, chn, &chn_len); if (r < 0) return r; - if (!iccsn_len || chn_len < 2 || chn_len > 8) { + if (!iccsn.len || chn_len < 2 || chn_len > 8) { return SC_ERROR_WRONG_CARD; } - sc_bin_to_hex(iccsn, iccsn_len, serial, sizeof(serial), 0); + sc_bin_to_hex(iccsn.value, iccsn.len, serial, sizeof(serial), 0); if (! (chn[0] == 0x12 diff --git a/src/libopensc/pkcs15-tccardos.c b/src/libopensc/pkcs15-tccardos.c index d8986a9d..8792cf15 100644 --- a/src/libopensc/pkcs15-tccardos.c +++ b/src/libopensc/pkcs15-tccardos.c @@ -305,9 +305,9 @@ static int sc_pkcs15_tccardos_init_func(sc_pkcs15_card_t *p15card) struct sc_path path; struct sc_file *file = NULL; char hex_buf[256]; - const unsigned char *iccsn; - size_t iccsn_len; struct sc_card *card = p15card->card; + sc_serial_number_t iccsn; + iccsn.len = sizeof iccsn.value; /* check if we have the correct card OS */ if (strcmp(card->name, "CardOS M4")) @@ -329,10 +329,10 @@ static int sc_pkcs15_tccardos_init_func(sc_pkcs15_card_t *p15card) if (p15card->tokeninfo->manufacturer_id == NULL) return SC_ERROR_OUT_OF_MEMORY; /* set the serial number */ - r = sc_parse_ef_gdo(card, &iccsn, &iccsn_len, NULL, 0); - if (r != SC_SUCCESS || iccsn_len < 5+8) + r = sc_parse_ef_gdo(card, iccsn.value, &iccsn.len, NULL, 0); + if (r != SC_SUCCESS || iccsn.len < 5+8) return SC_ERROR_INTERNAL; - sc_bin_to_hex(iccsn + 5, 8, hex_buf, sizeof(hex_buf), 0); + sc_bin_to_hex(iccsn.value + 5, 8, hex_buf, sizeof(hex_buf), 0); p15card->tokeninfo->serial_number = strdup(hex_buf); if (p15card->tokeninfo->serial_number == NULL) return SC_ERROR_OUT_OF_MEMORY;