Browse Source

cppcheck fixes and a config fix

./configure --disable-rsa --disable-ecc --disable-dsa
--enable-curve25519 --disable-ed25519 --disable-curve448
--disable-ed448 --enable-cryptonly

suites.c, testsuite.c: ensure port is an integer for snprintf.

unit.c: make memFailCount an integer for printf.

aes.c:
  Reduce variable scope.
  Check aes is not NULL before use in GHASH implementations.
XTS check sz is greater than or equal to a AES_BLOCK_SIZE rather than
0 as another block is processed.
  wc_AesXtsEncrypt, wc_AesXtsEncrypt - simplify braces and ifdefs
wc_AesEcbEncrypt - subtracting from sz is unnecessary as is unused
after.

asn.c:
StoreKey, StoreEccKey - compiler doesn't see ret != 0 when publicKey
is NULL.
  DecodeAuthInfo - count is not used when after break.
  DecodeSubtree - don't use min and max as variables (already macros).
SetEccPublicKey - initialize pubSz and set sz regardless for
compiler's sake.
wc_EncodeName_ex - use unique variable 'namesASN'; ret isn't set after
last check.
SetEccPublicKey - simplify code by using else rather than check ret
wasn't set.
  DecodeAsymKey - ret not modified in non-template implementaiton.
  SetAsymKeyDer - ret still at initialized value here.
DecodeResponseData - ensure dataASN is freed when single->next->status
failed to allocate.

test.c:
  curve255519_der_test() can't be compiled when NO_ASN is defined.

types.h:
  cast to the appropriate type in EXIT_TEST
test.h
don't return anything when THREAD_RETURN is void and EXIT_TEST is for
threading with stack size.
Sean Parkinson 2 years ago
parent
commit
142c7a9892
8 changed files with 105 additions and 74 deletions
  1. 1 1
      tests/suites.c
  2. 1 1
      tests/unit.c
  3. 1 1
      testsuite/testsuite.c
  4. 39 24
      wolfcrypt/src/aes.c
  5. 49 43
      wolfcrypt/src/asn.c
  6. 5 3
      wolfcrypt/test/test.c
  7. 8 0
      wolfssl/test.h
  8. 1 1
      wolfssl/wolfcrypt/types.h

+ 1 - 1
tests/suites.c

@@ -487,7 +487,7 @@ static int execute_test_case(int svr_argc, char** svr_argv,
         if (cliArgs.argc + 2 > MAX_ARGS)
             printf("cannot add the magic port number flag to client\n");
         else {
-            snprintf(portNumber, sizeof(portNumber), "%d", ready.port);
+            snprintf(portNumber, sizeof(portNumber), "%d", (int)ready.port);
             cli_argv[cliArgs.argc++] = portFlag;
             cli_argv[cliArgs.argc++] = portNumber;
         }

+ 1 - 1
tests/unit.c

@@ -52,7 +52,7 @@ int unit_test(int argc, char** argv)
 
 #ifdef WOLFSSL_FORCE_MALLOC_FAIL_TEST
     if (argc > 1) {
-        word32 memFailCount = atoi(argv[1]);
+        int memFailCount = atoi(argv[1]);
         printf("\n--- SET RNG MALLOC FAIL AT %d---\n", memFailCount);
         wolfSSL_SetMemFailCount(memFailCount);
     }

+ 1 - 1
testsuite/testsuite.c

@@ -378,7 +378,7 @@ static void simple_test(func_args* args)
 #ifndef USE_WINDOWS_API
     cliArgs.argc = NUMARGS;
     strcpy(argvc[1], "-p");
-    snprintf(argvc[2], sizeof(argvc[2]), "%d", svrArgs.signal->port);
+    snprintf(argvc[2], sizeof(argvc[2]), "%d", (int)svrArgs.signal->port);
 #else
     cliArgs.argc = 1;
 #endif

+ 39 - 24
wolfcrypt/src/aes.c

@@ -4344,12 +4344,11 @@ static WC_INLINE void FlattenSzInBits(byte* buf, word32 sz)
 static WC_INLINE void RIGHTSHIFTX(byte* x)
 {
     int i;
-    int carryOut = 0;
     int carryIn = 0;
     int borrow = x[15] & 0x01;
 
     for (i = 0; i < AES_BLOCK_SIZE; i++) {
-        carryOut = x[i] & 0x01;
+        int carryOut = x[i] & 0x01;
         x[i] = (x[i] >> 1) | (carryIn ? 0x80 : 0);
         carryIn = carryOut;
     }
@@ -5725,8 +5724,13 @@ void GHASH(Aes* aes, const byte* a, word32 aSz, const byte* c,
     byte x[AES_BLOCK_SIZE];
     byte scratch[AES_BLOCK_SIZE];
     word32 blocks, partial;
-    byte* h = aes->H;
+    byte* h;
 
+    if (aes == NULL) {
+        return;
+    }
+
+    h = aes->H;
     XMEMSET(x, 0, AES_BLOCK_SIZE);
 
     /* Hash in A, the Additional Authentication Data */
@@ -5924,6 +5928,10 @@ void GHASH(Aes* aes, const byte* a, word32 aSz, const byte* c,
     byte scratch[AES_BLOCK_SIZE];
     word32 blocks, partial;
 
+    if (aes == NULL) {
+        return;
+    }
+
     XMEMSET(x, 0, AES_BLOCK_SIZE);
 
     /* Hash in A, the Additional Authentication Data */
@@ -6220,6 +6228,10 @@ void GHASH(Aes* aes, const byte* a, word32 aSz, const byte* c,
     byte scratch[AES_BLOCK_SIZE];
     word32 blocks, partial;
 
+    if (aes == NULL) {
+        return;
+    }
+
     XMEMSET(x, 0, AES_BLOCK_SIZE);
 
     /* Hash in A, the Additional Authentication Data */
@@ -6352,6 +6364,10 @@ void GHASH(Aes* aes, const byte* a, word32 aSz, const byte* c,
     word32 blocks, partial;
     word64 bigH[2];
 
+    if (aes == NULL) {
+        return;
+    }
+
     XMEMCPY(bigH, aes->H, AES_BLOCK_SIZE);
     #ifdef LITTLE_ENDIAN_ORDER
         ByteReverseWords64(bigH, bigH, AES_BLOCK_SIZE);
@@ -6665,6 +6681,10 @@ void GHASH(Aes* aes, const byte* a, word32 aSz, const byte* c,
     word32 blocks, partial;
     word32 bigH[4];
 
+    if (aes == NULL) {
+        return;
+    }
+
     XMEMCPY(bigH, aes->H, AES_BLOCK_SIZE);
     #ifdef LITTLE_ENDIAN_ORDER
         ByteReverseWords(bigH, bigH, AES_BLOCK_SIZE);
@@ -10268,7 +10288,6 @@ int wc_AesEcbEncrypt(Aes* aes, byte* out, const byte* in, word32 sz)
       wc_AesEncryptDirect(aes, out, in);
       out += AES_BLOCK_SIZE;
       in  += AES_BLOCK_SIZE;
-      sz  -= AES_BLOCK_SIZE;
       blocks--;
     }
     return 0;
@@ -10289,7 +10308,6 @@ int wc_AesEcbDecrypt(Aes* aes, byte* out, const byte* in, word32 sz)
       wc_AesDecryptDirect(aes, out, in);
       out += AES_BLOCK_SIZE;
       in  += AES_BLOCK_SIZE;
-      sz  -= AES_BLOCK_SIZE;
       blocks--;
     }
     return 0;
@@ -10529,11 +10547,9 @@ static void shiftLeftArray(byte* ary, byte shift)
         ary[i] = 0;
     }
     else {
-        byte carry = 0;
-
         /* shifting over by 7 or less bits */
         for (i = 0; i < AES_BLOCK_SIZE - 1; i++) {
-            carry = ary[i+1] & (0XFF << (WOLFSSL_BIT_SIZE - shift));
+            byte carry = ary[i+1] & (0XFF << (WOLFSSL_BIT_SIZE - shift));
             carry >>= (WOLFSSL_BIT_SIZE - shift);
             ary[i] = (ary[i] << shift) + carry;
         }
@@ -11223,17 +11239,17 @@ int wc_AesXtsEncrypt(XtsAes* xaes, byte* out, const byte* in, word32 sz,
         while (blocks > 0) {
             word32 j;
             byte carry = 0;
-            byte buf[AES_BLOCK_SIZE];
 
     #ifdef HAVE_AES_ECB
-            if (in == out) { /* check for if inline */
+            if (in == out)
     #endif
-            XMEMCPY(buf, in, AES_BLOCK_SIZE);
-            xorbuf(buf, tmp, AES_BLOCK_SIZE);
-            wc_AesEncryptDirect(aes, out, buf);
-    #ifdef HAVE_AES_ECB
+            { /* check for if inline */
+                byte buf[AES_BLOCK_SIZE];
+
+                XMEMCPY(buf, in, AES_BLOCK_SIZE);
+                xorbuf(buf, tmp, AES_BLOCK_SIZE);
+                wc_AesEncryptDirect(aes, out, buf);
             }
-    #endif
             xorbuf(out, tmp, AES_BLOCK_SIZE);
 
             /* multiply by shift left and propagate carry */
@@ -11338,17 +11354,16 @@ int wc_AesXtsDecrypt(XtsAes* xaes, byte* out, const byte* in, word32 sz,
     #endif
 
         while (blocks > 0) {
-            byte buf[AES_BLOCK_SIZE];
-
     #ifdef HAVE_AES_ECB
-            if (in == out) { /* check for if inline */
+            if (in == out)
     #endif
-            XMEMCPY(buf, in, AES_BLOCK_SIZE);
-            xorbuf(buf, tmp, AES_BLOCK_SIZE);
-            wc_AesDecryptDirect(aes, out, buf);
-    #ifdef HAVE_AES_ECB
+            { /* check for if inline */
+                byte buf[AES_BLOCK_SIZE];
+
+                XMEMCPY(buf, in, AES_BLOCK_SIZE);
+                xorbuf(buf, tmp, AES_BLOCK_SIZE);
+                wc_AesDecryptDirect(aes, out, buf);
             }
-    #endif
             xorbuf(out, tmp, AES_BLOCK_SIZE);
 
             /* multiply by shift left and propagate carry */
@@ -11371,7 +11386,7 @@ int wc_AesXtsDecrypt(XtsAes* xaes, byte* out, const byte* in, word32 sz,
         }
 
         /* stealing operation of XTS to handle left overs */
-        if (sz > 0) {
+        if (sz >= AES_BLOCK_SIZE) {
             byte buf[AES_BLOCK_SIZE];
             byte tmp2[AES_BLOCK_SIZE];
 

+ 49 - 43
wolfcrypt/src/asn.c

@@ -9501,14 +9501,14 @@ static int StoreKey(DecodedCert* cert, const byte* source, word32* srcIdx,
         if (publicKey == NULL) {
             ret = MEMORY_E;
         }
-    }
-    if (ret == 0) {
-        XMEMCPY(publicKey, &source[*srcIdx], length);
-        cert->publicKey = publicKey;
-        cert->pubKeyStored = 1;
-        cert->pubKeySize   = length;
+        else {
+            XMEMCPY(publicKey, &source[*srcIdx], length);
+            cert->publicKey = publicKey;
+            cert->pubKeyStored = 1;
+            cert->pubKeySize   = length;
 
-        *srcIdx += length;
+            *srcIdx += length;
+        }
     }
 
     return ret;
@@ -9712,13 +9712,13 @@ static int StoreEccKey(DecodedCert* cert, const byte* source, word32* srcIdx,
         if (publicKey == NULL) {
             ret = MEMORY_E;
         }
-    }
-    if (ret == 0) {
-        /* Copy in whole public key and store pointer. */
-        XMEMCPY(publicKey, pubKey, cert->pubKeySize);
-        cert->publicKey = publicKey;
-        /* Indicate publicKey needs to be freed. */
-        cert->pubKeyStored = 1;
+        else {
+            /* Copy in whole public key and store pointer. */
+            XMEMCPY(publicKey, pubKey, cert->pubKeySize);
+            cert->publicKey = publicKey;
+            /* Indicate publicKey needs to be freed. */
+            cert->pubKeyStored = 1;
+        }
     }
 
     return ret;
@@ -14740,8 +14740,9 @@ static int DecodeAuthInfo(const byte* input, int sz, DecodedCert* cert)
         {
             cert->extAuthInfoSz = length;
             cert->extAuthInfo = input + idx;
+        #if defined(OPENSSL_ALL) && defined(WOLFSSL_QT)
             count++;
-        #if !defined(OPENSSL_ALL) || !defined(WOLFSSL_QT)
+        #else
             break;
         #endif
         }
@@ -15342,16 +15343,16 @@ static int DecodeSubtree(const byte* input, int sz, Base_entry** head,
 
     /* Process all subtrees. */
     while ((ret == 0) && (idx < (word32)sz)) {
-        byte min = 0;
-        byte max = 0;
+        byte minVal = 0;
+        byte maxVal = 0;
 
         /* Clear dynamic data and set choice for GeneralName and location to
          * store minimum and maximum.
          */
         XMEMSET(dataASN, 0, sizeof(*dataASN) * subTreeASN_Length);
         GetASN_Choice(&dataASN[1], generalNameChoice);
-        GetASN_Int8Bit(&dataASN[2], &min);
-        GetASN_Int8Bit(&dataASN[3], &max);
+        GetASN_Int8Bit(&dataASN[2], &minVal);
+        GetASN_Int8Bit(&dataASN[3], &maxVal);
         /* Parse GeneralSubtree. */
         ret = GetASN_Items(subTreeASN, dataASN, subTreeASN_Length, 0, input,
                            &idx, sz);
@@ -20136,7 +20137,7 @@ static int SetEccPublicKey(byte* output, ecc_key* key, int outLen,
 
     return idx;
 #else
-    word32 pubSz;
+    word32 pubSz = 0;
     int sz = 0;
     int ret = 0;
 
@@ -20189,7 +20190,7 @@ static int SetEccPublicKey(byte* output, ecc_key* key, int outLen,
     else if ((ret == 0) && (output != NULL) && (pubSz > (word32)outLen)) {
         ret = BUFFER_E;
     }
-    else if (ret == 0) {
+    else {
         /* Total size is the public point size. */
         sz = pubSz;
     }
@@ -21432,7 +21433,7 @@ static int wc_EncodeName_ex(EncodedName* name, const char* nameStr,
     return idx;
 #else
     ASNSetData dataASN[rdnASN_Length];
-    ASNItem nameASN[rdnASN_Length];
+    ASNItem namesASN[rdnASN_Length];
     byte dnOid[DN_OID_SZ] = { 0x55, 0x04, 0x00 };
     int ret = 0;
     int sz;
@@ -21449,7 +21450,7 @@ static int wc_EncodeName_ex(EncodedName* name, const char* nameStr,
         XMEMSET(dataASN, 0, rdnASN_Length * sizeof(ASNSetData));
         /* Copy the RDN encoding template. ASN.1 tag for the name string is set
          * based on type. */
-        XMEMCPY(nameASN, rdnASN, rdnASN_Length * sizeof(ASNItem));
+        XMEMCPY(namesASN, rdnASN, rdnASN_Length * sizeof(ASNItem));
 
         /* Set OID and ASN.1 tag for name depending on type. */
         switch (type) {
@@ -21472,18 +21473,17 @@ static int wc_EncodeName_ex(EncodedName* name, const char* nameStr,
                 oidSz = DN_OID_SZ;
                 break;
         }
-    }
-    if (ret == 0) {
+
         /* Set OID corresponding to the name type. */
         SetASN_Buffer(&dataASN[2], oid, oidSz);
         /* Set name string. */
         SetASN_Buffer(&dataASN[3], (const byte *)nameStr,
             (word32)XSTRLEN(nameStr));
         /* Set the ASN.1 tag for the name string. */
-        nameASN[3].tag = nameTag;
+        namesASN[3].tag = nameTag;
 
         /* Calculate size of encoded name and indexes of components. */
-        ret = SizeASN_Items(nameASN, dataASN, rdnASN_Length, &sz);
+        ret = SizeASN_Items(namesASN, dataASN, rdnASN_Length, &sz);
     }
     /* Check if name's buffer is big enough. */
     if ((ret == 0) && (sz > (int)sizeof(name->encoded))) {
@@ -21491,7 +21491,7 @@ static int wc_EncodeName_ex(EncodedName* name, const char* nameStr,
     }
     if (ret == 0) {
         /* Encode name into the buffer. */
-        SetASN_Items(nameASN, dataASN, rdnASN_Length, name->encoded);
+        SetASN_Items(namesASN, dataASN, rdnASN_Length, name->encoded);
         /* Cache the type and size, and set that it is used. */
         name->type = type;
         name->totalLen = sz;
@@ -21768,7 +21768,7 @@ int SetNameEx(byte* output, word32 outputSz, CertName* name, void* heap)
     if (dataASN == NULL) {
         ret = MEMORY_E;
     }
-    if (ret == 0) {
+    else {
         /* Allocate dynamic ASN.1 template items. */
         namesASN = (ASNItem*)XMALLOC(idx * sizeof(ASNItem), heap,
                                      DYNAMIC_TYPE_TMP_BUFFER);
@@ -26790,7 +26790,6 @@ static int DecodeAsymKey(const byte* input, word32* inOutIdx, word32 inSz,
     byte* privKey, word32* privKeyLen,
     byte* pubKey, word32* pubKeyLen, int keyType)
 {
-    int ret = 0;
 #ifndef WOLFSSL_ASN_TEMPLATE
     word32 oid;
     int version, length, endKeyIdx, privSz, pubSz;
@@ -26863,9 +26862,12 @@ static int DecodeAsymKey(const byte* input, word32* inOutIdx, word32 inSz,
         if (pubKey != NULL && pubKeyLen != NULL)
             XMEMCPY(pubKey, pub, *pubKeyLen);
     }
-    if (ret == 0 && endKeyIdx != (int)*inOutIdx)
+    if (endKeyIdx != (int)*inOutIdx)
         return ASN_PARSE_E;
+    return 0;
 #else
+    int ret = 0;
+
     CALLOC_ASNGETDATA(dataASN, edKeyASN_Length, ret, NULL);
 
     if (ret == 0) {
@@ -26910,8 +26912,8 @@ static int DecodeAsymKey(const byte* input, word32* inOutIdx, word32 inSz,
     }
 
     FREE_ASNGETDATA(dataASN, NULL);
-#endif /* WOLFSSL_ASN_TEMPLATE */
     return ret;
+#endif /* WOLFSSL_ASN_TEMPLATE */
 }
 
 static int DecodeAsymKeyPublic(const byte* input, word32* inOutIdx, word32 inSz,
@@ -27115,7 +27117,7 @@ static int SetAsymKeyDer(const byte* privKey, word32 privKeyLen,
     sz = seqSz + verSz + algoSz + privSz + pubSz;
 
     /* checkout output size */
-    if (ret == 0 && output != NULL && sz > outLen) {
+    if (output != NULL && sz > outLen) {
         ret = BAD_FUNC_ARG;
     }
 
@@ -28118,21 +28120,25 @@ static int DecodeResponseData(byte* source, word32* ioIndex,
                 if (single->next->status == NULL) {
                     XFREE(single->next, resp->heap, DYNAMIC_TYPE_OCSP_ENTRY);
                     single->next = NULL;
-                    return MEMORY_E;
+                    ret = MEMORY_E;
                 }
-                XMEMSET(single->next->status, 0, sizeof(CertStatus));
+                else {
+                    XMEMSET(single->next->status, 0, sizeof(CertStatus));
 
-                /* Entry to be freed. */
-                single->isDynamic = 1;
-                /* used will be 0 (false) */
+                    /* Entry to be freed. */
+                    single->isDynamic = 1;
+                    /* used will be 0 (false) */
 
-                single = single->next;
+                    single = single->next;
+                }
             }
         }
-        /* Decode SingleResponse into OcspEntry. */
-        ret = DecodeSingleResponse(source, &idx, dataASN[7].offset,
-                dataASN[6].length, single);
-        /* single->used set on successful decode. */
+        if (ret == 0) {
+            /* Decode SingleResponse into OcspEntry. */
+            ret = DecodeSingleResponse(source, &idx, dataASN[7].offset,
+                    dataASN[6].length, single);
+            /* single->used set on successful decode. */
+        }
     }
 
     /* Check if there were extensions. */

+ 5 - 3
wolfcrypt/test/test.c

@@ -24983,7 +24983,8 @@ static int curve25519_check_public_test(void)
 
 #endif /* HAVE_CURVE25519_SHARED_SECRET && HAVE_CURVE25519_KEY_IMPORT */
 
-#if defined(HAVE_CURVE25519_KEY_EXPORT) && defined(HAVE_CURVE25519_KEY_IMPORT)
+#if !defined(NO_ASN) && defined(HAVE_CURVE25519_KEY_EXPORT) && \
+    defined(HAVE_CURVE25519_KEY_IMPORT)
 static int curve255519_der_test(void)
 {
     int ret = 0;
@@ -25058,7 +25059,7 @@ static int curve255519_der_test(void)
 
     return ret;
 }
-#endif /* HAVE_CURVE25519_KEY_EXPORT && HAVE_CURVE25519_KEY_IMPORT */
+#endif /* !NO_ASN && HAVE_CURVE25519_KEY_EXPORT && HAVE_CURVE25519_KEY_IMPORT */
 
 WOLFSSL_TEST_SUBROUTINE int curve25519_test(void)
 {
@@ -25242,7 +25243,8 @@ WOLFSSL_TEST_SUBROUTINE int curve25519_test(void)
         return ret;
 #endif /* HAVE_CURVE25519_SHARED_SECRET && HAVE_CURVE25519_KEY_IMPORT */
 
-#if defined(HAVE_CURVE25519_KEY_IMPORT) && defined(HAVE_CURVE25519_KEY_IMPORT)
+#if !defined(NO_ASN) && defined(HAVE_CURVE25519_KEY_EXPORT) && \
+    defined(HAVE_CURVE25519_KEY_IMPORT)
     ret = curve255519_der_test();
     if (ret != 0)
         return ret;

+ 8 - 0
wolfssl/test.h

@@ -261,10 +261,18 @@
     #elif defined(WOLFSSL_TIRTOS)
         typedef void          THREAD_RETURN;
         typedef Task_Handle   THREAD_TYPE;
+        #ifdef HAVE_STACK_SIZE
+          #undef EXIT_TEST
+          #define EXIT_TEST(ret)
+        #endif
         #define WOLFSSL_THREAD
     #elif defined(WOLFSSL_ZEPHYR)
         typedef void            THREAD_RETURN;
         typedef struct k_thread THREAD_TYPE;
+        #ifdef HAVE_STACK_SIZE
+          #undef EXIT_TEST
+          #define EXIT_TEST(ret)
+        #endif
         #define WOLFSSL_THREAD
     #else
         typedef unsigned int  THREAD_RETURN;

+ 1 - 1
wolfssl/wolfcrypt/types.h

@@ -1080,7 +1080,7 @@ decouple library dependencies with standard string, memory and so on.
 
 
     #if defined(HAVE_STACK_SIZE)
-        #define EXIT_TEST(ret) return (void*)((size_t)(ret))
+        #define EXIT_TEST(ret) return (THREAD_RETURN)((size_t)(ret))
     #else
         #define EXIT_TEST(ret) return ret
     #endif