Merge pull request #9824 from embhorn/zd21239

Fix issues in TLS Extension size calculations

Author: JacobBarthelmeh

src/tls.c

@@ -2135,10 +2135,10 @@ static void TLSX_SNI_FreeAll(SNI* list, void* heap)
 }
 
 /** Tells the buffered size of the SNI objects in a list. */
-static word16 TLSX_SNI_GetSize(SNI* list)
+WOLFSSL_TEST_VIS word16 TLSX_SNI_GetSize(SNI* list)
 {
     SNI* sni;
-    word16 length = OPAQUE16_LEN; /* list length */
+    word32 length = OPAQUE16_LEN; /* list length */
 
     while ((sni = list)) {
         list = sni->next;
@@ -2147,12 +2147,16 @@ static word16 TLSX_SNI_GetSize(SNI* list)
 
         switch (sni->type) {
             case WOLFSSL_SNI_HOST_NAME:
-                length += (word16)XSTRLEN((char*)sni->data.host_name);
+                length += (word32)XSTRLEN((char*)sni->data.host_name);
             break;
         }
+
+        if (length > WOLFSSL_MAX_16BIT) {
+            return 0;
+        }
     }
 
-    return length;
+    return (word16)length;
 }
 
 /** Writes the SNI objects of a list in a buffer. */
@@ -3216,7 +3220,7 @@ static void TLSX_CSR_Free(CertificateStatusRequest* csr, void* heap)
 word16 TLSX_CSR_GetSize_ex(CertificateStatusRequest* csr, byte isRequest,
                                                              int idx)
 {
-    word16 size = 0;
+    word32 size = 0;
 
     /* shut up compiler warnings */
     (void) csr; (void) isRequest;
@@ -3237,15 +3241,25 @@ word16 TLSX_CSR_GetSize_ex(CertificateStatusRequest* csr, byte isRequest,
         if (csr->ssl != NULL && SSL_CM(csr->ssl) != NULL &&
                 SSL_CM(csr->ssl)->ocsp_stapling != NULL &&
                 SSL_CM(csr->ssl)->ocsp_stapling->statusCb != NULL) {
-            return OPAQUE8_LEN + OPAQUE24_LEN + csr->ssl->ocspCsrResp[idx].length;
+            if (WOLFSSL_MAX_16BIT - OPAQUE8_LEN - OPAQUE24_LEN <
+                    csr->ssl->ocspCsrResp[idx].length) {
+                return 0;
+            }
+            size = OPAQUE8_LEN + OPAQUE24_LEN +
+                    csr->ssl->ocspCsrResp[idx].length;
+            return (word16)size;
         }
-        return (word16)(OPAQUE8_LEN + OPAQUE24_LEN +
-                csr->responses[idx].length);
+        if (WOLFSSL_MAX_16BIT - OPAQUE8_LEN - OPAQUE24_LEN <
+                csr->responses[idx].length) {
+            return 0;
+        }
+        size = OPAQUE8_LEN + OPAQUE24_LEN + csr->responses[idx].length;
+        return (word16)size;
     }
 #else
     (void)idx;
 #endif
-    return size;
+    return (word16)size;
 }
 
 #if (defined(WOLFSSL_TLS13) && !defined(NO_WOLFSSL_SERVER))
@@ -3855,7 +3869,7 @@ static void TLSX_CSR2_FreeAll(CertificateStatusRequestItemV2* csr2, void* heap)
 static word16 TLSX_CSR2_GetSize(CertificateStatusRequestItemV2* csr2,
                                                                  byte isRequest)
 {
-    word16 size = 0;
+    word32 size = 0;
 
     /* shut up compiler warnings */
     (void) csr2; (void) isRequest;
@@ -3876,11 +3890,15 @@ static word16 TLSX_CSR2_GetSize(CertificateStatusRequestItemV2* csr2,
                         size += OCSP_NONCE_EXT_SZ;
                 break;
             }
+
+            if (size > WOLFSSL_MAX_16BIT) {
+                return 0;
+            }
         }
     }
 #endif
 
-    return size;
+    return (word16)size;
 }
 
 static int TLSX_CSR2_Write(CertificateStatusRequestItemV2* csr2,

tests/api.c

@@ -33145,6 +33145,7 @@ TEST_CASE testCases[] = {
     TEST_DECL(test_certificate_authorities_certificate_request),
     TEST_DECL(test_certificate_authorities_client_hello),
     TEST_DECL(test_TLSX_TCA_Find),
+    TEST_DECL(test_TLSX_SNI_GetSize_overflow),
     TEST_DECL(test_wolfSSL_wolfSSL_UseSecureRenegotiation),
     TEST_DECL(test_wolfSSL_SCR_Reconnect),
     TEST_DECL(test_wolfSSL_SCR_check_enabled),

tests/api/test_tls_ext.c

@@ -545,3 +545,67 @@ int test_certificate_authorities_client_hello(void) {
 #endif
     return EXPECT_RESULT();
 }
+
+/* Test that the SNI size calculation returns 0 on overflow instead of
+ * wrapping around to a small value (integer overflow vulnerability). */
+int test_TLSX_SNI_GetSize_overflow(void)
+{
+    EXPECT_DECLS;
+#if defined(HAVE_SNI) && !defined(NO_WOLFSSL_CLIENT) && !defined(NO_TLS)
+    WOLFSSL_CTX* ctx = NULL;
+    WOLFSSL* ssl = NULL;
+    TLSX* sni_ext = NULL;
+    SNI* head = NULL;
+    SNI* sni = NULL;
+    int i;
+    /* Each SNI adds ENUM_LEN(1) + OPAQUE16_LEN(2) + hostname_len to the size.
+     * With a 1-byte hostname, each entry adds 4 bytes. Starting from
+     * OPAQUE16_LEN(2) base, we need enough entries to exceed UINT16_MAX. */
+    const int num_sni = (0xFFFF / 4) + 2;
+
+    ExpectNotNull(ctx = wolfSSL_CTX_new(wolfSSLv23_client_method()));
+    ExpectNotNull(ssl = wolfSSL_new(ctx));
+
+    /* Add initial SNI via public API */
+    ExpectIntEQ(WOLFSSL_SUCCESS,
+                wolfSSL_UseSNI(ssl, WOLFSSL_SNI_HOST_NAME, "a", 1));
+
+    /* Find the SNI extension and manually build a long chain */
+    if (EXPECT_SUCCESS()) {
+        sni_ext = TLSX_Find(ssl->extensions, TLSX_SERVER_NAME);
+        ExpectNotNull(sni_ext);
+    }
+
+    if (EXPECT_SUCCESS()) {
+        head = (SNI*)sni_ext->data;
+        ExpectNotNull(head);
+    }
+
+    /* Append many SNI nodes to force overflow in the size calculation */
+    for (i = 1; EXPECT_SUCCESS() && i < num_sni; i++) {
+        sni = (SNI*)XMALLOC(sizeof(SNI), NULL, DYNAMIC_TYPE_TLSX);
+        ExpectNotNull(sni);
+        if (sni != NULL) {
+            XMEMSET(sni, 0, sizeof(SNI));
+            sni->type = WOLFSSL_SNI_HOST_NAME;
+            sni->data.host_name = (char*)XMALLOC(2, NULL, DYNAMIC_TYPE_TLSX);
+            ExpectNotNull(sni->data.host_name);
+            if (sni->data.host_name != NULL) {
+                sni->data.host_name[0] = 'a';
+                sni->data.host_name[1] = '\0';
+            }
+            sni->next = head->next;
+            head->next = sni;
+        }
+    }
+
+    if (EXPECT_SUCCESS()) {
+        /* The fixed calculation should return 0 (overflow detected) */
+        ExpectIntEQ(TLSX_SNI_GetSize((SNI*)sni_ext->data), 0);
+    }
+
+    wolfSSL_free(ssl);
+    wolfSSL_CTX_free(ctx);
+#endif
+    return EXPECT_RESULT();
+}

tests/api/test_tls_ext.h

@@ -27,5 +27,6 @@ int test_wolfSSL_DisableExtendedMasterSecret(void);
 int test_certificate_authorities_certificate_request(void);
 int test_certificate_authorities_client_hello(void);
 int test_TLSX_TCA_Find(void);
+int test_TLSX_SNI_GetSize_overflow(void);
 
 #endif /* TESTS_API_TEST_TLS_EMS_H */

wolfssl/internal.h

@@ -3169,7 +3169,10 @@ struct TLSX {
     struct TLSX* next; /* List Behavior   */
 };
 
-WOLFSSL_LOCAL TLSX* TLSX_Find(TLSX* list, TLSX_Type type);
+#ifdef WOLFSSL_API_PREFIX_MAP
+    #define TLSX_Find wolfSSL_TLSX_Find
+#endif
+WOLFSSL_TEST_VIS TLSX* TLSX_Find(TLSX* list, TLSX_Type type);
 WOLFSSL_LOCAL void  TLSX_Remove(TLSX** list, TLSX_Type type, void* heap);
 WOLFSSL_LOCAL void  TLSX_FreeAll(TLSX* list, void* heap);
 WOLFSSL_LOCAL int   TLSX_SupportExtensions(WOLFSSL* ssl);
@@ -3237,6 +3240,10 @@ WOLFSSL_LOCAL int TLSX_UseSNI(TLSX** extensions, byte type, const void* data,
 WOLFSSL_LOCAL byte TLSX_SNI_Status(TLSX* extensions, byte type);
 WOLFSSL_LOCAL word16 TLSX_SNI_GetRequest(TLSX* extensions, byte type,
                                                 void** data, byte ignoreStatus);
+#ifdef WOLFSSL_API_PREFIX_MAP
+    #define TLSX_SNI_GetSize wolfSSL_TLSX_SNI_GetSize
+#endif
+WOLFSSL_TEST_VIS word16 TLSX_SNI_GetSize(SNI* list);
 
 #ifndef NO_WOLFSSL_SERVER
 WOLFSSL_LOCAL void   TLSX_SNI_SetOptions(TLSX* extensions, byte type,