Skip to content

Commit e3e5179

Browse files
authored
Merge pull request #9869 from JacobBarthelmeh/f356
fix for sanity checks on serial input
2 parents df50430 + cbf5264 commit e3e5179

3 files changed

Lines changed: 98 additions & 3 deletions

File tree

src/x509.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16082,13 +16082,13 @@ int wolfSSL_X509_set_serialNumber(WOLFSSL_X509* x509, WOLFSSL_ASN1_INTEGER* s)
1608216082

1608316083
/* WOLFSSL_ASN1_INTEGER has type | size | data
1608416084
* Sanity check that the data is actually in ASN format */
16085-
if (s->length < 3 && s->data[0] != ASN_INTEGER &&
16085+
if (s->length < 3 || s->data[0] != ASN_INTEGER ||
1608616086
s->data[1] != s->length - 2) {
1608716087
return WOLFSSL_FAILURE;
1608816088
}
1608916089
XMEMCPY(x509->serial, s->data + 2, s->length - 2);
1609016090
x509->serialSz = s->length - 2;
16091-
x509->serial[s->length] = 0;
16091+
x509->serial[x509->serialSz] = 0;
1609216092

1609316093
return WOLFSSL_SUCCESS;
1609416094
}

tests/api/test_x509.c

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,3 +243,96 @@ int test_x509_GetCAByAKID(void)
243243
#endif /* WOLFSSL_AKID_NAME */
244244
return EXPECT_RESULT();
245245
}
246+
247+
int test_x509_set_serialNumber(void)
248+
{
249+
#if defined(OPENSSL_EXTRA) || defined(OPENSSL_EXTRA_X509_SMALL)
250+
EXPECT_DECLS;
251+
WOLFSSL_X509* x509 = NULL;
252+
WOLFSSL_ASN1_INTEGER* s = NULL;
253+
#if defined(OPENSSL_EXTRA_X509_SMALL)
254+
WOLFSSL_ASN1_INTEGER asnInt;
255+
#endif
256+
257+
ExpectNotNull(x509 = wolfSSL_X509_new());
258+
#if defined(OPENSSL_EXTRA_X509_SMALL)
259+
XMEMSET(&asnInt, 0, sizeof(asnInt));
260+
asnInt.data = asnInt.intData;
261+
asnInt.isDynamic = 0;
262+
asnInt.dataMax = (unsigned int)sizeof(asnInt.intData);
263+
s = &asnInt;
264+
#else
265+
ExpectNotNull(s = wolfSSL_ASN1_INTEGER_new());
266+
#endif
267+
268+
/* --- invalid inputs that must be rejected --- */
269+
270+
/* NULL x509 */
271+
ExpectIntEQ(X509_set_serialNumber(NULL, s), WOLFSSL_FAILURE);
272+
/* NULL s */
273+
ExpectIntEQ(X509_set_serialNumber(x509, NULL), WOLFSSL_FAILURE);
274+
275+
if (s != NULL) {
276+
/* length == 0: too short */
277+
s->length = 0;
278+
s->data[0] = ASN_INTEGER;
279+
s->data[1] = 0;
280+
ExpectIntEQ(wolfSSL_X509_set_serialNumber(x509, s),
281+
WOLFSSL_FAILURE);
282+
283+
/* length == 1: still too short */
284+
s->length = 1;
285+
s->data[0] = ASN_INTEGER;
286+
s->data[1] = 0;
287+
ExpectIntEQ(wolfSSL_X509_set_serialNumber(x509, s),
288+
WOLFSSL_FAILURE);
289+
290+
/* length == 2: still rejected - the guard requires length >= 3 */
291+
s->length = 2;
292+
s->data[0] = ASN_INTEGER;
293+
s->data[1] = 0;
294+
ExpectIntEQ(wolfSSL_X509_set_serialNumber(x509, s),
295+
WOLFSSL_FAILURE);
296+
297+
/* wrong type byte */
298+
s->length = 4;
299+
s->data[0] = 0x00; /* not ASN_INTEGER */
300+
s->data[1] = 2; /* length field */
301+
s->data[2] = 0x01;
302+
s->data[3] = 0x02;
303+
ExpectIntEQ(wolfSSL_X509_set_serialNumber(x509, s),
304+
WOLFSSL_FAILURE);
305+
306+
/* mismatched length byte (data[1] != s->length - 2) */
307+
s->length = 4;
308+
s->data[0] = ASN_INTEGER;
309+
s->data[1] = 99; /* claims 99 bytes but s->length - 2 == 2 */
310+
s->data[2] = 0x01;
311+
s->data[3] = 0x02;
312+
ExpectIntEQ(wolfSSL_X509_set_serialNumber(x509, s),
313+
WOLFSSL_FAILURE);
314+
315+
/* --- valid two-byte serial number --- */
316+
s->length = 4;
317+
s->data[0] = ASN_INTEGER;
318+
s->data[1] = 2;
319+
s->data[2] = 0x01;
320+
s->data[3] = 0x02;
321+
ExpectIntEQ(wolfSSL_X509_set_serialNumber(x509, s),
322+
WOLFSSL_SUCCESS);
323+
ExpectIntEQ(x509->serialSz, 2);
324+
/* NUL terminator must be placed right after the copied data */
325+
ExpectIntEQ(x509->serial[x509->serialSz], 0);
326+
ExpectIntEQ(x509->serial[0], 0x01);
327+
ExpectIntEQ(x509->serial[1], 0x02);
328+
}
329+
330+
#if !defined(OPENSSL_EXTRA_X509_SMALL)
331+
wolfSSL_ASN1_INTEGER_free(s);
332+
#endif
333+
wolfSSL_X509_free(x509);
334+
return EXPECT_RESULT();
335+
#else
336+
return TEST_SKIPPED;
337+
#endif /* OPENSSL_EXTRA || OPENSSL_EXTRA_X509_SMALL */
338+
}

tests/api/test_x509.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,11 @@
2424

2525
int test_x509_rfc2818_verification_callback(void);
2626
int test_x509_GetCAByAKID(void);
27+
int test_x509_set_serialNumber(void);
2728

2829
#define TEST_X509_DECLS \
2930
TEST_DECL_GROUP("x509", test_x509_rfc2818_verification_callback), \
30-
TEST_DECL_GROUP("x509", test_x509_GetCAByAKID)
31+
TEST_DECL_GROUP("x509", test_x509_GetCAByAKID), \
32+
TEST_DECL_GROUP("x509", test_x509_set_serialNumber)
3133

3234
#endif /* WOLFCRYPT_TEST_X509_H */

0 commit comments

Comments
 (0)