Merge pull request #9817 from LinuxJedi/static-fixes4

Static code analysis fixes

Author: JacobBarthelmeh

src/crl.c

@@ -98,6 +98,35 @@ int InitCRL(WOLFSSL_CRL* crl, WOLFSSL_CERT_MANAGER* cm)
 }
 
 
+#ifdef CRL_STATIC_REVOKED_LIST
+/* Compare two RevokedCert entries by (serialSz, serialNumber) for sorting.
+ * Returns < 0, 0, or > 0 like memcmp. */
+static int CompareRevokedCert(const RevokedCert* a, const RevokedCert* b)
+{
+    if (a->serialSz != b->serialSz)
+        return a->serialSz - b->serialSz;
+    return XMEMCMP(a->serialNumber, b->serialNumber, (size_t)a->serialSz);
+}
+
+/* Sort revoked cert array in-place using insertion sort. The array is bounded
+ * by CRL_MAX_REVOKED_CERTS so O(n^2) is fine. */
+static void SortCRL_CertList(RevokedCert* certs, int totalCerts)
+{
+    int i, j;
+    RevokedCert tmp;
+
+    for (i = 1; i < totalCerts; i++) {
+        XMEMCPY(&tmp, &certs[i], sizeof(RevokedCert));
+        j = i - 1;
+        while (j >= 0 && CompareRevokedCert(&certs[j], &tmp) > 0) {
+            XMEMCPY(&certs[j + 1], &certs[j], sizeof(RevokedCert));
+            j--;
+        }
+        XMEMCPY(&certs[j + 1], &tmp, sizeof(RevokedCert));
+    }
+}
+#endif /* CRL_STATIC_REVOKED_LIST */
+
 /* Initialize CRL Entry */
 static int InitCRL_Entry(CRL_Entry* crle, DecodedCRL* dcrl, const byte* buff,
                          int verified, void* heap)
@@ -132,12 +161,15 @@ static int InitCRL_Entry(CRL_Entry* crle, DecodedCRL* dcrl, const byte* buff,
 #endif
 #ifdef CRL_STATIC_REVOKED_LIST
     /* ParseCRL_CertList() has already cached the Revoked certs into
-       the crle->certs array */
+       the crle->certs array. Sort it so binary search in
+       FindRevokedSerial works correctly. */
+    crle->totalCerts = dcrl->totalCerts;
+    SortCRL_CertList(crle->certs, crle->totalCerts);
 #else
     crle->certs = dcrl->certs;   /* take ownership */
+    crle->totalCerts = dcrl->totalCerts;
 #endif
     dcrl->certs = NULL;
-    crle->totalCerts = dcrl->totalCerts;
     crle->crlNumberSet = dcrl->crlNumberSet;
     if (crle->crlNumberSet) {
         XMEMCPY(crle->crlNumber, dcrl->crlNumber, sizeof(crle->crlNumber));
@@ -313,25 +345,52 @@ static int FindRevokedSerial(RevokedCert* rc, byte* serial, int serialSz,
     int ret = 0;
     byte hash[SIGNER_DIGEST_SIZE];
 #ifdef CRL_STATIC_REVOKED_LIST
-    /* do binary search */
-    int low, high, mid;
+    if (serialHash == NULL) {
+        /* Binary search by (serialSz, serialNumber). The array was sorted in
+         * InitCRL_Entry by the same comparison key. */
+        int low = 0;
+        int high = totalCerts - 1;
 
-    low = 0;
-    high = totalCerts - 1;
+        while (low <= high) {
+            int mid = (low + high) / 2;
+            int cmp;
 
-    while (low <= high) {
-        mid = (low + high) / 2;
+            /* Compare by serial size first, then by serial content. Shorter
+             * serials sort before longer ones. */
+            if (rc[mid].serialSz != serialSz) {
+                cmp = rc[mid].serialSz - serialSz;
+            }
+            else {
+                cmp = XMEMCMP(rc[mid].serialNumber, serial,
+                              (size_t)rc[mid].serialSz);
+            }
 
-        if (XMEMCMP(rc[mid].serialNumber, serial, rc->serialSz) < 0) {
-            low = mid + 1;
-        }
-        else if (XMEMCMP(rc[mid].serialNumber, serial, rc->serialSz) > 0) {
-            high = mid - 1;
+            if (cmp < 0) {
+                low = mid + 1;
+            }
+            else if (cmp > 0) {
+                high = mid - 1;
+            }
+            else {
+                WOLFSSL_MSG("Cert revoked");
+                ret = CRL_CERT_REVOKED;
+                break;
+            }
         }
-        else {
-            WOLFSSL_MSG("Cert revoked");
-            ret = CRL_CERT_REVOKED;
-            break;
+    }
+    else {
+        /* Hash-based lookup -- linear scan required since the array is sorted
+         * by serial number, not by hash. */
+        int i;
+        for (i = 0; i < totalCerts; i++) {
+            ret = CalcHashId(rc[i].serialNumber, (word32)rc[i].serialSz, hash);
+            if (ret != 0)
+                break;
+            if (XMEMCMP(hash, serialHash, SIGNER_DIGEST_SIZE) == 0) {
+                WOLFSSL_MSG("Cert revoked");
+                ret = CRL_CERT_REVOKED;
+                break;
+            }
         }
     }
 #else

src/internal.c

@@ -1123,8 +1123,8 @@ static int ImportKeyState(WOLFSSL* ssl, const byte* exp, word32 len, byte ver,
         idx += OPAQUE16_LEN;
 
         if (wordCount > WOLFSSL_DTLS_WINDOW_WORDS) {
+            wordAdj = (wordCount - WOLFSSL_DTLS_WINDOW_WORDS) * sizeof(word32);
             wordCount = WOLFSSL_DTLS_WINDOW_WORDS;
-            wordAdj = (WOLFSSL_DTLS_WINDOW_WORDS - wordCount) * sizeof(word32);
         }
 
         XMEMSET(keys->peerSeq[0].window, 0xFF, DTLS_SEQ_SZ);
@@ -1139,8 +1139,8 @@ static int ImportKeyState(WOLFSSL* ssl, const byte* exp, word32 len, byte ver,
         idx += OPAQUE16_LEN;
 
         if (wordCount > WOLFSSL_DTLS_WINDOW_WORDS) {
+            wordAdj = (wordCount - WOLFSSL_DTLS_WINDOW_WORDS) * sizeof(word32);
             wordCount = WOLFSSL_DTLS_WINDOW_WORDS;
-            wordAdj = (WOLFSSL_DTLS_WINDOW_WORDS - wordCount) * sizeof(word32);
         }
 
         XMEMSET(keys->peerSeq[0].prevWindow, 0xFF, DTLS_SEQ_SZ);

src/ssl.c

@@ -16675,8 +16675,12 @@ int wolfSSL_select_next_proto(unsigned char **out, unsigned char *outLen,
 
     for (i = 0; i < inLen; i += lenIn) {
         lenIn = in[i++];
+        if (lenIn == 0 || i + lenIn > inLen)
+            break;
         for (j = 0; j < clientLen; j += lenClient) {
             lenClient = clientNames[j++];
+            if (lenClient == 0 || j + lenClient > clientLen)
+                break;
 
             if (lenIn != lenClient)
                 continue;
@@ -16689,8 +16693,14 @@ int wolfSSL_select_next_proto(unsigned char **out, unsigned char *outLen,
         }
     }
 
-    *out = (unsigned char *)clientNames + 1;
-    *outLen = clientNames[0];
+    if (clientLen > 0 && (unsigned int)clientNames[0] + 1 <= clientLen) {
+        *out = (unsigned char *)clientNames + 1;
+        *outLen = clientNames[0];
+    }
+    else {
+        *out = (unsigned char *)clientNames;
+        *outLen = 0;
+    }
     return WOLFSSL_NPN_NO_OVERLAP;
 }
 

src/tls.c

@@ -13605,6 +13605,9 @@ static int TLSX_ECH_Parse(WOLFSSL* ssl, const byte* readBuf, word16 size,
         }
         /* read hello inner len */
         ato16(readBuf_p, &ech->innerClientHelloLen);
+        if (ech->innerClientHelloLen < WC_AES_BLOCK_SIZE) {
+            return BUFFER_ERROR;
+        }
         ech->innerClientHelloLen -= WC_AES_BLOCK_SIZE;
         readBuf_p += 2;
         ech->outerClientPayload = readBuf_p;