Skip to content

Commit 2fc1110

Browse files
dgarskedgarske
authored
Merge pull request #8587 from lealem47/gh8574
Fix bug in ParseCRL_Extensions
2 parents 701e3ba + 02a4969 commit 2fc1110

7 files changed

Lines changed: 220 additions & 108 deletions

File tree

‎src/crl.c‎

Lines changed: 62 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,10 @@ static int InitCRL_Entry(CRL_Entry* crle, DecodedCRL* dcrl, const byte* buff,
135135
#endif
136136
dcrl->certs = NULL;
137137
crle->totalCerts = dcrl->totalCerts;
138-
crle->crlNumber = dcrl->crlNumber;
138+
crle->crlNumberSet = dcrl->crlNumberSet;
139+
if (crle->crlNumberSet) {
140+
XMEMCPY(crle->crlNumber, dcrl->crlNumber, CRL_MAX_NUM_SZ);
141+
}
139142
crle->verified = verified;
140143
if (!verified) {
141144
crle->tbsSz = dcrl->sigIndex - dcrl->certBegin;
@@ -587,7 +590,9 @@ static void SetCrlInfo(CRL_Entry* entry, CrlInfo *info)
587590
info->nextDate = (byte *)entry->nextDate;
588591
info->nextDateMaxLen = MAX_DATE_SIZE;
589592
info->nextDateFormat = entry->nextDateFormat;
590-
info->crlNumber = (sword32)entry->crlNumber;
593+
info->crlNumberSet = entry->crlNumberSet;
594+
if (info->crlNumberSet)
595+
XMEMCPY(info->crlNumber, entry->crlNumber, CRL_MAX_NUM_SZ);
591596
}
592597

593598
static void SetCrlInfoFromDecoded(DecodedCRL* entry, CrlInfo *info)
@@ -600,10 +605,55 @@ static void SetCrlInfoFromDecoded(DecodedCRL* entry, CrlInfo *info)
600605
info->nextDate = (byte *)entry->nextDate;
601606
info->nextDateMaxLen = MAX_DATE_SIZE;
602607
info->nextDateFormat = entry->nextDateFormat;
603-
info->crlNumber = (sword32)entry->crlNumber;
608+
info->crlNumberSet = entry->crlNumberSet;
609+
if (info->crlNumberSet)
610+
XMEMCPY(info->crlNumber, entry->crlNumber, CRL_MAX_NUM_SZ);
604611
}
605612
#endif
606613

614+
/* Returns MP_GT if prev crlNumber is smaller
615+
* MP_EQ if equal
616+
* MP_LT if prev crlNumber is larger */
617+
static int CompareCRLnumber(CRL_Entry* prev, CRL_Entry* curr)
618+
{
619+
int ret = 0;
620+
DECL_MP_INT_SIZE_DYN(prev_num, CRL_MAX_NUM_SZ * CHAR_BIT,
621+
CRL_MAX_NUM_SZ * CHAR_BIT);
622+
DECL_MP_INT_SIZE_DYN(curr_num, CRL_MAX_NUM_SZ * CHAR_BIT,
623+
CRL_MAX_NUM_SZ * CHAR_BIT);
624+
625+
NEW_MP_INT_SIZE(prev_num, CRL_MAX_NUM_SZ * CHAR_BIT, NULL,
626+
DYNAMIC_TYPE_TMP_BUFFER);
627+
NEW_MP_INT_SIZE(curr_num, CRL_MAX_NUM_SZ * CHAR_BIT, NULL,
628+
DYNAMIC_TYPE_TMP_BUFFER);
629+
#ifdef MP_INT_SIZE_CHECK_NULL
630+
if ((prev_num == NULL) || (curr_num == NULL)) {
631+
ret = MEMORY_E;
632+
}
633+
#endif
634+
635+
if (ret == 0 && ((INIT_MP_INT_SIZE(prev_num, CRL_MAX_NUM_SZ * CHAR_BIT)
636+
!= MP_OKAY) || (INIT_MP_INT_SIZE(curr_num,
637+
CRL_MAX_NUM_SZ * CHAR_BIT)) != MP_OKAY)) {
638+
ret = MP_INIT_E;
639+
}
640+
641+
if (ret == 0 && (mp_read_radix(prev_num, (char*)prev->crlNumber,
642+
MP_RADIX_HEX) != MP_OKAY ||
643+
mp_read_radix(curr_num, (char*)curr->crlNumber,
644+
MP_RADIX_HEX) != MP_OKAY)) {
645+
ret = BAD_FUNC_ARG;
646+
}
647+
648+
if (ret == 0)
649+
ret = mp_cmp(prev_num, curr_num);
650+
651+
FREE_MP_INT_SIZE(prev_num, NULL, DYNAMIC_TYPE_TMP_BUFFER);
652+
FREE_MP_INT_SIZE(curr_num, NULL, DYNAMIC_TYPE_TMP_BUFFER);
653+
654+
return ret;
655+
}
656+
607657
/* Add Decoded CRL, 0 on success */
608658
static int AddCRL(WOLFSSL_CRL* crl, DecodedCRL* dcrl, const byte* buff,
609659
int verified)
@@ -615,6 +665,7 @@ static int AddCRL(WOLFSSL_CRL* crl, DecodedCRL* dcrl, const byte* buff,
615665
CrlInfo old;
616666
CrlInfo cnew;
617667
#endif
668+
int ret = 0;
618669

619670
WOLFSSL_ENTER("AddCRL");
620671

@@ -645,12 +696,19 @@ static int AddCRL(WOLFSSL_CRL* crl, DecodedCRL* dcrl, const byte* buff,
645696

646697
for (curr = crl->crlList; curr != NULL; curr = curr->next) {
647698
if (XMEMCMP(curr->issuerHash, crle->issuerHash, CRL_DIGEST_SIZE) == 0) {
648-
if (crle->crlNumber <= curr->crlNumber) {
699+
ret = CompareCRLnumber(crle, curr);
700+
/* Error out if the CRL we're attempting to add isn't more
701+
* authoritative than the existing entry */
702+
if (ret == MP_LT || ret == MP_EQ) {
649703
WOLFSSL_MSG("Same or newer CRL entry already exists");
650704
CRL_Entry_free(crle, crl->heap);
651705
wc_UnLockRwLock(&crl->crlLock);
652706
return BAD_FUNC_ARG;
653707
}
708+
else if (ret < 0) {
709+
WOLFSSL_MSG("Error comparing CRL Numbers");
710+
return ret;
711+
}
654712

655713
crle->next = curr->next;
656714
if (prev != NULL) {

‎src/x509.c‎

Lines changed: 87 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -6423,8 +6423,7 @@ static int X509PrintSerial_ex(WOLFSSL_BIO* bio, byte* serial, int sz,
64236423
scratch + scratchLen, scratchSz - scratchLen,
64246424
"%02x%s", serial[i], (i < sz - 1) ?
64256425
(delimiter ? ":" : "") : "\n"))
6426-
>= scratchSz - scratchLen)
6427-
{
6426+
>= scratchSz - scratchLen) {
64286427
WOLFSSL_MSG("buffer overrun");
64296428
return WOLFSSL_FAILURE;
64306429
}
@@ -6437,10 +6436,8 @@ static int X509PrintSerial_ex(WOLFSSL_BIO* bio, byte* serial, int sz,
64376436

64386437
/* if serial can fit into byte then print on the same line */
64396438
else {
6440-
if ((scratchLen = XSNPRINTF(
6441-
scratch, MAX_WIDTH, " %d (0x%x)\n", serial[0], serial[0]))
6442-
>= MAX_WIDTH)
6443-
{
6439+
if ((scratchLen = XSNPRINTF(scratch, MAX_WIDTH, " %d (0x%x)\n",
6440+
(char)serial[0], serial[0])) >= MAX_WIDTH) {
64446441
WOLFSSL_MSG("buffer overrun");
64456442
return WOLFSSL_FAILURE;
64466443
}
@@ -8879,85 +8876,135 @@ static int X509CRLPrintExtensions(WOLFSSL_BIO* bio, WOLFSSL_X509_CRL* crl,
88798876
int indent)
88808877
{
88818878
char tmp[MAX_WIDTH]; /* buffer for XSNPRINTF */
8879+
int ret = 0;
88828880

88838881
if (XSNPRINTF(tmp, MAX_WIDTH, "%*s%s\n", indent, "",
88848882
"CRL extensions:") >= MAX_WIDTH) {
8885-
return WOLFSSL_FAILURE;
8883+
ret = WOLFSSL_FAILURE;
88868884
}
88878885

8888-
if (wolfSSL_BIO_write(bio, tmp, (int)XSTRLEN(tmp)) <= 0) {
8889-
return WOLFSSL_FAILURE;
8886+
if (ret == 0 && wolfSSL_BIO_write(bio, tmp, (int)XSTRLEN(tmp)) <= 0) {
8887+
ret = WOLFSSL_FAILURE;
88908888
}
88918889

8892-
if (crl->crlList->crlNumber) {
8893-
if (XSNPRINTF(tmp, MAX_WIDTH, "%*s%s\n", indent + 4, "",
8890+
if (ret == 0 && crl->crlList->crlNumberSet) {
8891+
char dec_string[49]; /* 20 octets can express numbers up to approx
8892+
49 decimal digits */
8893+
int freeMp = 0;
8894+
#ifdef WOLFSSL_SMALL_STACK
8895+
mp_int* dec_num = (mp_int*)XMALLOC(sizeof(*dec_num), NULL,
8896+
DYNAMIC_TYPE_BIGINT);
8897+
if (dec_num == NULL) {
8898+
ret = MEMORY_E;
8899+
}
8900+
#else
8901+
mp_int dec_num[1];
8902+
#endif
8903+
8904+
if (ret == 0 && (mp_init(dec_num) != MP_OKAY)) {
8905+
ret = MP_INIT_E;
8906+
}
8907+
else if (ret == 0) {
8908+
freeMp = 1;
8909+
}
8910+
8911+
if (ret == 0 && mp_read_radix(dec_num, (char *)crl->crlList->crlNumber,
8912+
MP_RADIX_HEX) != MP_OKAY) {
8913+
ret = WOLFSSL_FAILURE;
8914+
}
8915+
8916+
if (ret == 0 && mp_toradix(dec_num, dec_string, MP_RADIX_DEC)
8917+
!= MP_OKAY) {
8918+
ret = WOLFSSL_FAILURE;
8919+
}
8920+
8921+
if (ret == 0 && XSNPRINTF(tmp, MAX_WIDTH, "%*s%s\n", indent + 4, "",
88948922
"X509v3 CRL Number:") >= MAX_WIDTH) {
8895-
return WOLFSSL_FAILURE;
8923+
ret = WOLFSSL_FAILURE;
88968924
}
88978925

8898-
if (wolfSSL_BIO_write(bio, tmp, (int)XSTRLEN(tmp)) <= 0) {
8899-
return WOLFSSL_FAILURE;
8926+
if (ret == 0 && wolfSSL_BIO_write(bio, tmp, (int)XSTRLEN(tmp)) <= 0) {
8927+
ret = WOLFSSL_FAILURE;
89008928
}
89018929

8902-
if (XSNPRINTF(tmp, MAX_WIDTH, "%*s%d\n", indent + 8, "",
8903-
crl->crlList->crlNumber) >= MAX_WIDTH)
8904-
{
8905-
return WOLFSSL_FAILURE;
8930+
if (ret == 0 && XSNPRINTF(tmp, MAX_WIDTH, "%*s%s\n", indent + 8, "",
8931+
dec_string) >= MAX_WIDTH) {
8932+
ret = WOLFSSL_FAILURE;
89068933
}
8907-
if (wolfSSL_BIO_write(bio, tmp, (int)XSTRLEN(tmp)) <= 0) {
8908-
return WOLFSSL_FAILURE;
8934+
8935+
if (ret == 0 && wolfSSL_BIO_write(bio, tmp, (int)XSTRLEN(tmp)) <= 0) {
8936+
ret = WOLFSSL_FAILURE;
89098937
}
8938+
89108939
XMEMSET(tmp, 0, sizeof(tmp));
8940+
8941+
if (freeMp) {
8942+
mp_free(dec_num);
8943+
}
8944+
8945+
#ifdef WOLFSSL_SMALL_STACK
8946+
XFREE(dec_num, NULL, DYNAMIC_TYPE_BIGINT);
8947+
#endif
89118948
}
89128949

89138950
#if !defined(NO_SKID)
8914-
if (crl->crlList->extAuthKeyIdSet && crl->crlList->extAuthKeyId[0] != 0) {
8951+
if (ret == 0 && crl->crlList->extAuthKeyIdSet &&
8952+
crl->crlList->extAuthKeyId[0] != 0) {
89158953
word32 i;
89168954
char val[5];
89178955
int valSz = 5;
89188956

89198957
if (XSNPRINTF(tmp, MAX_WIDTH, "%*s%s", indent + 4, "",
89208958
"X509v3 Authority Key Identifier:") >= MAX_WIDTH) {
8921-
return WOLFSSL_FAILURE;
8959+
ret = WOLFSSL_FAILURE;
89228960
}
89238961

8924-
XSTRNCAT(tmp, "\n", MAX_WIDTH - XSTRLEN(tmp) - 1);
8962+
if (ret == 0) {
8963+
XSTRNCAT(tmp, "\n", MAX_WIDTH - XSTRLEN(tmp) - 1);
8964+
}
89258965

8926-
if (wolfSSL_BIO_write(bio, tmp, (int)XSTRLEN(tmp)) <= 0) {
8927-
return WOLFSSL_FAILURE;
8966+
if (ret == 0 && wolfSSL_BIO_write(bio, tmp, (int)XSTRLEN(tmp)) <= 0) {
8967+
ret = WOLFSSL_FAILURE;
89288968
}
89298969
XMEMSET(tmp, 0, MAX_WIDTH);
89308970

8931-
if (XSNPRINTF(tmp, MAX_WIDTH - 1, "%*s%s",
8971+
if (ret == 0 && XSNPRINTF(tmp, MAX_WIDTH - 1, "%*s%s",
89328972
indent + 8, "", "keyid") >= MAX_WIDTH) {
8933-
return WOLFSSL_FAILURE;
8973+
ret = WOLFSSL_FAILURE;
89348974
}
89358975

89368976

89378977
for (i = 0; i < XSTRLEN((char*)crl->crlList->extAuthKeyId); i++) {
89388978
/* check if buffer is almost full */
8939-
if (XSTRLEN(tmp) >= sizeof(tmp) - valSz) {
8979+
if (ret == 0 && XSTRLEN(tmp) >= sizeof(tmp) - valSz) {
89408980
if (wolfSSL_BIO_write(bio, tmp, (int)XSTRLEN(tmp)) <= 0) {
8941-
return WOLFSSL_FAILURE;
8981+
ret = WOLFSSL_FAILURE;
89428982
}
89438983
tmp[0] = '\0';
89448984
}
8945-
if (XSNPRINTF(val, (size_t)valSz, ":%02X",
8946-
crl->crlList->extAuthKeyId[i]) >= valSz)
8947-
{
8985+
if (ret == 0 && XSNPRINTF(val, (size_t)valSz, ":%02X",
8986+
crl->crlList->extAuthKeyId[i]) >= valSz) {
89488987
WOLFSSL_MSG("buffer overrun");
8949-
return WOLFSSL_FAILURE;
8988+
ret = WOLFSSL_FAILURE;
8989+
}
8990+
if (ret == 0) {
8991+
XSTRNCAT(tmp, val, valSz);
89508992
}
8951-
XSTRNCAT(tmp, val, valSz);
89528993
}
8953-
XSTRNCAT(tmp, "\n", XSTRLEN("\n") + 1);
8954-
if (wolfSSL_BIO_write(bio, tmp, (int)XSTRLEN(tmp)) <= 0) {
8955-
return WOLFSSL_FAILURE;
8994+
if (ret == 0) {
8995+
XSTRNCAT(tmp, "\n", XSTRLEN("\n") + 1);
8996+
}
8997+
if (ret == 0 && wolfSSL_BIO_write(bio, tmp, (int)XSTRLEN(tmp)) <= 0) {
8998+
ret = WOLFSSL_FAILURE;
89568999
}
89579000
}
89589001
#endif
89599002

8960-
return WOLFSSL_SUCCESS;
9003+
if (ret == 0) {
9004+
ret = WOLFSSL_SUCCESS;
9005+
}
9006+
9007+
return ret;
89619008
}
89629009

89639010
/* iterate through a CRL's Revoked Certs and print out in human
@@ -9189,7 +9236,7 @@ void wolfSSL_X509_CRL_free(WOLFSSL_X509_CRL *crl)
91899236
}
91909237
#endif /* HAVE_CRL && (OPENSSL_EXTRA || WOLFSSL_WPAS_SMALL) */
91919238

9192-
#ifdef OPENSSL_EXTRA
9239+
#if defined(HAVE_CRL) && defined(OPENSSL_EXTRA)
91939240
WOLFSSL_ASN1_TIME* wolfSSL_X509_CRL_get_lastUpdate(WOLFSSL_X509_CRL* crl)
91949241
{
91959242
if ((crl != NULL) && (crl->crlList != NULL) &&
@@ -9219,7 +9266,7 @@ int wolfSSL_X509_CRL_verify(WOLFSSL_X509_CRL* crl, WOLFSSL_EVP_PKEY* key)
92199266
return 0;
92209267
}
92219268
#endif
9222-
#endif /* OPENSSL_EXTRA */
9269+
#endif /* HAVE_CRL && OPENSSL_EXTRA */
92239270

92249271
#ifdef OPENSSL_EXTRA
92259272

‎tests/api.c‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43270,7 +43270,8 @@ static int test_wolfSSL_X509V3_set_ctx(void)
4327043270
{
4327143271
EXPECT_DECLS;
4327243272
#if (defined(OPENSSL_ALL) || defined(OPENSSL_EXTRA)) && \
43273-
defined(WOLFSSL_CERT_GEN) && defined(WOLFSSL_CERT_REQ)
43273+
defined(WOLFSSL_CERT_GEN) && defined(WOLFSSL_CERT_REQ) && \
43274+
defined(HAVE_CRL)
4327443275
WOLFSSL_X509V3_CTX ctx;
4327543276
WOLFSSL_X509* issuer = NULL;
4327643277
WOLFSSL_X509* subject = NULL;
@@ -56701,7 +56702,7 @@ static void updateCrlCb(CrlInfo* old, CrlInfo* cnew)
5670156702

5670256703
AssertTrue((f = XFOPEN(crl1, "rb")) != XBADFILE);
5670356704
AssertTrue(XFSEEK(f, 0, XSEEK_END) == 0);
56704-
AssertIntGE(sz = (size_t) XFTELL(f), 1);
56705+
AssertIntGE(sz = (word32) XFTELL(f), 1);
5670556706
AssertTrue(XFSEEK(f, 0, XSEEK_SET) == 0);
5670656707
AssertTrue( \
5670756708
(crl1Buff = (byte*)XMALLOC(sz, NULL, DYNAMIC_TYPE_FILE)) != NULL);
@@ -56711,7 +56712,7 @@ static void updateCrlCb(CrlInfo* old, CrlInfo* cnew)
5671156712

5671256713
AssertTrue((f = XFOPEN(crlRevoked, "rb")) != XBADFILE);
5671356714
AssertTrue(XFSEEK(f, 0, XSEEK_END) == 0);
56714-
AssertIntGE(sz = (size_t) XFTELL(f), 1);
56715+
AssertIntGE(sz = (word32) XFTELL(f), 1);
5671556716
AssertTrue(XFSEEK(f, 0, XSEEK_SET) == 0);
5671656717
AssertTrue( \
5671756718
(crlRevBuff = (byte*)XMALLOC(sz, NULL, DYNAMIC_TYPE_FILE)) != NULL);
@@ -56732,7 +56733,8 @@ static void updateCrlCb(CrlInfo* old, CrlInfo* cnew)
5673256733
AssertIntEQ(crl1Info.lastDateFormat, old->lastDateFormat);
5673356734
AssertIntEQ(crl1Info.nextDateMaxLen, old->nextDateMaxLen);
5673456735
AssertIntEQ(crl1Info.nextDateFormat, old->nextDateFormat);
56735-
AssertIntEQ(crl1Info.crlNumber, old->crlNumber);
56736+
AssertIntEQ(XMEMCMP(
56737+
crl1Info.crlNumber, old->crlNumber, CRL_MAX_NUM_SZ), 0);
5673656738
AssertIntEQ(XMEMCMP(
5673756739
crl1Info.issuerHash, old->issuerHash, old->issuerHashLen), 0);
5673856740
AssertIntEQ(XMEMCMP(
@@ -56746,7 +56748,8 @@ static void updateCrlCb(CrlInfo* old, CrlInfo* cnew)
5674656748
AssertIntEQ(crlRevInfo.lastDateFormat, cnew->lastDateFormat);
5674756749
AssertIntEQ(crlRevInfo.nextDateMaxLen, cnew->nextDateMaxLen);
5674856750
AssertIntEQ(crlRevInfo.nextDateFormat, cnew->nextDateFormat);
56749-
AssertIntEQ(crlRevInfo.crlNumber, cnew->crlNumber);
56751+
AssertIntEQ(XMEMCMP(
56752+
crlRevInfo.crlNumber, cnew->crlNumber, CRL_MAX_NUM_SZ), 0);
5675056753
AssertIntEQ(XMEMCMP(
5675156754
crlRevInfo.issuerHash, cnew->issuerHash, cnew->issuerHashLen), 0);
5675256755
AssertIntEQ(XMEMCMP(

0 commit comments

Comments
 (0)