Skip to content

Commit e2d3b63

Browse files
dgarskedgarske
authored
Merge pull request #10207 from ColtonWilley/d2i_pem_negative_length
Add signed-length validation to d2i, PEM, and buffer-load APIs
2 parents dbf72c4 + 3ffe25f commit e2d3b63

11 files changed

Lines changed: 123 additions & 14 deletions

File tree

‎src/crl.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -849,7 +849,7 @@ int BufferLoadCRL(WOLFSSL_CRL* crl, const byte* buff, long sz, int type,
849849

850850
WOLFSSL_ENTER("BufferLoadCRL");
851851

852-
if (crl == NULL || buff == NULL || sz == 0)
852+
if (crl == NULL || buff == NULL || sz <= 0)
853853
return BAD_FUNC_ARG;
854854

855855
if (type == WOLFSSL_FILETYPE_PEM) {
@@ -1175,7 +1175,7 @@ int GetCRLInfo(WOLFSSL_CRL* crl, CrlInfo* info, const byte* buff,
11751175

11761176
WOLFSSL_ENTER("GetCRLInfo");
11771177

1178-
if (crl == NULL || info == NULL || buff == NULL || sz == 0)
1178+
if (crl == NULL || info == NULL || buff == NULL || sz <= 0)
11791179
return BAD_FUNC_ARG;
11801180

11811181
if (type == WOLFSSL_FILETYPE_PEM) {

‎src/ocsp.c‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1188,6 +1188,10 @@ OcspResponse* wolfSSL_d2i_OCSP_RESPONSE(OcspResponse** response,
11881188

11891189
if (data == NULL || *data == NULL || len <= 0)
11901190
return NULL;
1191+
if (*data == NULL)
1192+
return NULL;
1193+
if (len <= 0)
1194+
return NULL;
11911195

11921196
if (response != NULL)
11931197
resp = *response;

‎src/pk_ec.c‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -449,6 +449,8 @@ static WOLFSSL_EC_GROUP* wolfssl_ec_group_d2i(WOLFSSL_EC_GROUP** group,
449449

450450
if (in_pp == NULL || *in_pp == NULL)
451451
return NULL;
452+
if (inSz <= 0)
453+
return NULL;
452454

453455
in = *in_pp;
454456

@@ -4998,7 +5000,10 @@ WOLFSSL_ECDSA_SIG* wolfSSL_d2i_ECDSA_SIG(WOLFSSL_ECDSA_SIG** sig,
49985000
WOLFSSL_ECDSA_SIG *s = NULL;
49995001

50005002
/* Validate parameter. */
5001-
if (pp == NULL) {
5003+
if (pp == NULL || *pp == NULL) {
5004+
err = 1;
5005+
}
5006+
if ((!err) && (len <= 0)) {
50025007
err = 1;
50035008
}
50045009
if (!err) {

‎src/pk_rsa.c‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -454,6 +454,10 @@ WOLFSSL_RSA *wolfSSL_d2i_RSAPublicKey(WOLFSSL_RSA **out,
454454
WOLFSSL_ERROR_MSG("Bad argument");
455455
err = 1;
456456
}
457+
if ((!err) && (derSz <= 0)) {
458+
WOLFSSL_ERROR_MSG("Bad argument");
459+
err = 1;
460+
}
457461
/* Create a new RSA key to return. */
458462
if ((!err) && ((rsa = wolfSSL_RSA_new()) == NULL)) {
459463
WOLFSSL_ERROR_MSG("RSA_new failed");
@@ -503,6 +507,10 @@ WOLFSSL_RSA *wolfSSL_d2i_RSAPrivateKey(WOLFSSL_RSA **out,
503507
WOLFSSL_ERROR_MSG("Bad argument");
504508
err = 1;
505509
}
510+
if ((!err) && (derSz <= 0)) {
511+
WOLFSSL_ERROR_MSG("Bad argument");
512+
err = 1;
513+
}
506514
/* Create a new RSA key to return. */
507515
if ((!err) && ((rsa = wolfSSL_RSA_new()) == NULL)) {
508516
WOLFSSL_ERROR_MSG("RSA_new failed");

‎src/ssl_asn1.c‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -913,7 +913,7 @@ WOLFSSL_ASN1_BIT_STRING* wolfSSL_d2i_ASN1_BIT_STRING(
913913

914914
WOLFSSL_ENTER("wolfSSL_d2i_ASN1_BIT_STRING");
915915

916-
if (src == NULL || *src == NULL || len == 0)
916+
if (src == NULL || *src == NULL || len <= 0)
917917
return NULL;
918918

919919
if (GetASNTag(*src, &idx, &tag, (word32)len) < 0)
@@ -2984,7 +2984,7 @@ static WOLFSSL_ASN1_STRING* d2i_ASN1_STRING(WOLFSSL_ASN1_STRING** out,
29842984

29852985
WOLFSSL_ENTER("d2i_ASN1_STRING");
29862986

2987-
if (src == NULL || *src == NULL || len == 0)
2987+
if (src == NULL || *src == NULL || len <= 0)
29882988
return NULL;
29892989

29902990
if (GetASNTag(*src, &idx, &tag, (word32)len) < 0)
@@ -3159,16 +3159,21 @@ int wolfSSL_ASN1_STRING_set(WOLFSSL_ASN1_STRING* asn1, const void* data, int sz)
31593159
}
31603160

31613161
if (ret == 1) {
3162+
/* Cast to size_t BEFORE adding 1 to prevent signed overflow
3163+
* when sz == INT_MAX. By this point sz >= 0 (negative sz is
3164+
* handled above as OpenSSL -1/strlen compat). */
3165+
size_t allocSz = (size_t)sz + 1;
3166+
31623167
/* Dispose of any existing dynamic data. */
31633168
if (asn1->isDynamic) {
31643169
XFREE(asn1->data, NULL, DYNAMIC_TYPE_OPENSSL);
31653170
asn1->data = NULL;
31663171
}
31673172

31683173
/* Check string will fit - including NUL. */
3169-
if (sz + 1 > CTC_NAME_SIZE) {
3174+
if (allocSz > CTC_NAME_SIZE) {
31703175
/* Allocate new buffer. */
3171-
asn1->data = (char*)XMALLOC((size_t)(sz + 1), NULL,
3176+
asn1->data = (char*)XMALLOC(allocSz, NULL,
31723177
DYNAMIC_TYPE_OPENSSL);
31733178
if (asn1->data == NULL) {
31743179
ret = 0;

‎src/ssl_load.c‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2423,6 +2423,10 @@ int ProcessBuffer(WOLFSSL_CTX* ctx, const unsigned char* buff, long sz,
24232423
if ((ret == 0) && (type == CHAIN_CERT_TYPE)) {
24242424
ret = BAD_FUNC_ARG;
24252425
}
2426+
/* Reject negative size - would wrap to huge word32. */
2427+
if ((ret == 0) && (sz < 0)) {
2428+
ret = BAD_FUNC_ARG;
2429+
}
24262430

24272431
#ifdef WOLFSSL_SMALL_STACK
24282432
if (ret == 0) {

‎src/x509.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4213,7 +4213,7 @@ static WOLFSSL_X509* d2i_X509orX509REQ(WOLFSSL_X509** x509,
42134213

42144214
WOLFSSL_ENTER("wolfSSL_X509_d2i");
42154215

4216-
if (in != NULL && len != 0
4216+
if (in != NULL && len > 0
42174217
#ifndef WOLFSSL_CERT_REQ
42184218
&& req == 0
42194219
#else
@@ -11373,7 +11373,7 @@ WOLFSSL_X509_ALGOR* wolfSSL_d2i_X509_ALGOR(WOLFSSL_X509_ALGOR** out,
1137311373

1137411374
WOLFSSL_ENTER("wolfSSL_d2i_X509_ALGOR");
1137511375

11376-
if (src == NULL || *src == NULL || len == 0)
11376+
if (src == NULL || *src == NULL || len <= 0)
1137711377
return NULL;
1137811378

1137911379
if (GetAlgoId(*src, &idx, &oid, oidIgnoreType, (word32)len) != 0)

‎tests/api.c‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2520,6 +2520,28 @@ static int test_wolfSSL_CTX_use_certificate_buffer(void)
25202520

25212521
} /* END test_wolfSSL_CTX_use_certificate_buffer */
25222522

2523+
static int test_ProcessBuffer_negative_size(void)
2524+
{
2525+
EXPECT_DECLS;
2526+
#if !defined(NO_CERTS) && !defined(NO_TLS) && !defined(NO_WOLFSSL_SERVER) && \
2527+
defined(USE_CERT_BUFFERS_2048) && !defined(NO_RSA)
2528+
WOLFSSL_CTX* ctx = NULL;
2529+
2530+
ExpectNotNull(ctx = wolfSSL_CTX_new(wolfSSLv23_server_method()));
2531+
2532+
ExpectIntEQ(wolfSSL_CTX_use_certificate_buffer(ctx,
2533+
server_cert_der_2048, -1, WOLFSSL_FILETYPE_ASN1),
2534+
WC_NO_ERR_TRACE(BAD_FUNC_ARG));
2535+
2536+
ExpectIntEQ(wolfSSL_CTX_use_certificate_buffer(ctx,
2537+
server_cert_der_2048, sizeof_server_cert_der_2048,
2538+
WOLFSSL_FILETYPE_ASN1), WOLFSSL_SUCCESS);
2539+
2540+
wolfSSL_CTX_free(ctx);
2541+
#endif
2542+
return EXPECT_RESULT();
2543+
}
2544+
25232545
static int test_wolfSSL_use_certificate_buffer(void)
25242546
{
25252547
EXPECT_DECLS;
@@ -10955,6 +10977,12 @@ static int test_wc_PemToDer(void)
1095510977

1095610978
XMEMSET(&info, 0, sizeof(info));
1095710979

10980+
{
10981+
const byte dummy = 'X';
10982+
ExpectIntEQ(wc_PemToDer(&dummy, -1, CERT_TYPE, &pDer, NULL,
10983+
&info, &eccKey), WC_NO_ERR_TRACE(BAD_FUNC_ARG));
10984+
}
10985+
1095810986
ExpectIntEQ(ret = load_file(ca_cert, &cert_buf, &cert_sz), 0);
1095910987
ExpectIntEQ(ret = wc_PemToDer(cert_buf, (long int)cert_sz, CERT_TYPE, &pDer, NULL,
1096010988
&info, &eccKey), 0);
@@ -11128,6 +11156,10 @@ static int test_wc_KeyPemToDer(void)
1112811156
ExpectIntEQ(wc_KeyPemToDer(cert_buf, 0, (byte*)&cert_der, cert_sz, ""),
1112911157
WC_NO_ERR_TRACE(BAD_FUNC_ARG));
1113011158

11159+
/* Bad arg: NULL der buffer with negative pemSz (NULL-deref guard). */
11160+
ExpectIntEQ(wc_KeyPemToDer(cert_buf, -1, NULL, 0, ""),
11161+
WC_NO_ERR_TRACE(BAD_FUNC_ARG));
11162+
1113111163
/* Test normal operation */
1113211164
cert_dersz = cert_sz; /* DER will be smaller than PEM */
1113311165
ExpectNotNull(cert_der = (byte*)malloc((size_t)cert_dersz));
@@ -21883,6 +21915,13 @@ static int test_wc_SetIssueBuffer(void)
2188321915

2188421916
ExpectIntEQ(0, wc_SetIssuerBuffer(&forgedCert, peerCertBuf, peerCertSz));
2188521917

21918+
/* Negative-size rejection: pin both wc_SetIssuerBuffer and
21919+
* wc_SetSubjectBuffer (representatives for the seven wc_Set* siblings). */
21920+
ExpectIntEQ(wc_SetIssuerBuffer(&forgedCert, peerCertBuf, -1),
21921+
WC_NO_ERR_TRACE(BAD_FUNC_ARG));
21922+
ExpectIntEQ(wc_SetSubjectBuffer(&forgedCert, peerCertBuf, -1),
21923+
WC_NO_ERR_TRACE(BAD_FUNC_ARG));
21924+
2188621925
wolfSSL_FreeX509(x509);
2188721926
#endif
2188821927
return EXPECT_RESULT();
@@ -26055,6 +26094,9 @@ static int test_wolfSSL_CTX_LoadCRL_largeCRLnum(void)
2605526094
WOLFSSL_SUCCESS);
2605626095
AssertIntEQ(XMEMCMP(
2605726096
crlInfo.crlNumber, exp_crlnum, XSTRLEN(exp_crlnum)), 0);
26097+
ExpectIntEQ(wolfSSL_CertManagerGetCRLInfo(
26098+
cm, &crlInfo, crlLrgCrlNumBuff, -1, WOLFSSL_FILETYPE_PEM),
26099+
WC_NO_ERR_TRACE(BAD_FUNC_ARG));
2605826100
/* Expect to fail loading CRL because of >21 octets CRL number */
2605926101
ExpectIntEQ(wolfSSL_CertManagerLoadCRLFile(cm, crl_lrgcrlnum2,
2606026102
WOLFSSL_FILETYPE_PEM),
@@ -34975,6 +35017,7 @@ TEST_CASE testCases[] = {
3497535017
TEST_DECL(test_wolfSSL_CTX_use_certificate),
3497635018
TEST_DECL(test_wolfSSL_CTX_use_certificate_file),
3497735019
TEST_DECL(test_wolfSSL_CTX_use_certificate_buffer),
35020+
TEST_DECL(test_ProcessBuffer_negative_size),
3497835021
TEST_DECL(test_wolfSSL_use_certificate_buffer),
3497935022
TEST_DECL(test_wolfSSL_CTX_use_PrivateKey_file),
3498035023
TEST_DECL(test_wolfSSL_CTX_use_RSAPrivateKey_file),

‎tests/api/test_ossl_ec.c‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1555,6 +1555,11 @@ int test_wolfSSL_ECDSA_SIG(void)
15551555
sig = NULL;
15561556

15571557
ExpectNull(wolfSSL_d2i_ECDSA_SIG(NULL, NULL, sizeof(sigData)));
1558+
/* Reject non-positive length and *pp == NULL (PR #10207). */
1559+
cp = sigData;
1560+
ExpectNull(wolfSSL_d2i_ECDSA_SIG(NULL, &cp, -1));
1561+
cp = NULL;
1562+
ExpectNull(wolfSSL_d2i_ECDSA_SIG(NULL, &cp, sizeof(sigData)));
15581563
cp = sigDataBad;
15591564
ExpectNull(wolfSSL_d2i_ECDSA_SIG(NULL, &cp, sizeof(sigDataBad)));
15601565
cp = sigData;

‎wolfcrypt/src/asn.c‎

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24955,10 +24955,10 @@ int PemToDer(const unsigned char* buff, long longSz, int type,
2495524955
const char* headerEnd = NULL;
2495624956
const char* footerEnd = NULL;
2495724957
const char* consumedEnd = NULL;
24958-
const char* bufferEnd = (const char*)(buff + longSz);
24958+
const char* bufferEnd = NULL;
2495924959
long neededSz;
2496024960
int ret = 0;
24961-
word32 sz = (word32)longSz;
24961+
word32 sz = 0;
2496224962
int encrypted_key = 0;
2496324963
DerBuffer* der;
2496424964
word32 algId = 0;
@@ -24979,6 +24979,14 @@ int PemToDer(const unsigned char* buff, long longSz, int type,
2497924979

2498024980
WOLFSSL_ENTER("PemToDer");
2498124981

24982+
/* Reject negative size - would wrap word32 and corrupt pointer arithmetic. */
24983+
if (longSz < 0) {
24984+
return BAD_FUNC_ARG;
24985+
}
24986+
24987+
bufferEnd = (const char*)(buff + longSz);
24988+
sz = (word32)longSz;
24989+
2498224990
/* get PEM header and footer based on type */
2498324991
ret = wc_PemGetHeaderFooter(type, &header, &footer);
2498424992
if (ret != 0)
@@ -30242,6 +30250,9 @@ int wc_SetAuthKeyIdFromCert(Cert *cert, const byte *der, int derSz)
3024230250
if (cert == NULL) {
3024330251
ret = BAD_FUNC_ARG;
3024430252
}
30253+
else if (derSz < 0) {
30254+
ret = BAD_FUNC_ARG;
30255+
}
3024530256
else {
3024630257
/* Check if decodedCert is cached */
3024730258
if (cert->der != der) {
@@ -30746,6 +30757,9 @@ int wc_SetIssuerBuffer(Cert* cert, const byte* der, int derSz)
3074630757
if (cert == NULL) {
3074730758
ret = BAD_FUNC_ARG;
3074830759
}
30760+
else if (derSz < 0) {
30761+
ret = BAD_FUNC_ARG;
30762+
}
3074930763
else {
3075030764
cert->selfSigned = 0;
3075130765

@@ -30775,6 +30789,9 @@ int wc_SetSubjectBuffer(Cert* cert, const byte* der, int derSz)
3077530789
if (cert == NULL) {
3077630790
ret = BAD_FUNC_ARG;
3077730791
}
30792+
else if (derSz < 0) {
30793+
ret = BAD_FUNC_ARG;
30794+
}
3077830795
else {
3077930796
/* Check if decodedCert is cached */
3078030797
if (cert->der != der) {
@@ -30802,6 +30819,9 @@ int wc_SetSubjectRaw(Cert* cert, const byte* der, int derSz)
3080230819
if (cert == NULL) {
3080330820
ret = BAD_FUNC_ARG;
3080430821
}
30822+
else if (derSz < 0) {
30823+
ret = BAD_FUNC_ARG;
30824+
}
3080530825
else {
3080630826
/* Check if decodedCert is cached */
3080730827
if (cert->der != der) {
@@ -30836,6 +30856,9 @@ int wc_SetIssuerRaw(Cert* cert, const byte* der, int derSz)
3083630856
if (cert == NULL) {
3083730857
ret = BAD_FUNC_ARG;
3083830858
}
30859+
else if (derSz < 0) {
30860+
ret = BAD_FUNC_ARG;
30861+
}
3083930862
else {
3084030863
/* Check if decodedCert is cached */
3084130864
if (cert->der != der) {
@@ -30873,6 +30896,9 @@ int wc_SetAltNamesBuffer(Cert* cert, const byte* der, int derSz)
3087330896
if (cert == NULL) {
3087430897
ret = BAD_FUNC_ARG;
3087530898
}
30899+
else if (derSz < 0) {
30900+
ret = BAD_FUNC_ARG;
30901+
}
3087630902
else {
3087730903
/* Check if decodedCert is cached */
3087830904
if (cert->der != der) {
@@ -30900,6 +30926,9 @@ int wc_SetDatesBuffer(Cert* cert, const byte* der, int derSz)
3090030926
if (cert == NULL) {
3090130927
ret = BAD_FUNC_ARG;
3090230928
}
30929+
else if (derSz < 0) {
30930+
ret = BAD_FUNC_ARG;
30931+
}
3090330932
else {
3090430933
/* Check if decodedCert is cached */
3090530934
if (cert->der != der) {

0 commit comments

Comments
 (0)