Skip to content

Commit 58254f7

Browse files
sjaeckelsjaeckel
committed
fix SSH string implementation
1 parent f89909b commit 58254f7

File tree

8 files changed

+60
-38
lines changed

8 files changed

+60
-38
lines changed

‎src/headers/tomcrypt_private.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ int ecc_set_curve_by_size(int size, ecc_key *key);
240240
int ecc_import_subject_public_key_info(const unsigned char *in, unsigned long inlen, ecc_key *key);
241241

242242
#ifdef LTC_SSH
243-
int ecc_ssh_ecdsa_encode_name(char *buffer, unsigned long *buflen, const ecc_key *key);
243+
int ecc_ssh_ecdsa_encode_name(char *buffer, ulong32 *buflen, const ecc_key *key);
244244
#endif
245245

246246
/* low level functions */

‎src/misc/ssh/ssh_decode_sequence_multi.c‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
Decode a SSH sequence using a VA list
2121
@param in Data to decode
2222
@param inlen Length of buffer to decode
23-
@remark <...> is of the form <type, data> (int, void*) except for string <type, data, size>
23+
@remark <...> is of the form <type, data> (int, void*) except for string <type, data, size> (int, void*, ulong32*)
2424
@return CRYPT_OK on success
2525
*/
2626
int ssh_decode_sequence_multi(const unsigned char *in, unsigned long inlen, ...)
@@ -33,7 +33,7 @@ int ssh_decode_sequence_multi(const unsigned char *in, unsigned long inlen, ...)
3333
char *sdata;
3434
ulong32 *u32data;
3535
ulong64 *u64data;
36-
unsigned long bufsize;
36+
ulong32 *bufsize;
3737
ulong32 size;
3838

3939
LTC_ARGCHK(in != NULL);
@@ -115,17 +115,20 @@ int ssh_decode_sequence_multi(const unsigned char *in, unsigned long inlen, ...)
115115
case LTC_SSHDATA_STRING:
116116
case LTC_SSHDATA_NAMELIST:
117117
sdata = va_arg(args, char*);
118-
bufsize = va_arg(args, unsigned long);
119-
if (size >= bufsize) {
118+
bufsize = va_arg(args, ulong32*);
119+
if (bufsize == NULL) {
120+
err = CRYPT_INVALID_ARG;
121+
goto error;
122+
}
123+
if (size + 1 >= *bufsize) {
120124
err = CRYPT_BUFFER_OVERFLOW;
121125
goto error;
122126
}
123127
if (size > 0) {
124-
XSTRNCPY(sdata, (const char *)in, size);
125-
sdata[size] = '\0'; /* strncpy doesn't NUL-terminate */
126-
} else {
127-
*sdata = '\0';
128+
XMEMCPY(sdata, (const char *)in, size);
128129
}
130+
sdata[size] = '\0';
131+
*bufsize = size;
129132
in += size;
130133
break;
131134
case LTC_SSHDATA_MPINT:

‎src/misc/ssh/ssh_encode_sequence_multi.c‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
Encode a SSH sequence using a VA list
2121
@param out [out] Destination for data
2222
@param outlen [in/out] Length of buffer and resulting length of output
23-
@remark <...> is of the form <type, data> (int, void*)
23+
@remark <...> is of the form <type, data> (int, void*) except for string <type, data, size> (int, void*, ulong32)
2424
@return CRYPT_OK on success
2525
*/
2626
int ssh_encode_sequence_multi(unsigned char *out, unsigned long *outlen, ...)
@@ -29,9 +29,10 @@ int ssh_encode_sequence_multi(unsigned char *out, unsigned long *outlen, ...)
2929
va_list args;
3030
ulong32 size;
3131
ssh_data_type type;
32-
void *vdata;
33-
const char *sdata;
32+
void *vdata;
33+
const char *sdata;
3434
int idata;
35+
ulong32 *psize;
3536
ulong32 u32data;
3637
ulong64 u64data;
3738

@@ -58,9 +59,9 @@ int ssh_encode_sequence_multi(unsigned char *out, unsigned long *outlen, ...)
5859
break;
5960
case LTC_SSHDATA_STRING:
6061
case LTC_SSHDATA_NAMELIST:
61-
sdata = va_arg(args, char*);
62+
LTC_UNUSED_PARAM( va_arg(args, char*) );
63+
size += va_arg(args, ulong32);
6264
size += 4;
63-
size += strlen(sdata);
6465
break;
6566
case LTC_SSHDATA_MPINT:
6667
vdata = va_arg(args, void*);
@@ -118,7 +119,7 @@ int ssh_encode_sequence_multi(unsigned char *out, unsigned long *outlen, ...)
118119
case LTC_SSHDATA_STRING:
119120
case LTC_SSHDATA_NAMELIST:
120121
sdata = va_arg(args, char*);
121-
size = strlen(sdata);
122+
size = va_arg(args, ulong32);
122123
STORE32H(size, out);
123124
out += 4;
124125
XMEMCPY(out, sdata, size);

‎src/pk/ecc/ecc_recover_key.c‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -114,19 +114,20 @@ int ecc_recover_key(const unsigned char *sig, unsigned long siglen,
114114
#ifdef LTC_SSH
115115
else if (sigformat == LTC_ECCSIG_RFC5656) {
116116
char name[64], name2[64];
117-
unsigned long namelen = sizeof(name2);
117+
ulong32 namelen = sizeof(name);
118+
ulong32 name2len = sizeof(name2);
118119

119120
/* Decode as SSH data sequence, per RFC4251 */
120121
if ((err = ssh_decode_sequence_multi(sig, siglen,
121-
LTC_SSHDATA_STRING, name, 64,
122+
LTC_SSHDATA_STRING, name, &namelen,
122123
LTC_SSHDATA_MPINT, r,
123124
LTC_SSHDATA_MPINT, s,
124125
LTC_SSHDATA_EOL, NULL)) != CRYPT_OK) { goto error; }
125126

126127

127128
/* Check curve matches identifier string */
128-
if ((err = ecc_ssh_ecdsa_encode_name(name2, &namelen, key)) != CRYPT_OK) { goto error; }
129-
if (XSTRCMP(name,name2) != 0) {
129+
if ((err = ecc_ssh_ecdsa_encode_name(name2, &name2len, key)) != CRYPT_OK) { goto error; }
130+
if ((namelen != name2len) || (XSTRCMP(name, name2) != 0)) {
130131
err = CRYPT_INVALID_ARG;
131132
goto error;
132133
}

‎src/pk/ecc/ecc_sign_hash.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -159,12 +159,12 @@ int ecc_sign_hash_ex(const unsigned char *in, unsigned long inlen,
159159
else if (sigformat == LTC_ECCSIG_RFC5656) {
160160
/* Get identifier string */
161161
char name[64];
162-
unsigned long namelen = sizeof(name);
162+
ulong32 namelen = sizeof(name);
163163
if ((err = ecc_ssh_ecdsa_encode_name(name, &namelen, key)) != CRYPT_OK) { goto errnokey; }
164164

165165
/* Store as SSH data sequence, per RFC4251 */
166166
err = ssh_encode_sequence_multi(out, outlen,
167-
LTC_SSHDATA_STRING, name,
167+
LTC_SSHDATA_STRING, name, namelen,
168168
LTC_SSHDATA_MPINT, r,
169169
LTC_SSHDATA_MPINT, s,
170170
LTC_SSHDATA_EOL, NULL);

‎src/pk/ecc/ecc_ssh_ecdsa_encode_name.c‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,11 @@
2121
@param key A public or private ECC key
2222
@return CRYPT_OK if successful
2323
*/
24-
int ecc_ssh_ecdsa_encode_name(char *buffer, unsigned long *buflen, const ecc_key *key)
24+
int ecc_ssh_ecdsa_encode_name(char *buffer, ulong32 *buflen, const ecc_key *key)
2525
{
2626
char oidstr[64];
2727
unsigned long oidlen = sizeof(oidstr);
28-
unsigned long size = 0;
29-
int err;
28+
int err, size = 0;
3029

3130
LTC_ARGCHK(buffer != NULL);
3231
LTC_ARGCHK(buflen != NULL);
@@ -52,8 +51,11 @@ int ecc_ssh_ecdsa_encode_name(char *buffer, unsigned long *buflen, const ecc_key
5251
size = snprintf(buffer, *buflen, "ecdsa-sha2-%s", oidstr);
5352
}
5453

55-
/* snprintf returns size that would have been written, but limits to buflen-1 chars plus terminator */
56-
if (size >= *buflen) {
54+
/* snprintf returns a negative value on error
55+
* or the size that would have been written, but limits to buflen-1 chars plus terminator */
56+
if (size < 0) {
57+
err = CRYPT_ERROR;
58+
} else if ((unsigned)size >= *buflen) {
5759
err = CRYPT_BUFFER_OVERFLOW;
5860
} else {
5961
err = CRYPT_OK;

‎src/pk/ecc/ecc_verify_hash.c‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -100,19 +100,20 @@ int ecc_verify_hash_ex(const unsigned char *sig, unsigned long siglen,
100100
#ifdef LTC_SSH
101101
else if (sigformat == LTC_ECCSIG_RFC5656) {
102102
char name[64], name2[64];
103-
unsigned long namelen = sizeof(name2);
103+
ulong32 namelen = sizeof(name);
104+
ulong32 name2len = sizeof(name2);
104105

105106
/* Decode as SSH data sequence, per RFC4251 */
106107
if ((err = ssh_decode_sequence_multi(sig, siglen,
107-
LTC_SSHDATA_STRING, name, 64,
108+
LTC_SSHDATA_STRING, name, &namelen,
108109
LTC_SSHDATA_MPINT, r,
109110
LTC_SSHDATA_MPINT, s,
110111
LTC_SSHDATA_EOL, NULL)) != CRYPT_OK) { goto error; }
111112

112113

113114
/* Check curve matches identifier string */
114-
if ((err = ecc_ssh_ecdsa_encode_name(name2, &namelen, key)) != CRYPT_OK) { goto error; }
115-
if (XSTRCMP(name,name2) != 0) {
115+
if ((err = ecc_ssh_ecdsa_encode_name(name2, &name2len, key)) != CRYPT_OK) { goto error; }
116+
if ((namelen != name2len) || (XSTRCMP(name, name2) != 0)) {
116117
err = CRYPT_INVALID_ARG;
117118
goto error;
118119
}

‎tests/ssh_test.c‎

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ static int _ssh_encoding_test(void)
6161
{
6262
unsigned char buffer[BUFSIZE];
6363
unsigned long buflen;
64+
ulong32 len;
6465
void *v, *zero;
6566
int err;
6667

@@ -124,8 +125,9 @@ static int _ssh_encoding_test(void)
124125
/* string */
125126
buflen = BUFSIZE;
126127
zeromem(buffer, BUFSIZE);
128+
len = strlen("testing");
127129
DO(ssh_encode_sequence_multi(buffer, &buflen,
128-
LTC_SSHDATA_STRING, "testing",
130+
LTC_SSHDATA_STRING, "testing", len,
129131
LTC_SSHDATA_EOL, NULL));
130132
COMPARE_TESTVECTOR(buffer, buflen, string, sizeof(string), "enc-string", 1);
131133

@@ -165,22 +167,25 @@ static int _ssh_encoding_test(void)
165167
/* name-list */
166168
buflen = BUFSIZE;
167169
zeromem(buffer, BUFSIZE);
170+
len = strlen("");
168171
DO(ssh_encode_sequence_multi(buffer, &buflen,
169-
LTC_SSHDATA_NAMELIST, "",
172+
LTC_SSHDATA_NAMELIST, "", len,
170173
LTC_SSHDATA_EOL, NULL));
171174
COMPARE_TESTVECTOR(buffer, buflen, nlist1, sizeof(nlist1), "enc-nlist", 1);
172175

173176
buflen = BUFSIZE;
174177
zeromem(buffer, BUFSIZE);
178+
len = strlen("zlib");
175179
DO(ssh_encode_sequence_multi(buffer, &buflen,
176-
LTC_SSHDATA_NAMELIST, "zlib",
180+
LTC_SSHDATA_NAMELIST, "zlib", len,
177181
LTC_SSHDATA_EOL, NULL));
178182
COMPARE_TESTVECTOR(buffer, buflen, nlist2, sizeof(nlist2), "enc-nlist", 2);
179183

180184
buflen = BUFSIZE;
181185
zeromem(buffer, BUFSIZE);
186+
len = strlen("zlib,none");
182187
DO(ssh_encode_sequence_multi(buffer, &buflen,
183-
LTC_SSHDATA_NAMELIST, "zlib,none",
188+
LTC_SSHDATA_NAMELIST, "zlib,none", len,
184189
LTC_SSHDATA_EOL, NULL));
185190
COMPARE_TESTVECTOR(buffer, buflen, nlist3, sizeof(nlist3), "enc-nlist", 3);
186191

@@ -195,6 +200,7 @@ static int _ssh_decoding_test(void)
195200
{
196201
char strbuf[BUFSIZE];
197202
void *u, *v;
203+
ulong32 size;
198204
ulong32 tmp32;
199205
ulong64 tmp64;
200206
unsigned char tmp8;
@@ -236,9 +242,11 @@ static int _ssh_decoding_test(void)
236242

237243
/* string */
238244
zeromem(strbuf, BUFSIZE);
245+
size = BUFSIZE;
239246
DO(ssh_decode_sequence_multi(string, sizeof(string),
240-
LTC_SSHDATA_STRING, strbuf, BUFSIZE,
247+
LTC_SSHDATA_STRING, strbuf, &size,
241248
LTC_SSHDATA_EOL, NULL));
249+
ENSURE(strlen("testing") == size);
242250
ENSURE(XSTRCMP(strbuf, "testing") == 0);
243251

244252
/* mpint */
@@ -266,21 +274,27 @@ static int _ssh_decoding_test(void)
266274

267275
/* name-list */
268276
zeromem(strbuf, BUFSIZE);
277+
size = BUFSIZE;
269278
DO(ssh_decode_sequence_multi(nlist1, sizeof(nlist1),
270-
LTC_SSHDATA_NAMELIST, strbuf, BUFSIZE,
279+
LTC_SSHDATA_NAMELIST, strbuf, &size,
271280
LTC_SSHDATA_EOL, NULL));
281+
ENSURE(strlen("") == size);
272282
ENSURE(XSTRCMP(strbuf, "") == 0);
273283

274284
zeromem(strbuf, BUFSIZE);
285+
size = BUFSIZE;
275286
DO(ssh_decode_sequence_multi(nlist2, sizeof(nlist2),
276-
LTC_SSHDATA_NAMELIST, strbuf, BUFSIZE,
287+
LTC_SSHDATA_NAMELIST, strbuf, &size,
277288
LTC_SSHDATA_EOL, NULL));
289+
ENSURE(strlen("zlib") == size);
278290
ENSURE(XSTRCMP(strbuf, "zlib") == 0);
279291

280292
zeromem(strbuf, BUFSIZE);
293+
size = BUFSIZE;
281294
DO(ssh_decode_sequence_multi(nlist3, sizeof(nlist3),
282-
LTC_SSHDATA_NAMELIST, strbuf, BUFSIZE,
295+
LTC_SSHDATA_NAMELIST, strbuf, &size,
283296
LTC_SSHDATA_EOL, NULL));
297+
ENSURE(strlen("zlib,none") == size);
284298
ENSURE(XSTRCMP(strbuf, "zlib,none") == 0);
285299

286300

0 commit comments

Comments
 (0)