From 2e72c8f85c63639584bafb713cfd64be8535aa1b Mon Sep 17 00:00:00 2001 From: Dave Pate Date: Mon, 7 Jan 2019 14:20:01 -0800 Subject: [PATCH 01/13] lib: resolves potential reads of uninitialized data --- lib/util.c | 3 ++- lib/ykpiv.c | 11 +++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/lib/util.c b/lib/util.c index 4133187..a978d01 100644 --- a/lib/util.c +++ b/lib/util.c @@ -274,8 +274,9 @@ ykpiv_rc ykpiv_util_list_keys(ykpiv_state *state, uint8_t *key_count, ykpiv_key for (i = 0; i < sizeof(SLOTS); i++) { cbBuf = sizeof(buf); + res = _read_certificate(state, SLOTS[i], buf, &cbBuf); - if (YKPIV_OK == (res = _read_certificate(state, SLOTS[i], buf, &cbBuf))) { + if ((res == YKPIV_OK) && (cbBuf > 0)) { // add current slot to result, grow result buffer if necessary cbRealloc = (sizeof(ykpiv_key) + cbBuf - 1) > (cbData - offset) ? MAX((sizeof(ykpiv_key) + cbBuf - 1) - (cbData - offset), CB_PAGE) : 0; diff --git a/lib/ykpiv.c b/lib/ykpiv.c index 3466b12..49f284f 100644 --- a/lib/ykpiv.c +++ b/lib/ykpiv.c @@ -1044,6 +1044,12 @@ static ykpiv_rc _ykpiv_get_version(ykpiv_state *state, ykpiv_version_t *p_versio if((res = _send_data(state, &apdu, data, &recv_len, &sw)) != YKPIV_OK) { return res; } else if(sw == SW_SUCCESS) { + + /* check that we received enough data for the verson number */ + if (recv_len < 3) { + return YKPIV_SIZE_ERROR; + } + state->ver.major = data[0]; state->ver.minor = data[1]; state->ver.patch = data[2]; @@ -1182,6 +1188,11 @@ static ykpiv_rc _ykpiv_get_serial(ykpiv_state *state, uint32_t *p_serial, bool f } } + /* check that we received enough data for the serial number */ + if (recv_len < 4) { + return YKPIV_SIZE_ERROR; + } + p_temp = (uint8_t*)(&state->serial); *p_temp++ = data[3]; From 28189201a410812ebdc183a36d907ae4ab13dad4 Mon Sep 17 00:00:00 2001 From: Dave Pate Date: Mon, 7 Jan 2019 14:37:30 -0800 Subject: [PATCH 02/13] lib: use secure zero memory platform functions --- lib/internal.h | 12 ++++++++++++ lib/util.c | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/lib/internal.h b/lib/internal.h index 498b912..60d9643 100644 --- a/lib/internal.h +++ b/lib/internal.h @@ -236,6 +236,18 @@ typedef enum _yc_log_level_t { void yc_log_event(uint32_t id, yc_log_level_t level, const char *sz_format, ...); +#ifdef _WIN32 +#include +#define yc_memzero SecureZeroMemory +#elif __OPENBSD__ +#include +#define yc_memzero explicit_bzero; +#else +#define __STDC_WANT_LIB_EXT1__ 1 +#include +#define yc_memzero(_p, _n) (void)memset_s(_p, (rsize_t)_n, 0, (rsize_t)_n) +#endif + #ifdef __cplusplus } #endif diff --git a/lib/util.c b/lib/util.c index a978d01..cc6da6f 100644 --- a/lib/util.c +++ b/lib/util.c @@ -1146,7 +1146,7 @@ ykpiv_rc ykpiv_util_get_protected_mgm(ykpiv_state *state, ykpiv_mgm *mgm) { Cleanup: - memset(data, 0, sizeof(data)); + yc_memzero(data, sizeof(data)); _ykpiv_end_transaction(state); return res; From b2dd16deb4295df89620a0846b6137c7ead11dbd Mon Sep 17 00:00:00 2001 From: Dave Pate Date: Mon, 7 Jan 2019 15:04:31 -0800 Subject: [PATCH 03/13] lib: clear buffers containing key material --- lib/internal.c | 8 ++++++-- lib/ykpiv.c | 2 ++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/internal.c b/lib/internal.c index 20b911a..a3b66cd 100644 --- a/lib/internal.c +++ b/lib/internal.c @@ -241,6 +241,7 @@ des_rc des_import_key(const int type, const unsigned char* keyraw, const size_t EXIT: #ifdef _WINDOWS if (pbSessionBlob) { + yc_memzero(pbSessionBlob, cbSessionBlob); free(pbSessionBlob); pbSessionBlob = NULL; } @@ -353,6 +354,7 @@ EXIT: bool yk_des_is_weak_key(const unsigned char *key, const size_t cb_key) { #ifdef _WINDOWS + bool rv = false; /* defined weak keys, borrowed from openssl to be consistent across platforms */ static const unsigned char weak_keys[][DES_LEN_DES] = { /* weak keys */ @@ -400,11 +402,13 @@ bool yk_des_is_weak_key(const unsigned char *key, const size_t cb_key) { if ((0 == memcmp(weak_keys[i], tmp, DES_LEN_DES)) || (0 == memcmp(weak_keys[i], tmp + DES_LEN_DES, DES_LEN_DES)) || (0 == memcmp(weak_keys[i], tmp + 2*DES_LEN_DES, DES_LEN_DES))) { - return true; + rv = true; + break; } } - return false; + yc_memzero(tmp, DES_LEN_3DES); + return rv; #else (void)cb_key; /* unused */ diff --git a/lib/ykpiv.c b/lib/ykpiv.c index 49f284f..f9ee4a1 100644 --- a/lib/ykpiv.c +++ b/lib/ykpiv.c @@ -838,6 +838,7 @@ ykpiv_rc ykpiv_set_mgmkey2(ykpiv_state *state, const unsigned char *new_key, con res = YKPIV_GENERIC_ERROR; Cleanup: + yc_memzero(&apdu, sizeof(APDU)); _ykpiv_end_transaction(state); return res; } @@ -1704,6 +1705,7 @@ ykpiv_rc ykpiv_import_private_key(ykpiv_state *state, const unsigned char key, u } Cleanup: + yc_memzero(key_data, sizeof(key_data)); _ykpiv_end_transaction(state); return res; } From eb250134f899d292d3af450755c4a16b23fe7fcb Mon Sep 17 00:00:00 2001 From: Dave Pate Date: Mon, 7 Jan 2019 15:10:18 -0800 Subject: [PATCH 04/13] lib: check internal authentication crypt errors --- lib/ykpiv.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/lib/ykpiv.c b/lib/ykpiv.c index f9ee4a1..8aa2b7a 100644 --- a/lib/ykpiv.c +++ b/lib/ykpiv.c @@ -684,6 +684,7 @@ ykpiv_rc ykpiv_authenticate(ykpiv_state *state, unsigned const char *key) { uint32_t recv_len = sizeof(data); int sw; ykpiv_rc res; + des_rc drc = DES_OK; des_key* mgm_key = NULL; size_t out_len = 0; @@ -728,7 +729,12 @@ ykpiv_rc ykpiv_authenticate(ykpiv_state *state, unsigned const char *key) { unsigned char *dataptr = apdu.st.data; unsigned char response[8]; out_len = sizeof(response); - des_decrypt(mgm_key, challenge, sizeof(challenge), response, &out_len); + drc = des_decrypt(mgm_key, challenge, sizeof(challenge), response, &out_len); + + if (drc != DES_OK) { + res = YKPIV_AUTHENTICATION_ERROR; + goto Cleanup; + } recv_len = sizeof(data); memset(apdu.raw, 0, sizeof(apdu)); @@ -766,7 +772,13 @@ ykpiv_rc ykpiv_authenticate(ykpiv_state *state, unsigned const char *key) { { unsigned char response[8]; out_len = sizeof(response); - des_encrypt(mgm_key, challenge, sizeof(challenge), response, &out_len); + drc = des_encrypt(mgm_key, challenge, sizeof(challenge), response, &out_len); + + if (drc != DES_OK) { + res = YKPIV_AUTHENTICATION_ERROR; + goto Cleanup; + } + if (memcmp(response, data + 4, 8) == 0) { res = YKPIV_OK; } From 934120888f1d28ea56b606bcf93217c892017772 Mon Sep 17 00:00:00 2001 From: Dave Pate Date: Mon, 7 Jan 2019 15:50:18 -0800 Subject: [PATCH 05/13] lib: define constant for max pin len magic numbers lib: clear pin buffers when no longer used --- lib/internal.c | 2 ++ lib/internal.h | 1 + lib/ykpiv.c | 43 ++++++++++++++++++++++++++++--------------- 3 files changed, 31 insertions(+), 15 deletions(-) diff --git a/lib/internal.c b/lib/internal.c index a3b66cd..496d976 100644 --- a/lib/internal.c +++ b/lib/internal.c @@ -462,6 +462,8 @@ pkcs5_rc pkcs5_pbkdf2_sha1(const uint8_t* password, const size_t cb_password, co */ if (STATUS_SUCCESS == BCryptOpenAlgorithmProvider(&hAlg, BCRYPT_SHA1_ALGORITHM, NULL, BCRYPT_ALG_HANDLE_HMAC_FLAG)) { + /* suppress const qualifier warning b/c BCrypt doesn't take const input buffers */ +#pragma warning(suppress: 4090) if (STATUS_SUCCESS != BCryptDeriveKeyPBKDF2(hAlg, (PUCHAR)password, (ULONG)cb_password, (PUCHAR)salt, (ULONG)cb_salt, iterations, key, (ULONG)cb_key, 0)) { rc = PKCS5_GENERAL_ERROR; } diff --git a/lib/internal.h b/lib/internal.h index 60d9643..632052e 100644 --- a/lib/internal.h +++ b/lib/internal.h @@ -126,6 +126,7 @@ extern "C" #define CB_OBJ_TAG_MIN 2 // 1 byte tag + 1 byte len #define CB_OBJ_TAG_MAX (CB_OBJ_TAG_MIN + 2) // 1 byte tag + 3 bytes len +#define CB_PIN_MAX 8 #define member_size(type, member) sizeof(((type*)0)->member) typedef enum { diff --git a/lib/ykpiv.c b/lib/ykpiv.c index 8aa2b7a..2c640a9 100644 --- a/lib/ykpiv.c +++ b/lib/ykpiv.c @@ -1246,6 +1246,7 @@ static ykpiv_rc _cache_pin(ykpiv_state *state, const char *pin, size_t len) { return YKPIV_OK; } if (state->pin) { + yc_memzero(state->pin, strnlen(state->pin, CB_PIN_MAX)); _ykpiv_free(state, state->pin); state->pin = NULL; } @@ -1268,7 +1269,7 @@ static ykpiv_rc _verify(ykpiv_state *state, const char *pin, const size_t pin_le int sw; ykpiv_rc res; - if (pin_len > 8) { + if (pin_len > CB_PIN_MAX) { return YKPIV_SIZE_ERROR; } @@ -1279,27 +1280,36 @@ static ykpiv_rc _verify(ykpiv_state *state, const char *pin, const size_t pin_le apdu.st.lc = pin ? 0x08 : 0; if (pin) { memcpy(apdu.st.data, pin, pin_len); - if (pin_len < 8) { - memset(apdu.st.data + pin_len, 0xff, 8 - pin_len); + if (pin_len < CB_PIN_MAX) { + memset(apdu.st.data + pin_len, 0xff, CB_PIN_MAX - pin_len); } } - if ((res = _send_data(state, &apdu, data, &recv_len, &sw)) != YKPIV_OK) { + + res = _send_data(state, &apdu, data, &recv_len, &sw); + yc_memzero(&apdu, sizeof(apdu)); + + if (res != YKPIV_OK) { return res; - } else if (sw == SW_SUCCESS) { + } + else if (sw == SW_SUCCESS) { if (pin && pin_len) { // Intentionally ignore errors. If the PIN fails to save, it will only // be a problem if a reconnect is attempted. Failure deferred until then. _cache_pin(state, pin, pin_len); } + if (tries) *tries = (sw & 0xf); return YKPIV_OK; - } else if ((sw >> 8) == 0x63) { + } + else if ((sw >> 8) == 0x63) { if (tries) *tries = (sw & 0xf); return YKPIV_WRONG_PIN; - } else if (sw == SW_ERR_AUTH_BLOCKED) { + } + else if (sw == SW_ERR_AUTH_BLOCKED) { if (tries) *tries = 0; return YKPIV_WRONG_PIN; - } else { + } + else { return YKPIV_GENERIC_ERROR; } } @@ -1385,10 +1395,10 @@ static ykpiv_rc _ykpiv_change_pin(ykpiv_state *state, int action, const char * c unsigned char data[0xff]; unsigned long recv_len = sizeof(data); ykpiv_rc res; - if (current_pin_len > 8) { + if (current_pin_len > CB_PIN_MAX) { return YKPIV_SIZE_ERROR; } - if (new_pin_len > 8) { + if (new_pin_len > CB_PIN_MAX) { return YKPIV_SIZE_ERROR; } if(action == CHREF_ACT_UNBLOCK_PIN) { @@ -1398,14 +1408,17 @@ static ykpiv_rc _ykpiv_change_pin(ykpiv_state *state, int action, const char * c templ[3] = 0x81; } memcpy(indata, current_pin, current_pin_len); - if(current_pin_len < 8) { - memset(indata + current_pin_len, 0xff, 8 - current_pin_len); + if(current_pin_len < CB_PIN_MAX) { + memset(indata + current_pin_len, 0xff, CB_PIN_MAX - current_pin_len); } - memcpy(indata + 8, new_pin, new_pin_len); - if(new_pin_len < 8) { - memset(indata + 8 + new_pin_len, 0xff, 8 - new_pin_len); + memcpy(indata + CB_PIN_MAX, new_pin, new_pin_len); + if(new_pin_len < CB_PIN_MAX) { + memset(indata + CB_PIN_MAX + new_pin_len, 0xff, CB_PIN_MAX - new_pin_len); } + res = ykpiv_transfer_data(state, templ, indata, sizeof(indata), data, &recv_len, &sw); + yc_memzero(indata, sizeof(indata)); + if(res != YKPIV_OK) { return res; } else if(sw != SW_SUCCESS) { From 340177f070440dd60261a7d44c1697f7ae9c4341 Mon Sep 17 00:00:00 2001 From: Dave Pate Date: Thu, 17 Jan 2019 15:49:21 -0800 Subject: [PATCH 06/13] lib: check that serial/version checks occur during select --- lib/ykpiv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/ykpiv.c b/lib/ykpiv.c index 2c640a9..2e416ce 100644 --- a/lib/ykpiv.c +++ b/lib/ykpiv.c @@ -291,8 +291,8 @@ ykpiv_rc _ykpiv_select_application(ykpiv_state *state) { * can determine how to get the serial number, which for the NEO/Yk4 * will result in another selection of the PIV applet. */ - _ykpiv_get_version(state, NULL); - _ykpiv_get_serial(state, NULL, false); + res = _ykpiv_get_version(state, NULL); + if (res == YKPIV_OK) res = _ykpiv_get_serial(state, NULL, false); return res; } From c4dbf9d02c3e38cb76965a0dd7bb98d61bc1d02b Mon Sep 17 00:00:00 2001 From: Dave Pate Date: Thu, 17 Jan 2019 15:59:30 -0800 Subject: [PATCH 07/13] lib: clear secrets in auth api --- lib/ykpiv.c | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/ykpiv.c b/lib/ykpiv.c index 2e416ce..854ccc1 100644 --- a/lib/ykpiv.c +++ b/lib/ykpiv.c @@ -1849,6 +1849,7 @@ ykpiv_rc ykpiv_auth_verifyresponse(ykpiv_state *state, uint8_t *response, const Cleanup: + yc_memzero(&apdu, sizeof(apdu)); _ykpiv_end_transaction(state); return res; } From a10ab1ace55aa70673c1f7d16324d582385476a1 Mon Sep 17 00:00:00 2001 From: Dave Pate Date: Mon, 21 Jan 2019 15:02:05 -0800 Subject: [PATCH 08/13] lib: correct zero memory defines, correct overflow checks in _write_certificate --- lib/internal.h | 7 +++++-- lib/util.c | 4 +++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/lib/internal.h b/lib/internal.h index 632052e..4f0dac7 100644 --- a/lib/internal.h +++ b/lib/internal.h @@ -240,9 +240,12 @@ void yc_log_event(uint32_t id, yc_log_level_t level, const char *sz_format, ...) #ifdef _WIN32 #include #define yc_memzero SecureZeroMemory -#elif __OPENBSD__ +#elif defined(BSD) #include -#define yc_memzero explicit_bzero; +#define yc_memzero explicit_bzero +#elif defined(__linux__) +#include +#define yc_memzero OPENSSL_cleanse #else #define __STDC_WANT_LIB_EXT1__ 1 #include diff --git a/lib/util.c b/lib/util.c index cc6da6f..11ce389 100644 --- a/lib/util.c +++ b/lib/util.c @@ -1399,8 +1399,10 @@ static ykpiv_rc _write_certificate(ykpiv_state *state, uint8_t slot, uint8_t *da // calculate the required length of the encoded object req_len = 1 /* cert tag */ + 3 /* compression tag + data*/ + 2 /* lrc */; req_len += _ykpiv_set_length(buf, data_len); + req_len += data_len; - if (req_len > _obj_size_max(state)) return YKPIV_SIZE_ERROR; + if (req_len < data_len) return YKPIV_SIZE_ERROR; /* detect overflow of unsigned size_t */ + if (req_len > _obj_size_max(state)) return YKPIV_SIZE_ERROR; /* obj_size_max includes limits for TLV encoding */ buf[offset++] = TAG_CERT; offset += _ykpiv_set_length(buf + offset, data_len); From 7ff30070172f53e2d810ceb951687e3a9abc0e7f Mon Sep 17 00:00:00 2001 From: Dave Pate Date: Tue, 22 Jan 2019 07:29:24 -0800 Subject: [PATCH 09/13] lib: clear secrets in ykpiv_import_private_key --- lib/ykpiv.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/ykpiv.c b/lib/ykpiv.c index 854ccc1..258fa0a 100644 --- a/lib/ykpiv.c +++ b/lib/ykpiv.c @@ -1695,7 +1695,8 @@ ykpiv_rc ykpiv_import_private_key(ykpiv_state *state, const unsigned char key, u padding = elem_len - lens[i]; remaining = (uintptr_t)key_data + sizeof(key_data) - (uintptr_t)in_ptr; if (padding > remaining) { - return YKPIV_ALGORITHM_ERROR; + res = YKPIV_ALGORITHM_ERROR; + goto Cleanup; } memset(in_ptr, 0, padding); in_ptr += padding; From f37cf3f462b81d0eb1434ad587075e19158cbbf3 Mon Sep 17 00:00:00 2001 From: Dave Pate Date: Tue, 22 Jan 2019 07:38:36 -0800 Subject: [PATCH 10/13] lib: clear secrets in set_protected_mgm --- lib/util.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/util.c b/lib/util.c index 11ce389..f14cc43 100644 --- a/lib/util.c +++ b/lib/util.c @@ -1184,7 +1184,7 @@ ykpiv_rc ykpiv_util_set_protected_mgm(ykpiv_state *state, ykpiv_mgm *mgm) { } } - if (YKPIV_OK != (res = _ykpiv_begin_transaction(state))) return YKPIV_PCSC_ERROR; + if (YKPIV_OK != (res = _ykpiv_begin_transaction(state))) { res = YKPIV_PCSC_ERROR; goto Cleanup; } if (YKPIV_OK != (res = _ykpiv_ensure_application_selected(state))) goto Cleanup; /* try to set the mgm key as long as we don't encounter a fatal error */ @@ -1279,8 +1279,8 @@ ykpiv_rc ykpiv_util_set_protected_mgm(ykpiv_state *state, ykpiv_mgm *mgm) { Cleanup: - memset(data, 0, sizeof(data)); - memset(mgm_key, 0, sizeof(mgm_key)); + yc_memzero(data, sizeof(data)); + yc_memzero(mgm_key, sizeof(mgm_key)); _ykpiv_end_transaction(state); return res; From afbe1b267032d2f4847e1d4523f57a6664c85e26 Mon Sep 17 00:00:00 2001 From: Dave Pate Date: Tue, 22 Jan 2019 07:53:22 -0800 Subject: [PATCH 11/13] lib: handle realloc failures safely --- lib/util.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/lib/util.c b/lib/util.c index f14cc43..de4caf9 100644 --- a/lib/util.c +++ b/lib/util.c @@ -218,6 +218,7 @@ ykpiv_rc ykpiv_util_list_keys(ykpiv_state *state, uint8_t *key_count, ykpiv_key ykpiv_rc res = YKPIV_OK; ykpiv_key *pKey = NULL; uint8_t *pData = NULL; + uint8_t *pTemp = NULL; size_t cbData = 0; size_t offset = 0; uint8_t buf[CB_BUF_MAX]; @@ -282,10 +283,13 @@ ykpiv_rc ykpiv_util_list_keys(ykpiv_state *state, uint8_t *key_count, ykpiv_key cbRealloc = (sizeof(ykpiv_key) + cbBuf - 1) > (cbData - offset) ? MAX((sizeof(ykpiv_key) + cbBuf - 1) - (cbData - offset), CB_PAGE) : 0; if (0 != cbRealloc) { - if (NULL == (pData = _ykpiv_realloc(state, pData, cbData + cbRealloc))) { + if (!(pTemp = _ykpiv_realloc(state, pData, cbData + cbRealloc))) { + /* realloc failed, pData will be freed in cleanup */ res = YKPIV_MEMORY_ERROR; goto Cleanup; } + pData = pTemp; + pTemp = NULL; } cbData += cbRealloc; @@ -556,6 +560,7 @@ ykpiv_rc ykpiv_util_read_msroots(ykpiv_state *state, uint8_t **data, size_t *dat int object_id = 0; uint8_t tag = 0; uint8_t *pData = NULL; + uint8_t *pTemp = NULL; size_t cbData = 0; size_t cbRealloc = 0; size_t offset = 0; @@ -606,10 +611,13 @@ ykpiv_rc ykpiv_util_read_msroots(ykpiv_state *state, uint8_t **data, size_t *dat cbRealloc = len > (cbData - offset) ? len - (cbData - offset) : 0; if (0 != cbRealloc) { - if (NULL == (pData = _ykpiv_realloc(state, pData, cbData + cbRealloc))) { + if (!(pTemp = _ykpiv_realloc(state, pData, cbData + cbRealloc))) { + /* realloc failed, pData will be freed in cleanup */ res = YKPIV_MEMORY_ERROR; goto Cleanup; } + pData = pTemp; + pTemp = NULL; } cbData += cbRealloc; From 5113a5ed02da3d16d2a683f08663ae9a9ecb4b41 Mon Sep 17 00:00:00 2001 From: Dave Pate Date: Tue, 22 Jan 2019 08:17:45 -0800 Subject: [PATCH 12/13] lib: tlv length buffer checks --- lib/internal.h | 1 + lib/ykpiv.c | 23 +++++++++++++++++++++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/lib/internal.h b/lib/internal.h index 4f0dac7..83d1b91 100644 --- a/lib/internal.h +++ b/lib/internal.h @@ -193,6 +193,7 @@ ykpiv_rc _ykpiv_ensure_application_selected(ykpiv_state *state); ykpiv_rc _ykpiv_select_application(ykpiv_state *state); unsigned int _ykpiv_set_length(unsigned char *buffer, size_t length); unsigned int _ykpiv_get_length(const unsigned char *buffer, size_t *len); +bool _ykpiv_has_valid_length(const unsigned char* buffer, size_t len); void* _ykpiv_alloc(ykpiv_state *state, size_t size); void* _ykpiv_realloc(ykpiv_state *state, void *address, size_t size); diff --git a/lib/ykpiv.c b/lib/ykpiv.c index 258fa0a..700ccc3 100644 --- a/lib/ykpiv.c +++ b/lib/ykpiv.c @@ -186,6 +186,19 @@ unsigned int _ykpiv_get_length(const unsigned char *buffer, size_t *len) { return 0; } +bool _ykpiv_has_valid_length(const unsigned char* buffer, size_t len) { + if ((buffer[0] < 0x81) && (len > 0)) { + return true; + } + else if (((*buffer & 0x7f) == 1) && (len > 1)) { + return true; + } + else if (((*buffer & 0x7f) == 2) && (len > 2)) { + return true; + } + return false; +} + static unsigned char *set_object(int object_id, unsigned char *buffer) { *buffer++ = 0x5c; if(object_id == YKPIV_OBJ_DISCOVERY) { @@ -1514,8 +1527,14 @@ ykpiv_rc _ykpiv_fetch_object(ykpiv_state *state, int object_id, } if(sw == SW_SUCCESS) { - size_t outlen; - unsigned int offs = _ykpiv_get_length(data + 1, &outlen); + size_t outlen = 0; + unsigned int offs = 0; + + if ((*len < 2) || !_ykpiv_has_valid_length(data + 1, *len - 1)) { + return YKPIV_SIZE_ERROR; + } + + offs = _ykpiv_get_length(data + 1, &outlen); if(offs == 0) { return YKPIV_SIZE_ERROR; } From 7b64528cf7ba87e803a3ed29c8ca877e88796e24 Mon Sep 17 00:00:00 2001 From: Dave Pate Date: Tue, 22 Jan 2019 13:59:06 -0800 Subject: [PATCH 13/13] lib: check tlv length encoding when reading complex data --- lib/util.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/util.c b/lib/util.c index de4caf9..3d4384c 100644 --- a/lib/util.c +++ b/lib/util.c @@ -1457,6 +1457,11 @@ static ykpiv_rc _get_metadata_item(uint8_t *data, size_t cb_data, uint8_t tag, u while (p_temp < (data + cb_data)) { tag_temp = *p_temp++; + + if (!_ykpiv_has_valid_length(p_temp, (data + cb_data - p_temp))) { + return YKPIV_SIZE_ERROR; + } + p_temp += _ykpiv_get_length(p_temp, &cb_temp); if (tag_temp == tag) {