Skip to content

Commit

Permalink
Coverity fixes
Browse files Browse the repository at this point in the history
pk.c:
	EncryptDerKey - setting wrong ret value on allocation failure.
	wolfssl_rsa_generate_key_native - now checks e is a valid long
before passing in.
	Fix formatting.

ssl_load.c:
	ProcessBufferPrivPkcs8Dec - now checking password is not NULL
before zeroizing. Allocation may fail and ForceZero doesn't check for
NULL.
	Fix formatting.

tests/api.c:
	test_RsaSigFailure_cm - Check cert_sz is greater than zero
before use.
	send_new_session_ticket - assert that building the message
doesn't return error or 0.
	test_ticket_nonce_malloc - fix setting of medium and big to use
preprocessor. Fix big to be medium + 20.

asn.c:
	GetLength_ex - Fix type of bytes so that it can go negative.

sp_int.h:
	sp_clamp - add one to ii while it is a signed.
	Fix formatting.
  • Loading branch information
SparkiDev committed Jul 10, 2024
1 parent 00e4215 commit fea7a89
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 20 deletions.
12 changes: 8 additions & 4 deletions src/pk.c
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ int EncryptDerKey(byte *der, int *derSz, const EVP_CIPHER* cipher,
DYNAMIC_TYPE_ENCRYPTEDINFO);
if (info == NULL) {
WOLFSSL_MSG("malloc failed");
ret = 0;
ret = MEMORY_E;
}
}
#endif
Expand Down Expand Up @@ -417,7 +417,8 @@ int EncryptDerKey(byte *der, int *derSz, const EVP_CIPHER* cipher,
(*derSz) += (int)paddingSz;

/* Encrypt DER buffer. */
ret = wc_BufferKeyEncrypt(info, der, (word32)*derSz, passwd, passwdSz, WC_MD5);
ret = wc_BufferKeyEncrypt(info, der, (word32)*derSz, passwd, passwdSz,
WC_MD5);
if (ret != 0) {
WOLFSSL_MSG("encrypt key failed");
}
Expand Down Expand Up @@ -3273,6 +3274,7 @@ static int wolfssl_rsa_generate_key_native(WOLFSSL_RSA* rsa, int bits,
#endif
int initTmpRng = 0;
WC_RNG* rng = NULL;
long en;
#endif

(void)cb;
Expand All @@ -3286,10 +3288,12 @@ static int wolfssl_rsa_generate_key_native(WOLFSSL_RSA* rsa, int bits,
/* Something went wrong so return memory error. */
ret = MEMORY_E;
}
if ((ret == 0) && ((en = (long)wolfSSL_BN_get_word(e)) <= 0)) {
ret = BAD_FUNC_ARG;
}
if (ret == 0) {
/* Generate an RSA key. */
ret = wc_MakeRsaKey((RsaKey*)rsa->internal, bits,
(long)wolfSSL_BN_get_word(e), rng);
ret = wc_MakeRsaKey((RsaKey*)rsa->internal, bits, en, rng);
if (ret != MP_OKAY) {
WOLFSSL_ERROR_MSG("wc_MakeRsaKey failed");
}
Expand Down
11 changes: 8 additions & 3 deletions src/ssl_load.c
Original file line number Diff line number Diff line change
Expand Up @@ -1227,8 +1227,13 @@ static int ProcessBufferPrivPkcs8Dec(EncryptedInfo* info, DerBuffer* der,
der->length = (word32)ret;
}

/* Ensure password is zeroized. */
ForceZero(password, (word32)passwordSz);
#ifdef WOLFSSL_SMALL_STACK
if (password != NULL)
#endif
{
/* Ensure password is zeroized. */
ForceZero(password, (word32)passwordSz);
}
#ifdef WOLFSSL_SMALL_STACK
/* Dispose of password memory. */
XFREE(password, heap, DYNAMIC_TYPE_STRING);
Expand Down Expand Up @@ -5563,7 +5568,7 @@ long wolfSSL_CTX_set_tmp_dh(WOLFSSL_CTX* ctx, WOLFSSL_DH* dh)
ret = wolfssl_ctx_set_tmp_dh(ctx, p, pSz, g, gSz);
}

if (ret != 1 && ctx != NULL) {
if ((ret != 1) && (ctx != NULL)) {
/* Free the allocated buffers if not assigned into SSL. */
XFREE(p, ctx->heap, DYNAMIC_TYPE_PUBLIC_KEY);
XFREE(g, ctx->heap, DYNAMIC_TYPE_PUBLIC_KEY);
Expand Down
15 changes: 12 additions & 3 deletions tests/api.c
Original file line number Diff line number Diff line change
Expand Up @@ -65624,7 +65624,7 @@ static int test_RsaSigFailure_cm(void)
size_t cert_sz = 0;

ExpectIntEQ(load_file(server_cert, &cert_buf, &cert_sz), 0);
if (cert_buf != NULL) {
if ((cert_buf != NULL) && (cert_sz > 0)) {
/* corrupt DER - invert last byte, which is signature */
cert_buf[cert_sz-1] = ~cert_buf[cert_sz-1];
/* test bad cert */
Expand Down Expand Up @@ -77618,6 +77618,7 @@ static int send_new_session_ticket(WOLFSSL *ssl, byte nonceLength, byte filler)

sz = BuildTls13Message(ssl, buf, 2048, buf+5, idx - 5,
handshake, 0, 0, 0);
AssertIntGT(sz, 0);
test_ctx = (struct test_memio_ctx*)wolfSSL_GetIOWriteCtx(ssl);
AssertNotNull(test_ctx);
ret = test_memio_write_cb(ssl, (char*)buf, sz, test_ctx);
Expand Down Expand Up @@ -77716,8 +77717,16 @@ static int test_ticket_nonce_malloc(void)
}

small = TLS13_TICKET_NONCE_STATIC_SZ;
medium = small + 20 <= 255 ? small + 20 : 255;
big = medium + 20 <= 255 ? small + 20 : 255;
#if TLS13_TICKET_NONCE_STATIC_SZ + 20 <= 255
medium = small + 20;
#else
medium = 255;
#endif
#if TLS13_TICKET_NONCE_STATIC_SZ + 20 + 20 <= 255
big = small + 20;
#else
big = 255;
#endif

ExpectIntEQ(test_ticket_nonce_malloc_do(ssl_s, ssl_c, small), TEST_SUCCESS);
ExpectPtrEq(ssl_c->session->ticketNonce.data,
Expand Down
7 changes: 4 additions & 3 deletions wolfcrypt/src/asn.c
Original file line number Diff line number Diff line change
Expand Up @@ -2285,7 +2285,7 @@ int GetLength_ex(const byte* input, word32* inOutIdx, int* len, word32 maxIdx,
/* Bottom 7 bits are the number of bytes to calculate length with.
* Note: 0 indicates indefinite length encoding *not* 0 bytes of length.
*/
word32 bytes = (word32)b & 0x7FU;
int bytes = (int)(b & 0x7F);
int minLen;

/* Calculate minimum length to be encoded with bytes. */
Expand All @@ -2297,10 +2297,11 @@ int GetLength_ex(const byte* input, word32* inOutIdx, int* len, word32 maxIdx,
minLen = 0x80;
}
/* Only support up to the number of bytes that fit into return var. */
else if (bytes > sizeof(length)) {
else if (bytes > (int)sizeof(length)) {
WOLFSSL_MSG("GetLength - overlong data length spec");
return ASN_PARSE_E;
} else {
}
else {
minLen = 1 << ((bytes - 1) * 8);
}

Expand Down
14 changes: 7 additions & 7 deletions wolfssl/wolfcrypt/sp_int.h
Original file line number Diff line number Diff line change
Expand Up @@ -692,14 +692,14 @@ typedef struct sp_ecc_ctx {
*
* @param [in] a SP integer to update.
*/
#define sp_clamp(a) \
do { \
int ii; \
if ((a)->used > 0) { \
#define sp_clamp(a) \
do { \
int ii; \
if ((a)->used > 0) { \
for (ii = (int)(a)->used - 1; ii >= 0 && (a)->dp[ii] == 0; ii--) { \
} \
(a)->used = (unsigned int)ii + 1; \
} \
} \
(a)->used = (unsigned int)(ii + 1); \
} \
} while (0)

/* Check the compiled and linked math implementation are the same.
Expand Down

0 comments on commit fea7a89

Please sign in to comment.