summaryrefslogtreecommitdiff
path: root/user/yubikey-personalization/uninit.patch
blob: 713ce1699ad8a009e77da3015894426e17b92cf8 (plain) (blame)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
From f86b334504693afe9ee6ec61416182d23c66e1ad Mon Sep 17 00:00:00 2001
From: Gabriel Kihlman <g.kihlman@yubico.com>
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");