Various hardening fixes across sniffer, QUIC, PKCS#11, TLS and tooling

.github/workflows/membrowse-zephyr.yml

@@ -45,6 +45,7 @@ jobs:
         with:
           ref: ${{ github.event.workflow_run.head_sha }}
           fetch-depth: 2
+          persist-credentials: false
 
       - name: Download Zephyr build artifact
         id: download
@@ -77,7 +78,10 @@ jobs:
 
       - name: Run Membrowse PR Action
         if: steps.verify.outputs.have_artifacts == 'true'
-        uses: membrowse/membrowse-action@v1
+        # Pinned to a full commit SHA: this job downloads fork-controlled
+        # artifacts and holds MEMBROWSE_API_KEY, so a moved tag must not be
+        # able to swap the action out from under it. v1.2.9.
+        uses: membrowse/membrowse-action@3f23ea562abaa6614855d6be5ec83b3cd1d274dd
         with:
           target_name: ${{ matrix.target_name }}
           elf: zephyr-artifacts/${{ matrix.target_name }}/zephyr.elf
@@ -106,7 +110,8 @@ jobs:
 
       - name: Post Membrowse PR comment
         if: ${{ env.MEMBROWSE_API_KEY != '' }}
-        uses: membrowse/membrowse-action/comment-action@v1
+        # Pinned to a full commit SHA, same rationale as the analyze job. v1.2.9.
+        uses: membrowse/membrowse-action/comment-action@3f23ea562abaa6614855d6be5ec83b3cd1d274dd
         with:
           api_key: ${{ secrets.MEMBROWSE_API_KEY }}
           commit: ${{ github.event.workflow_run.head_sha }}

examples/asn1/asn1.c

@@ -98,13 +98,20 @@ static int asn1App_ReadFile(FILE* fp, unsigned char** pdata, word32* plen)
             /* Add read data amount to length. */
             len += (word32)read_len;
 
+            /* Stop before the length or reallocation size can wrap word32. */
+            if (len > (word32)(0xFFFFFFFFU - DATA_INC_LEN)) {
+                XFREE(data, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+                data = NULL;
+                break;
+            }
+
             /* Stop if we are at end-of-file. */
             if (feof(fp)) {
                 break;
             }
 
             /* Make space for more data to be added to buffer. */
-            p = (unsigned char*)XREALLOC(data, len + DATA_INC_LEN, NULL,
+            p = (unsigned char*)XREALLOC(data, (size_t)len + DATA_INC_LEN, NULL,
                 DYNAMIC_TYPE_TMP_BUFFER);
             if (p == NULL) {
                 /* Reallocation failed - free current buffer. */

examples/pem/pem.c

@@ -112,13 +112,20 @@ static int pemApp_ReadFile(FILE* fp, unsigned char** pdata, word32* plen)
             /* Add read data amount to length. */
             len += (word32)read_len;
 
+            /* Stop before the length or reallocation size can wrap word32. */
+            if (len > (word32)(0xFFFFFFFFU - DATA_INC_LEN - BLOCK_SIZE_MAX)) {
+                XFREE(data, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+                data = NULL;
+                break;
+            }
+
             /* Stop if we are at end-of-file. */
             if (feof(fp)) {
                 break;
             }
 
             /* Make space for more data to be added to buffer. */
-            p = (unsigned char*)XREALLOC(data, len + DATA_INC_LEN +
+            p = (unsigned char*)XREALLOC(data, (size_t)len + DATA_INC_LEN +
                 BLOCK_SIZE_MAX, NULL, DYNAMIC_TYPE_TMP_BUFFER);
             if (p == NULL) {
                 /* Reallocation failed - free current buffer. */

src/quic.c

@@ -75,12 +75,12 @@ static QuicRecord *quic_record_make(WOLFSSL *ssl,
         XMEMSET(qr, 0, sizeof(*qr));
         qr->level = level;
         if (level == wolfssl_encryption_early_data) {
-            qr->capacity = qr->len = (word32)len;
-            if (qr->capacity > WOLFSSL_QUIC_MAX_RECORD_CAPACITY) {
+            if (len > (size_t)WOLFSSL_QUIC_MAX_RECORD_CAPACITY) {
                 WOLFSSL_MSG("QUIC early data length larger than expected");
                 quic_record_free(ssl, qr);
                 return NULL;
             }
+            qr->capacity = qr->len = (word32)len;
         }
         else {
             qr->capacity = qr->len = (word32) qr_length(data, len);
@@ -147,6 +147,9 @@ static int quic_record_append(WOLFSSL *ssl, QuicRecord *qr, const uint8_t *data,
             uint8_t *ndata = (uint8_t*)XREALLOC(qr->data, qr->len, ssl->heap,
                                                 DYNAMIC_TYPE_TMP_BUFFER);
             if (!ndata) {
+                /* keep len consistent with the unchanged buffer so a later
+                 * append does not copy into the smaller allocation */
+                qr->len = 0;
                 ret = WOLFSSL_FAILURE;
                 goto cleanup;
             }

src/sniffer.c

@@ -2138,7 +2138,13 @@ static int CheckIp6Hdr(Ip6Hdr* iphdr, IpInfo* info, int length, char* error)
     if (iphdr->next_header != TCP_PROTOCOL) {
         Ip6ExtHdr* exthdr = (Ip6ExtHdr*)((byte*)iphdr + IP6_HDR_SZ);
         do {
-            int hdrsz = (exthdr->length + 1) * 8;
+            int hdrsz;
+            /* make sure the extension header is fully present before reading */
+            if (length - exthdrsz < (int)sizeof(Ip6ExtHdr)) {
+                SetError(PACKET_HDR_SHORT_STR, error, NULL, 0);
+                return WOLFSSL_FATAL_ERROR;
+            }
+            hdrsz = (exthdr->length + 1) * 8;
             if (hdrsz > length - exthdrsz) {
                 SetError(PACKET_HDR_SHORT_STR, error, NULL, 0);
                 return WOLFSSL_FATAL_ERROR;
@@ -3733,6 +3739,10 @@ static int ProcessServerHello(int msgSz, const byte* input, int* sslBytes,
         return WOLFSSL_FATAL_ERROR;
     }
     if (b) {
+        if (ID_LEN > *sslBytes) {
+            SetError(SERVER_HELLO_INPUT_STR, error, session, FATAL_ERROR_STATE);
+            return WOLFSSL_FATAL_ERROR;
+        }
     #ifdef WOLFSSL_TLS13
         XMEMCPY(session->sslServer->session->sessionID, input, ID_LEN);
         session->sslServer->session->sessionIDSz = ID_LEN;
@@ -3804,6 +3814,13 @@ static int ProcessServerHello(int msgSz, const byte* input, int* sslBytes,
             word16 extType;
             word16 extLen;
 
+            /* make sure can read extension type and length */
+            if (*sslBytes < EXT_TYPE_SZ + LENGTH_SZ) {
+                SetError(SERVER_HELLO_INPUT_STR, error, session,
+                         FATAL_ERROR_STATE);
+                return WOLFSSL_FATAL_ERROR;
+            }
+
             extType    = (word16)((input[0] << 8) | input[1]);
             input     += EXT_TYPE_SZ;
             *sslBytes -= EXT_TYPE_SZ;
@@ -3895,6 +3912,13 @@ static int ProcessServerHello(int msgSz, const byte* input, int* sslBytes,
                 break;
             } /* switch (extType) */
 
+            /* make sure the extension fits in the remaining declared length */
+            if ((word32)extLen + EXT_TYPE_SZ + LENGTH_SZ > len) {
+                SetError(SERVER_HELLO_INPUT_STR, error, session,
+                         FATAL_ERROR_STATE);
+                return WOLFSSL_FATAL_ERROR;
+            }
+
             input     += extLen;
             *sslBytes -= extLen;
             len       -= extLen + EXT_TYPE_SZ + LENGTH_SZ;
@@ -4175,6 +4199,12 @@ static int ProcessClientHello(const byte* input, int* sslBytes,
         word16 extType;
         word16 extLen;
 
+        /* make sure can read extension type and length */
+        if (*sslBytes < EXT_TYPE_SZ + LENGTH_SZ) {
+            SetError(CLIENT_HELLO_INPUT_STR, error, session, FATAL_ERROR_STATE);
+            return WOLFSSL_FATAL_ERROR;
+        }
+
         extType    = (word16)((input[0] << 8) | input[1]);
         input     += EXT_TYPE_SZ;
         *sslBytes -= EXT_TYPE_SZ;
@@ -4362,6 +4392,12 @@ static int ProcessClientHello(const byte* input, int* sslBytes,
             break;
         }
 
+        /* make sure the extension fits in the remaining declared length */
+        if ((word32)extLen + EXT_TYPE_SZ + LENGTH_SZ > len) {
+            SetError(CLIENT_HELLO_INPUT_STR, error, session, FATAL_ERROR_STATE);
+            return WOLFSSL_FATAL_ERROR;
+        }
+
         input     += extLen;
         *sslBytes -= extLen;
         len       -= extLen + EXT_TYPE_SZ + LENGTH_SZ;

src/ssl.c

@@ -6203,16 +6203,16 @@ int wolfSSL_set_compression(WOLFSSL* ssl)
          * a chain */
         if ((flags & WOLFSSL_BIO_FLAG_READ) && (ssl->biord != NULL)) {
             if ((flags & WOLFSSL_BIO_FLAG_WRITE) && (ssl->biord != ssl->biowr)) {
-                if (ssl->biowr != NULL && ssl->biowr->prev != NULL)
+                if (ssl->biowr != NULL && ssl->biowr->prev == NULL)
                     wolfSSL_BIO_free(ssl->biowr);
                 ssl->biowr = NULL;
             }
-            if (ssl->biord->prev != NULL)
+            if (ssl->biord->prev == NULL)
                 wolfSSL_BIO_free(ssl->biord);
             ssl->biord = NULL;
         }
         else if ((flags & WOLFSSL_BIO_FLAG_WRITE) && (ssl->biowr != NULL)) {
-            if (ssl->biowr->prev != NULL)
+            if (ssl->biowr->prev == NULL)
                 wolfSSL_BIO_free(ssl->biowr);
             ssl->biowr = NULL;
         }

wolfcrypt/src/asn.c

@@ -35625,6 +35625,12 @@ int CompareOcspReqResp(OcspRequest* req, OcspResponse* resp)
 
     /* Nonces are not critical. The responder may not necessarily add
      * the nonce to the response. */
+#ifdef WOLFSSL_FORCE_OCSP_NONCE_CHECK
+    if (req->nonceSz && resp->nonce == NULL) {
+        WOLFSSL_MSG("\tnonce required but missing from response");
+        return WOLFSSL_FATAL_ERROR;
+    }
+#endif
     if (req->nonceSz && resp->nonce != NULL
 #ifndef WOLFSSL_FORCE_OCSP_NONCE_CHECK
             && resp->nonceSz != 0

wolfcrypt/src/compress.c

@@ -213,6 +213,7 @@ int wc_DeCompressDynamic(byte** out, int maxSz, int memoryType,
     int result   = 0;
     int i;
     word32 tmpSz = 0;
+    word32 maxBufSz;
     byte*  tmp;
 
     (void)memoryType;
@@ -221,14 +222,27 @@ int wc_DeCompressDynamic(byte** out, int maxSz, int memoryType,
     if (out == NULL || in == NULL) {
         return BAD_FUNC_ARG;
     }
-    /* Cap input so the initial doubling and additive growth in the loop
-     * cannot overflow word32 or the int return type. */
-    if (inSz > (word32)(INT_MAX / 2)) {
+    /* Cap input so the initial doubling and the growth in the loop cannot
+     * overflow word32 or the int return type. Zero input has no buffer to
+     * size against. */
+    if (inSz == 0 || inSz > (word32)(INT_MAX / 2)) {
         return BAD_FUNC_ARG;
     }
     i = (maxSz == 1)? 1 : 2; /* start with output buffer twice the size of input
                               * unless max was set to 1 */
 
+    /* Largest output buffer permitted: inSz * maxSz keeps the historic
+     * maxSz ratio, clamped to INT_MAX (and unbounded becomes INT_MAX). */
+    if (maxSz > 0) {
+        if ((word32)maxSz > (word32)INT_MAX / inSz)
+            maxBufSz = (word32)INT_MAX;
+        else
+            maxBufSz = inSz * (word32)maxSz;
+    }
+    else {
+        maxBufSz = (word32)INT_MAX;
+    }
+
     stream.next_in = (Bytef*)in;
     stream.avail_in = (uInt)inSz;
     /* Check for source > 64K on 16-bit machine: */
@@ -277,18 +291,17 @@ int wc_DeCompressDynamic(byte** out, int maxSz, int memoryType,
             word32 newSz;
             byte*  newTmp;
 
-            if (maxSz > 0 && i >= maxSz) {
+            if (tmpSz >= maxBufSz) {
                 WOLFSSL_MSG("Hit max decompress size!");
                 break;
             }
-            i++;
 
-            if (tmpSz > (word32)INT_MAX - inSz) {
-                WOLFSSL_MSG("Decompress buffer would exceed INT_MAX");
-                result = DECOMPRESS_E;
-                break;
-            }
-            newSz = tmpSz + inSz;
+            /* Double the buffer so cumulative copy work stays linear in the
+             * output size, clamped to the permitted maximum. */
+            if (tmpSz > maxBufSz / 2)
+                newSz = maxBufSz;
+            else
+                newSz = tmpSz * 2;
             newTmp = (byte*)XMALLOC(newSz, heap, memoryType);
             if (newTmp == NULL) {
                 WOLFSSL_MSG("Memory error with increasing buffer size");
@@ -298,14 +311,14 @@ int wc_DeCompressDynamic(byte** out, int maxSz, int memoryType,
             XFREE(tmp, heap, memoryType);
             tmp   = newTmp;
             stream.next_out  = tmp + stream.total_out;
-            stream.avail_out = stream.avail_out + (uInt)inSz;
+            stream.avail_out = (uInt)(newSz - stream.total_out);
             tmpSz  = newSz;
             result = inflate(&stream, Z_BLOCK);
         }
     } while (result == Z_OK);
 
     if (result == Z_STREAM_END) {
-        if (stream.total_out > (uLong)INT_MAX) {
+        if (stream.total_out >= (uLong)INT_MAX) {
             result = DECOMPRESS_E;
         }
         else {

wolfcrypt/src/port/Espressif/esp_crt_bundle/esp_crt_bundle.c

@@ -1382,6 +1382,13 @@ static esp_err_t wolfssl_esp_crt_bundle_init(const uint8_t *x509_bundle,
             ESP_LOGCBI(TAG, "- This certificate at 0x%x, length: %u",
                              (intptr_t)cur_crt, cert_len);
 
+            if (cur_crt + CRT_HEADER_OFFSET + cert_len > bundle_end) {
+                ESP_LOGE(TAG, "Invalid certificate bundle cert length");
+                _esp_crt_bundle_is_valid = ESP_FAIL;
+                ret = ESP_ERR_INVALID_ARG;
+                break;
+            }
+
             /* TODO: optional gate out serial check for performance.       */
             /* Useful only for custom cert bundle, known to have no zeros. */
             if (wolfssl_is_zero_serial_number(cur_crt + CRT_HEADER_OFFSET,

wolfcrypt/src/wc_pkcs11.c

@@ -2513,6 +2513,13 @@ static int Pkcs11GetRsaPublicKey(RsaKey* key, Pkcs11Session* session,
     if (ret == 0) {
         modSz = (int)tmpl[0].ulValueLen;
         expSz = (int)tmpl[1].ulValueLen;
+        /* reject token lengths that do not fit in a positive int */
+        if (modSz <= 0 || (CK_ULONG)modSz != tmpl[0].ulValueLen ||
+                expSz <= 0 || (CK_ULONG)expSz != tmpl[1].ulValueLen) {
+            ret = WC_HW_E;
+        }
+    }
+    if (ret == 0) {
         mod = (unsigned char*)XMALLOC(modSz, key->heap,
                                                        DYNAMIC_TYPE_TMP_BUFFER);
         if (mod == NULL)
@@ -2526,7 +2533,9 @@ static int Pkcs11GetRsaPublicKey(RsaKey* key, Pkcs11Session* session,
     }
     if (ret == 0) {
         tmpl[0].pValue = mod;
+        tmpl[0].ulValueLen = (CK_ULONG)modSz;
         tmpl[1].pValue = exp;
+        tmpl[1].ulValueLen = (CK_ULONG)expSz;
 
         PKCS11_DUMP_TEMPLATE("Get RSA Public Key", tmpl, tmplCnt);
         rv = session->func->C_GetAttributeValue(session->handle, pubKey,
@@ -3272,12 +3281,19 @@ static int Pkcs11GetEccPublicKey(ecc_key* key, Pkcs11Session* session,
 
     if (ret == 0) {
         pointSz = (int)tmpl[0].ulValueLen;
+        /* reject a token length that does not fit in a positive int */
+        if (pointSz <= 0 || (CK_ULONG)pointSz != tmpl[0].ulValueLen) {
+            ret = WC_HW_E;
+        }
+    }
+    if (ret == 0) {
         point = (unsigned char*)XMALLOC(pointSz, key->heap, DYNAMIC_TYPE_ECC);
         if (point == NULL)
             ret = MEMORY_E;
     }
     if (ret == 0) {
         tmpl[0].pValue = point;
+        tmpl[0].ulValueLen = (CK_ULONG)pointSz;
 
         PKCS11_DUMP_TEMPLATE("Get Ec Public Key", tmpl, tmplCnt);
         rv = session->func->C_GetAttributeValue(session->handle, pubKey,
@@ -6223,6 +6239,7 @@ static int Pkcs11GetCert(Pkcs11Session* session, wc_CryptoInfo* info) {
     CK_ULONG            count = 0;
     CK_OBJECT_HANDLE    certHandle = CK_INVALID_HANDLE;
     byte               *certData = NULL;
+    int                 certDataSz = 0;
     CK_ATTRIBUTE    certTemplate[2] = {
         { CKA_CLASS,           &certClass, sizeof(certClass)   }
     };
@@ -6264,19 +6281,21 @@ static int Pkcs11GetCert(Pkcs11Session* session, wc_CryptoInfo* info) {
         goto exit;
     }
 
-    if (tmpl[0].ulValueLen <= 0) {
+    certDataSz = (int)tmpl[0].ulValueLen;
+    /* reject a token length that does not fit in a positive int */
+    if (certDataSz <= 0 || (CK_ULONG)certDataSz != tmpl[0].ulValueLen) {
         ret = WC_HW_E;
         goto exit;
     }
 
-    certData = (byte *)XMALLOC(
-        (int)tmpl[0].ulValueLen, info->cert.heap, DYNAMIC_TYPE_CERT);
+    certData = (byte *)XMALLOC(certDataSz, info->cert.heap, DYNAMIC_TYPE_CERT);
     if (certData == NULL) {
         ret = MEMORY_E;
         goto exit;
     }
 
     tmpl[0].pValue = certData;
+    tmpl[0].ulValueLen = (CK_ULONG)certDataSz;
     rv = session->func->C_GetAttributeValue(
         session->handle, certHandle, tmpl, tmplCnt);
     PKCS11_RV("C_GetAttributeValue", rv);
@@ -6286,7 +6305,7 @@ static int Pkcs11GetCert(Pkcs11Session* session, wc_CryptoInfo* info) {
     }
 
     *info->cert.certDataOut = certData;
-    *info->cert.certSz = (word32)tmpl[0].ulValueLen;
+    *info->cert.certSz = (word32)certDataSz;
     if (info->cert.certFormatOut != NULL) {
         *info->cert.certFormatOut = CTC_FILETYPE_ASN1;
     }