From 000fc64f272fb0fbfae37ea6daefda8658680a62 Mon Sep 17 00:00:00 2001 From: Sean Parkinson Date: Mon, 16 Sep 2019 10:21:08 +1000 Subject: [PATCH] Fixes from overnight build failures ssl.c: Certificate store fields freed without being NULLed and then freed again. integer.c: Compiler complained that a->dp may be NULL in mp_set_bit when setting bit. pkcs12.c: ret is zero after GetLength and this is an error but data is freed only when ret != 0. pkcs7.c: derArr was not zeroized for full allocated size. --- src/ssl.c | 31 ++++++++++--------------------- wolfcrypt/src/integer.c | 2 +- wolfcrypt/src/pkcs12.c | 4 ++++ wolfcrypt/src/pkcs7.c | 2 +- 4 files changed, 16 insertions(+), 23 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index e18ae778c..eb19fc82f 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -21277,22 +21277,22 @@ WOLFSSL_X509_STORE* wolfSSL_X509_STORE_new(void) { WOLFSSL_X509_STORE* store = NULL; - if((store = (WOLFSSL_X509_STORE*)XMALLOC(sizeof(WOLFSSL_X509_STORE), NULL, - DYNAMIC_TYPE_X509_STORE)) == NULL) + if ((store = (WOLFSSL_X509_STORE*)XMALLOC(sizeof(WOLFSSL_X509_STORE), NULL, + DYNAMIC_TYPE_X509_STORE)) == NULL) goto err_exit; XMEMSET(store, 0, sizeof(WOLFSSL_X509_STORE)); store->isDynamic = 1; - if((store->cm = wolfSSL_CertManagerNew()) == NULL) + if ((store->cm = wolfSSL_CertManagerNew()) == NULL) goto err_exit; #ifdef HAVE_CRL store->crl = NULL; - if((store->crl = (WOLFSSL_X509_CRL *)XMALLOC(sizeof(WOLFSSL_X509_CRL), - NULL, DYNAMIC_TYPE_TMP_BUFFER)) == NULL) + if ((store->crl = (WOLFSSL_X509_CRL *)XMALLOC(sizeof(WOLFSSL_X509_CRL), + NULL, DYNAMIC_TYPE_TMP_BUFFER)) == NULL) goto err_exit; - if(InitCRL(store->crl, NULL) < 0) + if (InitCRL(store->crl, NULL) < 0) goto err_exit; #endif @@ -21307,19 +21307,9 @@ WOLFSSL_X509_STORE* wolfSSL_X509_STORE_new(void) return store; err_exit: - if(store == NULL) + if (store == NULL) return NULL; - if(store->cm != NULL) - wolfSSL_CertManagerFree(store->cm); -#ifdef HAVE_CRL - if(store->crl != NULL) - wolfSSL_X509_CRL_free(store->crl); -#endif -#ifdef OPENSSL_EXTRA - if (store->param != NULL){ - XFREE(store->param,NULL,DYNAMIC_TYPE_OPENSSL); - } -#endif + wolfSSL_X509_STORE_free(store); return NULL; @@ -21336,9 +21326,8 @@ void wolfSSL_X509_STORE_free(WOLFSSL_X509_STORE* store) wolfSSL_X509_CRL_free(store->crl); #endif #ifdef OPENSSL_EXTRA - if (store->param != NULL){ - XFREE(store->param,NULL,DYNAMIC_TYPE_OPENSSL); - } + if (store->param != NULL) + XFREE(store->param, NULL, DYNAMIC_TYPE_OPENSSL); #endif XFREE(store, NULL, DYNAMIC_TYPE_X509_STORE); } diff --git a/wolfcrypt/src/integer.c b/wolfcrypt/src/integer.c index 1404e39ef..0bd1d87ee 100644 --- a/wolfcrypt/src/integer.c +++ b/wolfcrypt/src/integer.c @@ -2831,7 +2831,7 @@ int mp_set_bit (mp_int * a, int b) { int i = b / DIGIT_BIT, res; - if (a->used < (int)(i + 1)) { + if (a->dp == NULL || a->used < (int)(i + 1)) { /* grow a to accommodate the single bit */ if ((res = mp_grow (a, i + 1)) != MP_OKAY) { return res; diff --git a/wolfcrypt/src/pkcs12.c b/wolfcrypt/src/pkcs12.c index c221f7b22..80140da5a 100644 --- a/wolfcrypt/src/pkcs12.c +++ b/wolfcrypt/src/pkcs12.c @@ -1077,6 +1077,8 @@ int wc_PKCS12_parse(WC_PKCS12* pkcs12, const char* psw, ERROR_OUT(ASN_PARSE_E, exit_pk12par); } if ((ret = GetLength(data, &idx, &size, ci->dataSz)) <= 0) { + if (ret == 0) + ret = ASN_PARSE_E; goto exit_pk12par; } if (*pkey == NULL) { @@ -1196,6 +1198,8 @@ int wc_PKCS12_parse(WC_PKCS12* pkcs12, const char* psw, } if ((ret = GetLength(data, &idx, &size, ci->dataSz)) <= 0) { + if (ret == 0) + ret = ASN_PARSE_E; goto exit_pk12par; } if (data[idx++] != ASN_OCTET_STRING) { diff --git a/wolfcrypt/src/pkcs7.c b/wolfcrypt/src/pkcs7.c index c41d6b724..ad1a41b78 100644 --- a/wolfcrypt/src/pkcs7.c +++ b/wolfcrypt/src/pkcs7.c @@ -1509,7 +1509,7 @@ static int FlattenAttributes(PKCS7* pkcs7, byte* output, EncodedAttrib* ea, if (derArr == NULL) { return MEMORY_E; } - ForceZero(derArr, eaSz); + ForceZero(derArr, eaSz * sizeof(FlatAttrib*)); for (i = 0; i < eaSz; i++) { derArr[i] = (FlatAttrib*) XMALLOC(sizeof(FlatAttrib), pkcs7->heap,