Skip to content

Commit 02a4969

Browse files
lealem47lealem47
committed
Fix bug in ParseCRL_Extensions
1 parent 7898823 commit 02a4969

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
@@ -6414,8 +6414,7 @@ static int X509PrintSerial_ex(WOLFSSL_BIO* bio, byte* serial, int sz,
64146414
scratch + scratchLen, scratchSz - scratchLen,
64156415
"%02x%s", serial[i], (i < sz - 1) ?
64166416
(delimiter ? ":" : "") : "\n"))
6417-
>= scratchSz - scratchLen)
6418-
{
6417+
>= scratchSz - scratchLen) {
64196418
WOLFSSL_MSG("buffer overrun");
64206419
return WOLFSSL_FAILURE;
64216420
}
@@ -6428,10 +6427,8 @@ static int X509PrintSerial_ex(WOLFSSL_BIO* bio, byte* serial, int sz,
64286427

64296428
/* if serial can fit into byte then print on the same line */
64306429
else {
6431-
if ((scratchLen = XSNPRINTF(
6432-
scratch, MAX_WIDTH, " %d (0x%x)\n", serial[0], serial[0]))
6433-
>= MAX_WIDTH)
6434-
{
6430+
if ((scratchLen = XSNPRINTF(scratch, MAX_WIDTH, " %d (0x%x)\n",
6431+
(char)serial[0], serial[0])) >= MAX_WIDTH) {
64356432
WOLFSSL_MSG("buffer overrun");
64366433
return WOLFSSL_FAILURE;
64376434
}
@@ -8870,85 +8867,135 @@ static int X509CRLPrintExtensions(WOLFSSL_BIO* bio, WOLFSSL_X509_CRL* crl,
88708867
int indent)
88718868
{
88728869
char tmp[MAX_WIDTH]; /* buffer for XSNPRINTF */
8870+
int ret = 0;
88738871

88748872
if (XSNPRINTF(tmp, MAX_WIDTH, "%*s%s\n", indent, "",
88758873
"CRL extensions:") >= MAX_WIDTH) {
8876-
return WOLFSSL_FAILURE;
8874+
ret = WOLFSSL_FAILURE;
88778875
}
88788876

8879-
if (wolfSSL_BIO_write(bio, tmp, (int)XSTRLEN(tmp)) <= 0) {
8880-
return WOLFSSL_FAILURE;
8877+
if (ret == 0 && wolfSSL_BIO_write(bio, tmp, (int)XSTRLEN(tmp)) <= 0) {
8878+
ret = WOLFSSL_FAILURE;
88818879
}
88828880

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

8889-
if (wolfSSL_BIO_write(bio, tmp, (int)XSTRLEN(tmp)) <= 0) {
8890-
return WOLFSSL_FAILURE;
8917+
if (ret == 0 && wolfSSL_BIO_write(bio, tmp, (int)XSTRLEN(tmp)) <= 0) {
8918+
ret = WOLFSSL_FAILURE;
88918919
}
88928920

8893-
if (XSNPRINTF(tmp, MAX_WIDTH, "%*s%d\n", indent + 8, "",
8894-
crl->crlList->crlNumber) >= MAX_WIDTH)
8895-
{
8896-
return WOLFSSL_FAILURE;
8921+
if (ret == 0 && XSNPRINTF(tmp, MAX_WIDTH, "%*s%s\n", indent + 8, "",
8922+
dec_string) >= MAX_WIDTH) {
8923+
ret = WOLFSSL_FAILURE;
88978924
}
8898-
if (wolfSSL_BIO_write(bio, tmp, (int)XSTRLEN(tmp)) <= 0) {
8899-
return WOLFSSL_FAILURE;
8925+
8926+
if (ret == 0 && wolfSSL_BIO_write(bio, tmp, (int)XSTRLEN(tmp)) <= 0) {
8927+
ret = WOLFSSL_FAILURE;
89008928
}
8929+
89018930
XMEMSET(tmp, 0, sizeof(tmp));
8931+
8932+
if (freeMp) {
8933+
mp_free(dec_num);
8934+
}
8935+
8936+
#ifdef WOLFSSL_SMALL_STACK
8937+
XFREE(dec_num, NULL, DYNAMIC_TYPE_BIGINT);
8938+
#endif
89028939
}
89038940

89048941
#if !defined(NO_SKID)
8905-
if (crl->crlList->extAuthKeyIdSet && crl->crlList->extAuthKeyId[0] != 0) {
8942+
if (ret == 0 && crl->crlList->extAuthKeyIdSet &&
8943+
crl->crlList->extAuthKeyId[0] != 0) {
89068944
word32 i;
89078945
char val[5];
89088946
int valSz = 5;
89098947

89108948
if (XSNPRINTF(tmp, MAX_WIDTH, "%*s%s", indent + 4, "",
89118949
"X509v3 Authority Key Identifier:") >= MAX_WIDTH) {
8912-
return WOLFSSL_FAILURE;
8950+
ret = WOLFSSL_FAILURE;
89138951
}
89148952

8915-
XSTRNCAT(tmp, "\n", MAX_WIDTH - XSTRLEN(tmp) - 1);
8953+
if (ret == 0) {
8954+
XSTRNCAT(tmp, "\n", MAX_WIDTH - XSTRLEN(tmp) - 1);
8955+
}
89168956

8917-
if (wolfSSL_BIO_write(bio, tmp, (int)XSTRLEN(tmp)) <= 0) {
8918-
return WOLFSSL_FAILURE;
8957+
if (ret == 0 && wolfSSL_BIO_write(bio, tmp, (int)XSTRLEN(tmp)) <= 0) {
8958+
ret = WOLFSSL_FAILURE;
89198959
}
89208960
XMEMSET(tmp, 0, MAX_WIDTH);
89218961

8922-
if (XSNPRINTF(tmp, MAX_WIDTH - 1, "%*s%s",
8962+
if (ret == 0 && XSNPRINTF(tmp, MAX_WIDTH - 1, "%*s%s",
89238963
indent + 8, "", "keyid") >= MAX_WIDTH) {
8924-
return WOLFSSL_FAILURE;
8964+
ret = WOLFSSL_FAILURE;
89258965
}
89268966

89278967

89288968
for (i = 0; i < XSTRLEN((char*)crl->crlList->extAuthKeyId); i++) {
89298969
/* check if buffer is almost full */
8930-
if (XSTRLEN(tmp) >= sizeof(tmp) - valSz) {
8970+
if (ret == 0 && XSTRLEN(tmp) >= sizeof(tmp) - valSz) {
89318971
if (wolfSSL_BIO_write(bio, tmp, (int)XSTRLEN(tmp)) <= 0) {
8932-
return WOLFSSL_FAILURE;
8972+
ret = WOLFSSL_FAILURE;
89338973
}
89348974
tmp[0] = '\0';
89358975
}
8936-
if (XSNPRINTF(val, (size_t)valSz, ":%02X",
8937-
crl->crlList->extAuthKeyId[i]) >= valSz)
8938-
{
8976+
if (ret == 0 && XSNPRINTF(val, (size_t)valSz, ":%02X",
8977+
crl->crlList->extAuthKeyId[i]) >= valSz) {
89398978
WOLFSSL_MSG("buffer overrun");
8940-
return WOLFSSL_FAILURE;
8979+
ret = WOLFSSL_FAILURE;
8980+
}
8981+
if (ret == 0) {
8982+
XSTRNCAT(tmp, val, valSz);
89418983
}
8942-
XSTRNCAT(tmp, val, valSz);
89438984
}
8944-
XSTRNCAT(tmp, "\n", XSTRLEN("\n") + 1);
8945-
if (wolfSSL_BIO_write(bio, tmp, (int)XSTRLEN(tmp)) <= 0) {
8946-
return WOLFSSL_FAILURE;
8985+
if (ret == 0) {
8986+
XSTRNCAT(tmp, "\n", XSTRLEN("\n") + 1);
8987+
}
8988+
if (ret == 0 && wolfSSL_BIO_write(bio, tmp, (int)XSTRLEN(tmp)) <= 0) {
8989+
ret = WOLFSSL_FAILURE;
89478990
}
89488991
}
89498992
#endif
89508993

8951-
return WOLFSSL_SUCCESS;
8994+
if (ret == 0) {
8995+
ret = WOLFSSL_SUCCESS;
8996+
}
8997+
8998+
return ret;
89528999
}
89539000

89549001
/* iterate through a CRL's Revoked Certs and print out in human
@@ -9180,7 +9227,7 @@ void wolfSSL_X509_CRL_free(WOLFSSL_X509_CRL *crl)
91809227
}
91819228
#endif /* HAVE_CRL && (OPENSSL_EXTRA || WOLFSSL_WPAS_SMALL) */
91829229

9183-
#ifdef OPENSSL_EXTRA
9230+
#if defined(HAVE_CRL) && defined(OPENSSL_EXTRA)
91849231
WOLFSSL_ASN1_TIME* wolfSSL_X509_CRL_get_lastUpdate(WOLFSSL_X509_CRL* crl)
91859232
{
91869233
if ((crl != NULL) && (crl->crlList != NULL) &&
@@ -9210,7 +9257,7 @@ int wolfSSL_X509_CRL_verify(WOLFSSL_X509_CRL* crl, WOLFSSL_EVP_PKEY* key)
92109257
return 0;
92119258
}
92129259
#endif
9213-
#endif /* OPENSSL_EXTRA */
9260+
#endif /* HAVE_CRL && OPENSSL_EXTRA */
92149261

92159262
#ifdef OPENSSL_EXTRA
92169263

‎tests/api.c‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42980,7 +42980,8 @@ static int test_wolfSSL_X509V3_set_ctx(void)
4298042980
{
4298142981
EXPECT_DECLS;
4298242982
#if (defined(OPENSSL_ALL) || defined(OPENSSL_EXTRA)) && \
42983-
defined(WOLFSSL_CERT_GEN) && defined(WOLFSSL_CERT_REQ)
42983+
defined(WOLFSSL_CERT_GEN) && defined(WOLFSSL_CERT_REQ) && \
42984+
defined(HAVE_CRL)
4298442985
WOLFSSL_X509V3_CTX ctx;
4298542986
WOLFSSL_X509* issuer = NULL;
4298642987
WOLFSSL_X509* subject = NULL;
@@ -56410,7 +56411,7 @@ static void updateCrlCb(CrlInfo* old, CrlInfo* cnew)
5641056411

5641156412
AssertTrue((f = XFOPEN(crl1, "rb")) != XBADFILE);
5641256413
AssertTrue(XFSEEK(f, 0, XSEEK_END) == 0);
56413-
AssertIntGE(sz = (size_t) XFTELL(f), 1);
56414+
AssertIntGE(sz = (word32) XFTELL(f), 1);
5641456415
AssertTrue(XFSEEK(f, 0, XSEEK_SET) == 0);
5641556416
AssertTrue( \
5641656417
(crl1Buff = (byte*)XMALLOC(sz, NULL, DYNAMIC_TYPE_FILE)) != NULL);
@@ -56420,7 +56421,7 @@ static void updateCrlCb(CrlInfo* old, CrlInfo* cnew)
5642056421

5642156422
AssertTrue((f = XFOPEN(crlRevoked, "rb")) != XBADFILE);
5642256423
AssertTrue(XFSEEK(f, 0, XSEEK_END) == 0);
56423-
AssertIntGE(sz = (size_t) XFTELL(f), 1);
56424+
AssertIntGE(sz = (word32) XFTELL(f), 1);
5642456425
AssertTrue(XFSEEK(f, 0, XSEEK_SET) == 0);
5642556426
AssertTrue( \
5642656427
(crlRevBuff = (byte*)XMALLOC(sz, NULL, DYNAMIC_TYPE_FILE)) != NULL);
@@ -56441,7 +56442,8 @@ static void updateCrlCb(CrlInfo* old, CrlInfo* cnew)
5644156442
AssertIntEQ(crl1Info.lastDateFormat, old->lastDateFormat);
5644256443
AssertIntEQ(crl1Info.nextDateMaxLen, old->nextDateMaxLen);
5644356444
AssertIntEQ(crl1Info.nextDateFormat, old->nextDateFormat);
56444-
AssertIntEQ(crl1Info.crlNumber, old->crlNumber);
56445+
AssertIntEQ(XMEMCMP(
56446+
crl1Info.crlNumber, old->crlNumber, CRL_MAX_NUM_SZ), 0);
5644556447
AssertIntEQ(XMEMCMP(
5644656448
crl1Info.issuerHash, old->issuerHash, old->issuerHashLen), 0);
5644756449
AssertIntEQ(XMEMCMP(
@@ -56455,7 +56457,8 @@ static void updateCrlCb(CrlInfo* old, CrlInfo* cnew)
5645556457
AssertIntEQ(crlRevInfo.lastDateFormat, cnew->lastDateFormat);
5645656458
AssertIntEQ(crlRevInfo.nextDateMaxLen, cnew->nextDateMaxLen);
5645756459
AssertIntEQ(crlRevInfo.nextDateFormat, cnew->nextDateFormat);
56458-
AssertIntEQ(crlRevInfo.crlNumber, cnew->crlNumber);
56460+
AssertIntEQ(XMEMCMP(
56461+
crlRevInfo.crlNumber, cnew->crlNumber, CRL_MAX_NUM_SZ), 0);
5645956462
AssertIntEQ(XMEMCMP(
5646056463
crlRevInfo.issuerHash, cnew->issuerHash, cnew->issuerHashLen), 0);
5646156464
AssertIntEQ(XMEMCMP(

0 commit comments

Comments
 (0)