Merge pull request #6644 from julek-wolfssl/zd/16441

TLSX_CA_Names_Parse: Verify the length of the extension

Author: JacobBarthelmeh

src/tls.c

@@ -6634,6 +6634,9 @@ static int TLSX_CA_Names_Parse(WOLFSSL *ssl, const byte* input,
     if (ssl->client_ca_names == NULL)
         return MEMORY_ERROR;
 
+    if (length < OPAQUE16_LEN)
+        return BUFFER_ERROR;
+
     ato16(input, &extLen);
     input += OPAQUE16_LEN;
     length -= OPAQUE16_LEN;
@@ -6644,6 +6647,7 @@ static int TLSX_CA_Names_Parse(WOLFSSL *ssl, const byte* input,
         word32 idx = 0;
         WOLFSSL_X509_NAME* name = NULL;
         int ret = 0;
+        int didInit = FALSE;
         /* Use a DecodedCert struct to get access to GetName to
          * parse DN name */
 #ifdef WOLFSSL_SMALL_STACK
@@ -6655,28 +6659,33 @@ static int TLSX_CA_Names_Parse(WOLFSSL *ssl, const byte* input,
         DecodedCert cert[1];
 #endif
 
+        if (length < OPAQUE16_LEN)
+            return BUFFER_ERROR;
         ato16(input, &extLen);
         idx += OPAQUE16_LEN;
 
         if (extLen > length)
-            return BUFFER_ERROR;
-
-        InitDecodedCert(cert, input + idx, extLen, ssl->heap);
-        idx += extLen;
+            ret = BUFFER_ERROR;
 
-        ret = GetName(cert, SUBJECT, extLen);
+        if (ret == 0) {
+            InitDecodedCert(cert, input + idx, extLen, ssl->heap);
+            didInit = TRUE;
+            idx += extLen;
+            ret = GetName(cert, SUBJECT, extLen);
+        }
 
         if (ret == 0 && (name = wolfSSL_X509_NAME_new()) == NULL)
             ret = MEMORY_ERROR;
 
-        if (ret == 0)
+        if (ret == 0) {
             CopyDecodedName(name, cert, SUBJECT);
+            if (wolfSSL_sk_X509_NAME_push(ssl->client_ca_names, name)
+                    == WOLFSSL_FAILURE)
+                ret = MEMORY_ERROR;
+        }
 
-        if (ret == 0 && wolfSSL_sk_X509_NAME_push(ssl->client_ca_names, name)
-                == WOLFSSL_FAILURE)
-            ret = MEMORY_ERROR;
-
-        FreeDecodedCert(cert);
+        if (didInit)
+            FreeDecodedCert(cert);
 
 #ifdef WOLFSSL_SMALL_STACK
         XFREE(cert, ssl->heap, DYNAMIC_TYPE_DCERT);

tests/api.c

@@ -63086,6 +63086,55 @@ static int test_dtls_no_extensions(void)
     return EXPECT_RESULT();
 }
 
+static int test_TLSX_CA_NAMES_bad_extension(void)
+{
+    EXPECT_DECLS;
+#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_TLS13) && \
+    !defined(NO_CERTS) && !defined(WOLFSSL_NO_CA_NAMES) && \
+    defined(OPENSSL_EXTRA) && defined(WOLFSSL_SHA384) && \
+    defined(HAVE_NULL_CIPHER)
+    /* This test should only fail (with BUFFER_ERROR) when we actually try to
+     * parse the CA Names extension. Otherwise it will return other non-related
+     * errors. If CA Names will be parsed in more configurations, that should
+     * be reflected in the macro guard above. */
+    WOLFSSL *ssl_c = NULL;
+    WOLFSSL_CTX *ctx_c = NULL;
+    struct test_memio_ctx test_ctx;
+    /* HRR + SH using TLS_DHE_PSK_WITH_NULL_SHA384 */
+    const byte shBadCaNamesExt[] = {
+        0x16, 0x03, 0x04, 0x00, 0x3f, 0x02, 0x00, 0x00, 0x3b, 0x03, 0x03, 0xcf,
+        0x21, 0xad, 0x74, 0xe5, 0x9a, 0x61, 0x11, 0xbe, 0x1d, 0x8c, 0x02, 0x1e,
+        0x65, 0xb8, 0x91, 0xc2, 0xa2, 0x11, 0x16, 0x7a, 0xbb, 0x8c, 0x5e, 0x07,
+        0x9e, 0x09, 0xe2, 0xc8, 0xa8, 0x33, 0x9c, 0x00, 0x13, 0x03, 0x00, 0x00,
+        0x13, 0x94, 0x7e, 0x00, 0x03, 0x0b, 0xf7, 0x03, 0x00, 0x2b, 0x00, 0x02,
+        0x03, 0x04, 0x00, 0x33, 0x00, 0x02, 0x00, 0x19, 0x16, 0x03, 0x03, 0x00,
+        0x5c, 0x02, 0x00, 0x00, 0x3b, 0x03, 0x03, 0x03, 0xcf, 0x21, 0xad, 0x74,
+        0x00, 0x00, 0x83, 0x3f, 0x3b, 0x80, 0x01, 0xac, 0x65, 0x8c, 0x19, 0x2a,
+        0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x02, 0x00, 0x9e, 0x09, 0x1c, 0xe8,
+        0xa8, 0x09, 0x9c, 0x00, 0xc0, 0xb5, 0x00, 0x00, 0x11, 0x8f, 0x00, 0x00,
+        0x03, 0x3f, 0x00, 0x0c, 0x00, 0x2b, 0x00, 0x02, 0x03, 0x04, 0x13, 0x05,
+        0x00, 0x00, 0x08, 0x00, 0x00, 0x06, 0x00, 0x04, 0x00, 0x09, 0x00, 0x00,
+        0x0d, 0x00, 0x00, 0x11, 0x00, 0x00, 0x0d, 0x00, 0x2f, 0x00, 0x01, 0xff,
+        0xff, 0xff, 0xff, 0xfa, 0x0d, 0x00, 0x00, 0x00, 0xad, 0x02
+    };
+
+    XMEMSET(&test_ctx, 0, sizeof(test_ctx));
+
+    ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, NULL, &ssl_c, NULL,
+        wolfTLSv1_3_client_method, NULL), 0);
+
+    XMEMCPY(test_ctx.c_buff, shBadCaNamesExt, sizeof(shBadCaNamesExt));
+    test_ctx.c_len = sizeof(shBadCaNamesExt);
+
+    ExpectIntEQ(wolfSSL_connect(ssl_c), -1);
+    ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), BUFFER_ERROR);
+
+    wolfSSL_free(ssl_c);
+    wolfSSL_CTX_free(ctx_c);
+#endif
+    return EXPECT_RESULT();
+}
+
 /*----------------------------------------------------------------------------*
  | Main
  *----------------------------------------------------------------------------*/
@@ -64337,6 +64386,7 @@ TEST_CASE testCases[] = {
     TEST_DECL(test_dtls_ipv6_check),
     TEST_DECL(test_wolfSSL_SCR_after_resumption),
     TEST_DECL(test_dtls_no_extensions),
+    TEST_DECL(test_TLSX_CA_NAMES_bad_extension),
     /* This test needs to stay at the end to clean up any caches allocated. */
     TEST_DECL(test_wolfSSL_Cleanup)
 };