From f86b334504693afe9ee6ec61416182d23c66e1ad Mon Sep 17 00:00:00 2001 From: Gabriel Kihlman Date: Mon, 27 Apr 2020 14:52:53 +0200 Subject: [PATCH] Initialize bufs to 0 to avoid potentially leaking uninitialized memory Based on a report from Christian Reitter doing fuzzing with MSAN. Extracts of logs: ==16111==WARNING: MemorySanitizer: use-of-uninitialized-value #0 0x4d59d4 in yk_write_to_key /yubikey-personalization/ykcore/ykcore.c:715:8 #1 0x4d9c00 in _yk_write /yubikey-personalization/ykcore/ykcore.c:233:7 #2 0x4dc74d in yk_write_scan_map /yubikey-personalization/ykcore/ykcore.c:357:9 #3 0x4ce352 in ykpersonalize_main /yubikey-personalization/ykpersonalize.c:423:9 [...] Uninitialized value was stored to memory at #0 0x45392b in __msan_memcpy (/yubikey-personalization/.libs/ykpersonalize+0x45392b) #1 0x4d52f7 in yk_write_to_key /yubikey-personalization/ykcore/ykcore.c:689:2 #2 0x4d9c00 in _yk_write /yubikey-personalization/ykcore/ykcore.c:233:7 #3 0x4dc74d in yk_write_scan_map /yubikey-personalization/ykcore/ykcore.c:357:9 #4 0x4ce352 in ykpersonalize_main /yubikey-personalization/ykpersonalize.c:423:9 [...] Uninitialized value was created by an allocation of 'scan_codes' in the stack frame of function 'ykpersonalize_main' ==18180==WARNING: MemorySanitizer: use-of-uninitialized-value #0 0x4d5a24 in yk_write_to_key /yubikey-personalization/ykcore/ykcore.c:715:8 #1 0x4d9c50 in _yk_write /yubikey-personalization/ykcore/ykcore.c:233:7 #2 0x4dae6c in yk_write_command /yubikey-personalization/ykcore/ykcore.c:288:8 #3 0x4cec93 in ykpersonalize_main /yubikey-personalization/ykpersonalize.c:440:10 Uninitialized value was stored to memory at #0 0x45392b in __msan_memcpy (/yubikey-personalization/.libs/ykpersonalize+0x45392b) #1 0x4d5347 in yk_write_to_key /yubikey-personalization/ykcore/ykcore.c:689:2 #2 0x4d9c50 in _yk_write /yubikey-personalization/ykcore/ykcore.c:233:7 #3 0x4dae6c in yk_write_command /yubikey-personalization/ykcore/ykcore.c:288:8 #4 0x4cec93 in ykpersonalize_main /yubikey-personalization/ykpersonalize.c:440:10 Uninitialized value was stored to memory at #0 0x45392b in __msan_memcpy (/yubikey-personalization/.libs/ykpersonalize+0x45392b) #1 0x4dacdb in yk_write_command /yubikey-personalization/ykcore/ykcore.c:280:3 #2 0x4cec93 in ykpersonalize_main /yubikey-personalization/ykpersonalize.c:440:10 Uninitialized value was stored to memory at #0 0x45392b in __msan_memcpy (/yubikey-personalization/.libs/ykpersonalize+0x45392b) #1 0x7f6fd2ea32f9 in ykp_set_fixed /yubikey-personalization/ykpers.c:787:1 #2 0x50193c in _set_fixed /yubikey-personalization/ykpers-args.c:900:2 #3 0x4ed040 in args_to_config /yubikey-personalization/ykpers-args.c:558:9 #4 0x4c865c in ykpersonalize_main /yubikey-personalization/ykpersonalize.c:167:8 Uninitialized value was created by an allocation of 'fixedbin' in the stack frame of function '_set_fixed' #0 0x501130 in _set_fixed /yubikey-personalization/ykpers-args.c:889 --- ykpers-args.c | 8 ++++---- ykpers.c | 12 ++++++------ ykpersonalize.c | 13 +++++++------ 3 files changed, 17 insertions(+), 16 deletions(-) diff --git a/ykpers-args.c b/ykpers-args.c index 53f7c22b..62ff7b2f 100644 --- a/ykpers-args.c +++ b/ykpers-args.c @@ -548,7 +548,7 @@ int args_to_config(int argc, char **argv, YKP_CONFIG *cfg, char *oathid, else if (strncmp(optarg, "uid", 3) == 0) { char *uid = optarg+4; size_t uidlen; - unsigned char uidbin[256]; + unsigned char uidbin[256] = {0}; size_t uidbinlen = 0; int rc; char *uidtmp = NULL; @@ -787,7 +787,7 @@ int args_to_config(int argc, char **argv, YKP_CONFIG *cfg, char *oathid, size_t key_bytes = (size_t)ykp_get_supported_key_length(cfg); int res = 0; char *key_tmp = NULL; - char keybuf[20]; + char keybuf[20] = {0}; if(keylocation == 2) { const char *prompt = " AES key, 16 bytes (32 characters hex) : "; @@ -865,7 +865,7 @@ int args_to_config(int argc, char **argv, YKP_CONFIG *cfg, char *oathid, static int _set_fixed(char *opt, YKP_CONFIG *cfg) { const char *fixed = opt; size_t fixedlen = strlen (fixed); - unsigned char fixedbin[256]; + unsigned char fixedbin[256] = {0}; size_t fixedbinlen = 0; int rc = hex_modhex_decode(fixedbin, &fixedbinlen, fixed, fixedlen, @@ -898,7 +898,7 @@ static int _format_decimal_as_hex(uint8_t *dst, size_t dst_len, uint8_t *src) /* For details, see YubiKey Manual 2010-09-16 section 5.3.4 - OATH-HOTP Token Identifier */ static int _format_oath_id(uint8_t *dst, size_t dst_len, uint8_t vendor, uint8_t type, uint32_t mui) { - uint8_t buf[8 + 1]; + uint8_t buf[8 + 1] = {0}; if (mui > 99999999) return 0; diff --git a/ykpers.c b/ykpers.c index 7941d0e3..81cb0dff 100644 --- a/ykpers.c +++ b/ykpers.c @@ -264,7 +264,7 @@ int ykp_get_supported_key_length(const YKP_CONFIG *cfg) /* Decode 128 bit AES key into cfg->ykcore_config.key */ int ykp_AES_key_from_hex(YKP_CONFIG *cfg, const char *hexkey) { - char aesbin[256]; + char aesbin[256] = {0}; /* Make sure that the hexkey is exactly 32 characters */ if (strlen(hexkey) != 32) { @@ -311,7 +311,7 @@ int ykp_HMAC_key_from_raw(YKP_CONFIG *cfg, const char *key) { * and 32 bits into the first four bytes of cfg->ykcore_config.uid. */ int ykp_HMAC_key_from_hex(YKP_CONFIG *cfg, const char *hexkey) { - char aesbin[256]; + char aesbin[256] = {0}; size_t i; /* Make sure that the hexkey is exactly 40 characters */ @@ -351,9 +351,9 @@ int ykp_AES_key_from_passphrase(YKP_CONFIG *cfg, const char *passphrase, 0 }; const char **random_place; - uint8_t _salt[8]; + uint8_t _salt[8] = {0}; size_t _salt_len = 0; - unsigned char buf[sizeof(cfg->ykcore_config.key) + 4]; + unsigned char buf[sizeof(cfg->ykcore_config.key) + 4] = {0}; int rc; int key_bytes = ykp_get_supported_key_length(cfg); YK_PRF_METHOD prf_method = {20, yk_hmac_sha1}; @@ -931,7 +931,7 @@ static const char str_extended_flags[] = "extended_flags"; static int _ykp_legacy_export_config(const YKP_CONFIG *cfg, char *buf, size_t len) { if (cfg) { - char buffer[256]; + char buffer[256] = {0}; struct map_st *p; unsigned char t_flags; bool key_bits_in_uid = false; @@ -1131,7 +1131,7 @@ int ykp_write_config(const YKP_CONFIG *cfg, void *userdata), void *userdata) { if(cfg) { - char buffer[1024]; + char buffer[1024] = {0}; int ret = _ykp_legacy_export_config(cfg, buffer, 1024); if(ret) { writer(buffer, strlen(buffer), userdata); diff --git a/ykpersonalize.c b/ykpersonalize.c index 15338c6a..2c991e32 100644 --- a/ykpersonalize.c +++ b/ykpersonalize.c @@ -48,17 +48,17 @@ int main(int argc, char **argv) FILE *outf = NULL; const char *outfname = NULL; int data_format = YKP_FORMAT_LEGACY; bool verbose = false; - unsigned char access_code[256]; + unsigned char access_code[256] = {0}; char *acc_code = NULL; char *new_acc_code = NULL; - unsigned char scan_codes[sizeof(SCAN_MAP)]; - unsigned char device_info[128]; + unsigned char scan_codes[sizeof(SCAN_MAP)] = {0}; + unsigned char device_info[128] = {0}; size_t device_info_len = 0; YK_KEY *yk = 0; YKP_CONFIG *cfg = ykp_alloc(); YK_STATUS *st = ykds_alloc(); bool autocommit = false; - char data[1024]; + char data[1024] = {0}; bool dry_run = false; /* Options */ @@ -184,7 +184,7 @@ int main(int argc, char **argv) } } if(new_acc_code) { - unsigned char accbin[256]; + unsigned char accbin[256] = {0}; size_t accbinlen = 0; int rc = hex_modhex_decode (accbin, &accbinlen, new_acc_code, strlen(new_acc_code), @@ -261,7 +261,8 @@ int main(int argc, char **argv) goto err; } } else { - char commitbuf[256]; size_t commitlen; + char commitbuf[256] = {0}; + size_t commitlen; if (ykp_command(cfg) == SLOT_SWAP) { fprintf(stderr, "Configuration in slot 1 and 2 will be swapped\n");