From d7e11fb189d683624e76faceaf69f2b0281ea0c2 Mon Sep 17 00:00:00 2001 From: MatrixSSL Administrator Date: Thu, 13 Oct 2016 15:41:50 +0300 Subject: [PATCH] Fixed several coverity issues and a small issue in configuration files (accidental #can_define) statements. --- apps/ssl/client.c | 10 ++++++++++ configs/default/matrixsslConfig.h | 10 +++++----- configs/noecc/matrixsslConfig.h | 10 +++++----- configs/rsaonly/matrixsslConfig.h | 10 +++++----- configs/tls/matrixsslConfig.h | 10 +++++----- crypto/keyformat/crl.c | 2 +- crypto/keyformat/pkcs.c | 10 ---------- crypto/keyformat/x509.c | 2 +- crypto/test/algorithmTest.c | 7 ++++++- crypto/test/dhperf/dhperf.c | 9 +++++++-- matrixssl/matrixsslConfig.h | 10 +++++----- matrixssl/sslEncode.c | 3 ++- 12 files changed, 52 insertions(+), 41 deletions(-) diff --git a/apps/ssl/client.c b/apps/ssl/client.c index 016fa34..bf8b6ca 100644 --- a/apps/ssl/client.c +++ b/apps/ssl/client.c @@ -62,6 +62,7 @@ #define USE_HEADER_KEYS #define ALLOW_ANON_CONNECTIONS 1 +#define CRL_MAX_LENGTH 1048576 /* Maximum length for CRL: 1 megabyte. */ /* If the algorithm type is supported, load a CA for it */ #ifdef USE_HEADER_KEYS @@ -1826,6 +1827,15 @@ int32 fetchCRL(psPool_t *pool, char *url, uint32_t urlLen, psAssert((replyPtr+4) < (char*)&(crlChunk[HTTP_REPLY_CHUNK_SIZE])); replyPtr += 4; /* Move past that "\r\n\r\n" to get to start */ + /* Check buffer length appears acceptable */ + if (crlBinLen < 1 || crlBinLen > CRL_MAX_LENGTH) { + _psTrace("fetchCRL: Unacceptable size for CRL\n"); + /* Note: If this fails you may need to check CRL_MAX_LENGTH, + as you possibly need to allow larger CRL. */ + close(fd); + return -1; + } + /* Allocate the CRL buffer. Will be full size if sawContentLength */ if ((crlBin = psMalloc(pool, crlBinLen)) == NULL) { _psTrace("fetchCRL: Memory allocation error for CRL buffer\n"); diff --git a/configs/default/matrixsslConfig.h b/configs/default/matrixsslConfig.h index e923a91..fdfc689 100644 --- a/configs/default/matrixsslConfig.h +++ b/configs/default/matrixsslConfig.h @@ -309,8 +309,8 @@ extern "C" { SSL_DEFAULT_x_BUF_SIZE value in bytes, maximum SSL_MAX_BUF_SIZE */ #ifndef USE_DTLS -#can_define SSL_DEFAULT_IN_BUF_SIZE 1500 /* Base recv buf size, bytes */ -#can_define SSL_DEFAULT_OUT_BUF_SIZE 1500 /* Base send buf size, bytes */ +#define SSL_DEFAULT_IN_BUF_SIZE 1500 /* Base recv buf size, bytes */ +#define SSL_DEFAULT_OUT_BUF_SIZE 1500 /* Base send buf size, bytes */ #else /******************************************************************************/ /** @@ -320,9 +320,9 @@ extern "C" { limitation in MatrixDTLS so connections will not succeed if a peer has a PTMU set larger than this value. */ -#define DTLS_PMTU 1500/* 1500 Default/Maximum datagram len */ -#define SSL_DEFAULT_IN_BUF_SIZE DTLS_PMTU/* See PMTU comments above */ -#define SSL_DEFAULT_OUT_BUF_SIZE DTLS_PMTU/* See PMTU comments above */ +#define DTLS_PMTU 1500 /* 1500 Default/Maximum datagram len */ +#define SSL_DEFAULT_IN_BUF_SIZE DTLS_PMTU /* See PMTU comments above */ +#define SSL_DEFAULT_OUT_BUF_SIZE DTLS_PMTU /* See PMTU comments above */ //#define DTLS_SEND_RECORDS_INDIVIDUALLY /* Max one record per datagram */ #endif diff --git a/configs/noecc/matrixsslConfig.h b/configs/noecc/matrixsslConfig.h index fa9fb52..36ec978 100644 --- a/configs/noecc/matrixsslConfig.h +++ b/configs/noecc/matrixsslConfig.h @@ -309,8 +309,8 @@ extern "C" { SSL_DEFAULT_x_BUF_SIZE value in bytes, maximum SSL_MAX_BUF_SIZE */ #ifndef USE_DTLS -#can_define SSL_DEFAULT_IN_BUF_SIZE 1500 /* Base recv buf size, bytes */ -#can_define SSL_DEFAULT_OUT_BUF_SIZE 1500 /* Base send buf size, bytes */ +#define SSL_DEFAULT_IN_BUF_SIZE 1500 /* Base recv buf size, bytes */ +#define SSL_DEFAULT_OUT_BUF_SIZE 1500 /* Base send buf size, bytes */ #else /******************************************************************************/ /** @@ -320,9 +320,9 @@ extern "C" { limitation in MatrixDTLS so connections will not succeed if a peer has a PTMU set larger than this value. */ -#define DTLS_PMTU 1500/* 1500 Default/Maximum datagram len */ -#define SSL_DEFAULT_IN_BUF_SIZE DTLS_PMTU/* See PMTU comments above */ -#define SSL_DEFAULT_OUT_BUF_SIZE DTLS_PMTU/* See PMTU comments above */ +#define DTLS_PMTU 1500 /* 1500 Default/Maximum datagram len */ +#define SSL_DEFAULT_IN_BUF_SIZE DTLS_PMTU /* See PMTU comments above */ +#define SSL_DEFAULT_OUT_BUF_SIZE DTLS_PMTU /* See PMTU comments above */ //#define DTLS_SEND_RECORDS_INDIVIDUALLY /* Max one record per datagram */ #endif diff --git a/configs/rsaonly/matrixsslConfig.h b/configs/rsaonly/matrixsslConfig.h index d614833..e48a431 100644 --- a/configs/rsaonly/matrixsslConfig.h +++ b/configs/rsaonly/matrixsslConfig.h @@ -309,8 +309,8 @@ extern "C" { SSL_DEFAULT_x_BUF_SIZE value in bytes, maximum SSL_MAX_BUF_SIZE */ #ifndef USE_DTLS -#can_define SSL_DEFAULT_IN_BUF_SIZE 1500 /* Base recv buf size, bytes */ -#can_define SSL_DEFAULT_OUT_BUF_SIZE 1500 /* Base send buf size, bytes */ +#define SSL_DEFAULT_IN_BUF_SIZE 1500 /* Base recv buf size, bytes */ +#define SSL_DEFAULT_OUT_BUF_SIZE 1500 /* Base send buf size, bytes */ #else /******************************************************************************/ /** @@ -320,9 +320,9 @@ extern "C" { limitation in MatrixDTLS so connections will not succeed if a peer has a PTMU set larger than this value. */ -#define DTLS_PMTU 1500/* 1500 Default/Maximum datagram len */ -#define SSL_DEFAULT_IN_BUF_SIZE DTLS_PMTU/* See PMTU comments above */ -#define SSL_DEFAULT_OUT_BUF_SIZE DTLS_PMTU/* See PMTU comments above */ +#define DTLS_PMTU 1500 /* 1500 Default/Maximum datagram len */ +#define SSL_DEFAULT_IN_BUF_SIZE DTLS_PMTU /* See PMTU comments above */ +#define SSL_DEFAULT_OUT_BUF_SIZE DTLS_PMTU /* See PMTU comments above */ //#define DTLS_SEND_RECORDS_INDIVIDUALLY /* Max one record per datagram */ #endif diff --git a/configs/tls/matrixsslConfig.h b/configs/tls/matrixsslConfig.h index e923a91..fdfc689 100644 --- a/configs/tls/matrixsslConfig.h +++ b/configs/tls/matrixsslConfig.h @@ -309,8 +309,8 @@ extern "C" { SSL_DEFAULT_x_BUF_SIZE value in bytes, maximum SSL_MAX_BUF_SIZE */ #ifndef USE_DTLS -#can_define SSL_DEFAULT_IN_BUF_SIZE 1500 /* Base recv buf size, bytes */ -#can_define SSL_DEFAULT_OUT_BUF_SIZE 1500 /* Base send buf size, bytes */ +#define SSL_DEFAULT_IN_BUF_SIZE 1500 /* Base recv buf size, bytes */ +#define SSL_DEFAULT_OUT_BUF_SIZE 1500 /* Base send buf size, bytes */ #else /******************************************************************************/ /** @@ -320,9 +320,9 @@ extern "C" { limitation in MatrixDTLS so connections will not succeed if a peer has a PTMU set larger than this value. */ -#define DTLS_PMTU 1500/* 1500 Default/Maximum datagram len */ -#define SSL_DEFAULT_IN_BUF_SIZE DTLS_PMTU/* See PMTU comments above */ -#define SSL_DEFAULT_OUT_BUF_SIZE DTLS_PMTU/* See PMTU comments above */ +#define DTLS_PMTU 1500 /* 1500 Default/Maximum datagram len */ +#define SSL_DEFAULT_IN_BUF_SIZE DTLS_PMTU /* See PMTU comments above */ +#define SSL_DEFAULT_OUT_BUF_SIZE DTLS_PMTU /* See PMTU comments above */ //#define DTLS_SEND_RECORDS_INDIVIDUALLY /* Max one record per datagram */ #endif diff --git a/crypto/keyformat/crl.c b/crypto/keyformat/crl.c index cdbbf40..f6bb9b6 100644 --- a/crypto/keyformat/crl.c +++ b/crypto/keyformat/crl.c @@ -399,7 +399,7 @@ static int32_t nextUpdateTest(char *c, int32 timeType) if (sysTime.wDay > d) { return -1; } else if (sysTime.wDay == d) { - if (sysTime.wHour > h) + if (sysTime.wHour > h) { return -1; } } diff --git a/crypto/keyformat/pkcs.c b/crypto/keyformat/pkcs.c index 358c5c3..e53be48 100644 --- a/crypto/keyformat/pkcs.c +++ b/crypto/keyformat/pkcs.c @@ -1898,8 +1898,6 @@ static int32 pkcs_1_mgf1(psPool_t *pool, const unsigned char *seed, psSha512Update(&md.sha512, buf, 4); psSha512Final(&md.sha512, buf); #endif - } else { - return PS_UNSUPPORTED_FAIL; } /* store it */ @@ -2268,10 +2266,6 @@ int32 pkcs1OaepDecode(psPool_t *pool, const unsigned char *msg, uint32 msglen, psMd5Update(&md.md5, lparam, lparamlen); psMd5Final(&md.md5, seed); } - if (err < 0) { - psTraceCrypto("Hash error in OAEP decode\n"); - goto LBL_ERR; - } } else { /* can't pass hash routine a NULL so use DB with zero length */ if (hash_idx == PKCS1_SHA1_ID) { @@ -2283,10 +2277,6 @@ int32 pkcs1OaepDecode(psPool_t *pool, const unsigned char *msg, uint32 msglen, psMd5Update(&md.md5, DB, 0); psMd5Final(&md.md5, seed); } - if (err < 0) { - psTraceCrypto("Zero hash error in OAEP decode\n"); - goto LBL_ERR; - } } /* diff --git a/crypto/keyformat/x509.c b/crypto/keyformat/x509.c index f64e2fb..b457518 100644 --- a/crypto/keyformat/x509.c +++ b/crypto/keyformat/x509.c @@ -1334,7 +1334,7 @@ int32_t psX509GetConcatenatedDomainComponent(const x509DNattributes_t *DN, dc->len - DN_NUM_TERMINATING_NULLS); pos += dc->len - DN_NUM_TERMINATING_NULLS; if (i != 0) { - strncpy(*out_str + pos, ".", 1); + (*out_str)[pos] = '.'; pos++; } } diff --git a/crypto/test/algorithmTest.c b/crypto/test/algorithmTest.c index 8153e2a..dde1256 100644 --- a/crypto/test/algorithmTest.c +++ b/crypto/test/algorithmTest.c @@ -816,7 +816,7 @@ int32 psAesTestGCM(void) } memcpy(iv, tests[i].iv, 12); psAesInitGCM(&eCtx, tests[i].key, tests[i].keylen); - res = psAesReadyGCMRandomIV(&eCtx, iv, tests[i].aad, tests[i].aadlen, + res |= psAesReadyGCMRandomIV(&eCtx, iv, tests[i].aad, tests[i].aadlen, NULL); if (res != PS_SUCCESS) { memset(ciphertext_rand, 0, sizeof ciphertext_rand); @@ -3487,8 +3487,13 @@ static int test_hmac_vector(int32 size, din, din_len, md_res, key_out, &key_length); + } else { + _psTraceInt("FAILED: HMAC vector unsupported size: %d\n", + (int) size); + return PS_FAILURE; } + equals = (rv == PS_SUCCESS && memcmp(dout, md_res, size) == 0); if (equals != should_succeed) { diff --git a/crypto/test/dhperf/dhperf.c b/crypto/test/dhperf/dhperf.c index d85f9bb..e6bd2bc 100644 --- a/crypto/test/dhperf/dhperf.c +++ b/crypto/test/dhperf/dhperf.c @@ -173,6 +173,11 @@ int main(int argc, char **argv) #ifdef DO_GEN_SECRET psDhExportParameters(misc, &dhParams, &p, &pLen, &g, &gLen); + if (p == NULL || g == NULL) { + _psTrace(" DH parameters could not be used\n"); + fprintf(stderr, "FAIL: DH parameters did not work.\n"); + exit(1); + } if (psDhGenKeyInts(misc, dhParams.size, &dhParams.p, &dhParams.g, &dhKeyPriv, NULL) < 0) { _psTrace(" FAILED OPERATION\n"); @@ -198,8 +203,8 @@ int main(int argc, char **argv) _psTraceInt(TIME_UNITS " genSecret\n", psDiffMsecs(start, end, NULL)); #endif /* DO_GEN_SECRET */ - psFree(p, misc); - psFree(g, misc); + psFreeAndClear(&p, misc); + psFreeAndClear(&g, misc); pkcs3ClearDhParams(&dhParams); i++; } diff --git a/matrixssl/matrixsslConfig.h b/matrixssl/matrixsslConfig.h index e923a91..fdfc689 100644 --- a/matrixssl/matrixsslConfig.h +++ b/matrixssl/matrixsslConfig.h @@ -309,8 +309,8 @@ extern "C" { SSL_DEFAULT_x_BUF_SIZE value in bytes, maximum SSL_MAX_BUF_SIZE */ #ifndef USE_DTLS -#can_define SSL_DEFAULT_IN_BUF_SIZE 1500 /* Base recv buf size, bytes */ -#can_define SSL_DEFAULT_OUT_BUF_SIZE 1500 /* Base send buf size, bytes */ +#define SSL_DEFAULT_IN_BUF_SIZE 1500 /* Base recv buf size, bytes */ +#define SSL_DEFAULT_OUT_BUF_SIZE 1500 /* Base send buf size, bytes */ #else /******************************************************************************/ /** @@ -320,9 +320,9 @@ extern "C" { limitation in MatrixDTLS so connections will not succeed if a peer has a PTMU set larger than this value. */ -#define DTLS_PMTU 1500/* 1500 Default/Maximum datagram len */ -#define SSL_DEFAULT_IN_BUF_SIZE DTLS_PMTU/* See PMTU comments above */ -#define SSL_DEFAULT_OUT_BUF_SIZE DTLS_PMTU/* See PMTU comments above */ +#define DTLS_PMTU 1500 /* 1500 Default/Maximum datagram len */ +#define SSL_DEFAULT_IN_BUF_SIZE DTLS_PMTU /* See PMTU comments above */ +#define SSL_DEFAULT_OUT_BUF_SIZE DTLS_PMTU /* See PMTU comments above */ //#define DTLS_SEND_RECORDS_INDIVIDUALLY /* Max one record per datagram */ #endif diff --git a/matrixssl/sslEncode.c b/matrixssl/sslEncode.c index 30e57ea..6b698bb 100644 --- a/matrixssl/sslEncode.c +++ b/matrixssl/sslEncode.c @@ -4360,7 +4360,8 @@ static int32 writeCertificateStatus(ssl_t *ssl, sslBuf_t *out) } response; } CertificateStatus; */ *c = 0x1; c++; - *c = (unsigned char)((ocspLen & 0xFF0000) >> 16); c++; + /* ocspLen is 16 bit value. */ + *c = 0; c++; *c = (ocspLen & 0xFF00) >> 8; c++; *c = (ocspLen & 0xFF); c++; memcpy(c, ssl->keys->OCSPResponseBuf, ocspLen);