Free old peerSigSpec pointer before allocating. Add length checks for DSA Q buffer in EVP layer.

src/tls.c

@@ -12135,6 +12135,13 @@ int TLSX_CKS_Parse(WOLFSSL* ssl, byte* input, word16 length,
         return 0;
     }
 
+    /* sigSpec may point at the previous copy. */
+    if (ssl->sigSpec == ssl->peerSigSpec) {
+        ssl->sigSpec = NULL;
+        ssl->sigSpecSz = 0;
+    }
+    XFREE(ssl->peerSigSpec, ssl->heap, DYNAMIC_TYPE_TLSX);
+
     /* Copy as the lifetime of input seems to be ephemeral. */
     ssl->peerSigSpec = (byte*)XMALLOC(length, ssl->heap, DYNAMIC_TYPE_TLSX);
     if (ssl->peerSigSpec == NULL) {

tests/api/test_evp_pkey.c

@@ -2057,6 +2057,59 @@ int test_wolfSSL_EVP_PKEY_sign_verify_ec(void)
     return EXPECT_RESULT();
 }
 
+/* EVP_PKEY_verify() with a DSA key must only use the signature and digest
+ * lengths it is given. */
+int test_wolfSSL_EVP_PKEY_verify_dsa_len(void)
+{
+    EXPECT_DECLS;
+#if defined(OPENSSL_EXTRA) && !defined(NO_DSA) && !defined(HAVE_SELFTEST) && \
+    !defined(WC_FIPS_186_5_PLUS) && defined(USE_CERT_BUFFERS_2048)
+    DSA* dsa = NULL;
+    EVP_PKEY* pkey = NULL;
+    EVP_PKEY_CTX* ctx = NULL;
+    byte hash[WC_SHA_DIGEST_SIZE];
+    byte sig[DSA_MAX_SIG_SIZE];
+    size_t sigLen = sizeof(sig);
+    byte* shortSig = NULL;
+
+    XMEMSET(hash, 0x5a, sizeof(hash));
+    ExpectNotNull(dsa = DSA_new());
+    ExpectIntEQ(DSA_LoadDer(dsa, dsa_key_der_2048,
+        (int)sizeof_dsa_key_der_2048), 1);
+    ExpectNotNull(pkey = EVP_PKEY_new());
+    ExpectIntEQ(EVP_PKEY_assign_DSA(pkey, dsa), WOLFSSL_SUCCESS);
+    if (EXPECT_FAIL()) {
+        DSA_free(dsa);
+    }
+    ExpectNotNull(ctx = EVP_PKEY_CTX_new(pkey, NULL));
+    ExpectIntEQ(EVP_PKEY_sign_init(ctx), WOLFSSL_SUCCESS);
+    ExpectIntEQ(EVP_PKEY_sign(ctx, sig, &sigLen, hash, sizeof(hash)),
+        WOLFSSL_SUCCESS);
+    ExpectIntEQ(EVP_PKEY_verify_init(ctx), WOLFSSL_SUCCESS);
+    ExpectIntEQ(EVP_PKEY_verify(ctx, sig, sigLen, hash, sizeof(hash)),
+        WOLFSSL_SUCCESS);
+
+    ExpectNotNull(shortSig = (byte*)XMALLOC(sigLen - 1, HEAP_HINT,
+        DYNAMIC_TYPE_TMP_BUFFER));
+    if (shortSig != NULL)
+        XMEMCPY(shortSig, sig, sigLen - 1);
+    ExpectIntEQ(EVP_PKEY_verify(ctx, shortSig, sigLen - 1, hash,
+        sizeof(hash)), WC_NO_ERR_TRACE(WOLFSSL_FAILURE));
+    XFREE(shortSig, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
+
+    /* A valid signature and digest do not verify when the lengths given
+     * leave part of them out. */
+    ExpectIntEQ(EVP_PKEY_verify(ctx, sig, sigLen - 1, hash, sizeof(hash)),
+        WC_NO_ERR_TRACE(WOLFSSL_FAILURE));
+    ExpectIntEQ(EVP_PKEY_verify(ctx, sig, sigLen, hash, sizeof(hash) - 1),
+        WC_NO_ERR_TRACE(WOLFSSL_FAILURE));
+
+    EVP_PKEY_CTX_free(ctx);
+    EVP_PKEY_free(pkey);
+#endif
+    return EXPECT_RESULT();
+}
+
 
 int test_wolfSSL_EVP_MD_rsa_signing(void)
 {

tests/api/test_evp_pkey.h

@@ -60,6 +60,7 @@ int test_wolfSSL_EVP_SignInit_ex(void);
 int test_wolfSSL_EVP_PKEY_sign_verify_rsa(void);
 int test_wolfSSL_EVP_PKEY_sign_verify_dsa(void);
 int test_wolfSSL_EVP_PKEY_sign_verify_ec(void);
+int test_wolfSSL_EVP_PKEY_verify_dsa_len(void);
 int test_wolfSSL_EVP_MD_rsa_signing(void);
 int test_wc_RsaPSS_DigitalSignVerify(void);
 int test_wolfSSL_EVP_MD_ecc_signing(void);
@@ -117,6 +118,7 @@ int test_wolfSSL_CTX_use_PrivateKey_pkcs8_repopulate(void);
     TEST_DECL_GROUP("evp_pkey", test_wolfSSL_EVP_PKEY_sign_verify_rsa),        \
     TEST_DECL_GROUP("evp_pkey", test_wolfSSL_EVP_PKEY_sign_verify_dsa),        \
     TEST_DECL_GROUP("evp_pkey", test_wolfSSL_EVP_PKEY_sign_verify_ec),         \
+    TEST_DECL_GROUP("evp_pkey", test_wolfSSL_EVP_PKEY_verify_dsa_len),         \
     TEST_DECL_GROUP("evp_pkey", test_wolfSSL_EVP_MD_rsa_signing),              \
     TEST_DECL_GROUP("evp_pkey", test_wc_RsaPSS_DigitalSignVerify),             \
     TEST_DECL_GROUP("evp_pkey", test_wolfSSL_EVP_MD_ecc_signing),              \

tests/api/test_tls13.c

@@ -13979,3 +13979,102 @@ int test_tls13_psk_key_zeroized(void)
 #endif
     return EXPECT_RESULT();
 }
+
+#if defined(WOLFSSL_TLS13) && defined(WOLFSSL_DUAL_ALG_CERTS) && \
+    defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(HAVE_ECC) && \
+    defined(USE_CERT_BUFFERS_256) && defined(USE_WOLFSSL_MEMORY) && \
+    !defined(WOLFSSL_NO_MALLOC) && !defined(WOLFSSL_STATIC_MEMORY) && \
+    !defined(WOLFSSL_DEBUG_MEMORY)
+#define TEST_TLS13_CKS_REPARSE
+
+static wolfSSL_Free_cb test_tls13_cks_prev_free;
+static void*           test_tls13_cks_watch;
+static byte            test_tls13_cks_freed;
+
+static void test_tls13_cks_free_cb(void* ptr)
+{
+    if (ptr != NULL && ptr == test_tls13_cks_watch) {
+        /* Held until the test ends so the address is not handed out again. */
+        test_tls13_cks_freed++;
+        return;
+    }
+    if (test_tls13_cks_prev_free != NULL)
+        test_tls13_cks_prev_free(ptr);
+    else
+        free(ptr);
+}
+#endif
+
+/* The server copies the peer's CKS list from each ClientHello. After a
+ * HelloRetryRequest the copy taken from the first ClientHello has to be
+ * released when the second one replaces it. */
+int test_tls13_cks_hrr_reparse(void)
+{
+    EXPECT_DECLS;
+#if defined(TEST_TLS13_CKS_REPARSE) && defined(USE_WOLFSSL_MEMORY)
+    WOLFSSL_CTX *ctx_c = NULL;
+    WOLFSSL_CTX *ctx_s = NULL;
+    WOLFSSL *ssl_c = NULL;
+    WOLFSSL *ssl_s = NULL;
+    struct test_memio_ctx test_ctx;
+    byte cks[1] = { WOLFSSL_CKS_SIGSPEC_NATIVE };
+    wolfSSL_Malloc_cb  prev_mc = NULL;
+    wolfSSL_Realloc_cb prev_rc = NULL;
+    byte hooked = 0;
+
+    test_tls13_cks_watch = NULL;
+    test_tls13_cks_freed = 0;
+
+    XMEMSET(&test_ctx, 0, sizeof(test_ctx));
+    ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s,
+        wolfTLSv1_3_client_method, wolfTLSv1_3_server_method), 0);
+    ExpectIntEQ(wolfSSL_use_AltPrivateKey_buffer(ssl_s, ecc_key_der_256,
+        sizeof_ecc_key_der_256, WOLFSSL_FILETYPE_ASN1), WOLFSSL_SUCCESS);
+    ExpectIntEQ(wolfSSL_UseCKS(ssl_c, cks, (word16)sizeof(cks)),
+        WOLFSSL_SUCCESS);
+    /* No key share entries, so the server has to ask for one. */
+    ExpectIntEQ(wolfSSL_NoKeyShares(ssl_c), WOLFSSL_SUCCESS);
+
+    ExpectIntNE(wolfSSL_connect(ssl_c), WOLFSSL_SUCCESS);
+    ExpectIntEQ(wolfSSL_get_error(ssl_c,
+        WC_NO_ERR_TRACE(WOLFSSL_FATAL_ERROR)), WOLFSSL_ERROR_WANT_READ);
+    ExpectIntNE(wolfSSL_accept(ssl_s), WOLFSSL_SUCCESS);
+    ExpectIntEQ(wolfSSL_get_error(ssl_s,
+        WC_NO_ERR_TRACE(WOLFSSL_FATAL_ERROR)), WOLFSSL_ERROR_WANT_READ);
+    ExpectIntEQ(ssl_s->options.serverState,
+        SERVER_HELLO_RETRY_REQUEST_COMPLETE);
+    ExpectNotNull(test_tls13_cks_watch = ssl_s->peerSigSpec);
+
+    ExpectIntEQ(wolfSSL_GetAllocators(&prev_mc, &test_tls13_cks_prev_free,
+        &prev_rc), 0);
+    ExpectIntEQ(wolfSSL_SetAllocators(prev_mc, test_tls13_cks_free_cb,
+        prev_rc), 0);
+    if (EXPECT_SUCCESS())
+        hooked = 1;
+
+    ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0);
+
+    if (hooked)
+        (void)wolfSSL_SetAllocators(prev_mc, test_tls13_cks_prev_free,
+            prev_rc);
+
+    ExpectNotNull(ssl_c->peerSigSpec);
+    ExpectIntEQ(test_tls13_cks_freed, 1);
+    ExpectNotNull(ssl_s->peerSigSpec);
+    ExpectPtrNE(ssl_s->peerSigSpec, test_tls13_cks_watch);
+    ExpectIntEQ(ssl_s->peerSigSpecSz, sizeof(cks));
+    /* With no preference of its own the server follows the second list. */
+    ExpectPtrEq(ssl_s->sigSpec, ssl_s->peerSigSpec);
+
+    if (test_tls13_cks_freed > 0 && ssl_s != NULL &&
+            ssl_s->peerSigSpec != test_tls13_cks_watch)
+        XFREE(test_tls13_cks_watch, NULL, DYNAMIC_TYPE_TLSX);
+    test_tls13_cks_watch = NULL;
+    test_tls13_cks_prev_free = NULL;
+    wolfSSL_free(ssl_c);
+    wolfSSL_free(ssl_s);
+    wolfSSL_CTX_free(ctx_c);
+    wolfSSL_CTX_free(ctx_s);
+#endif
+    return EXPECT_RESULT();
+}

tests/api/test_tls13.h

@@ -162,6 +162,7 @@ int test_tls13_hs_secret_zeroized_psk_ke(void);
 int test_tls13_hs_secret_zeroized_sha384(void);
 int test_tls13_early_secret_zeroized(void);
 int test_tls13_psk_key_zeroized(void);
+int test_tls13_cks_hrr_reparse(void);
 
 #define TEST_TLS13_DECLS                                        \
     TEST_DECL_GROUP("tls13", test_tls13_apis),                  \
@@ -302,6 +303,7 @@ int test_tls13_psk_key_zeroized(void);
     TEST_DECL_GROUP("tls13", test_tls13_hs_secret_zeroized_psk_ke), \
     TEST_DECL_GROUP("tls13", test_tls13_hs_secret_zeroized_sha384), \
     TEST_DECL_GROUP("tls13", test_tls13_early_secret_zeroized), \
-    TEST_DECL_GROUP("tls13", test_tls13_psk_key_zeroized)
+    TEST_DECL_GROUP("tls13", test_tls13_psk_key_zeroized), \
+    TEST_DECL_GROUP("tls13", test_tls13_cks_hrr_reparse)
 
 #endif /* WOLFCRYPT_TEST_TLS13_H */

wolfcrypt/src/evp.c

@@ -3613,6 +3613,12 @@ int wolfSSL_EVP_PKEY_verify(WOLFSSL_EVP_PKEY_CTX *ctx, const unsigned char *sig,
 #ifndef NO_DSA
      case WC_EVP_PKEY_DSA: {
         int dsacheck = 0;
+        int bytes;
+        if (!ctx->pkey->dsa || tbslen < WC_SHA_DIGEST_SIZE)
+            return WOLFSSL_FAILURE;
+        bytes = wolfSSL_BN_num_bytes(ctx->pkey->dsa->q);
+        if (bytes <= 0 || siglen < (size_t)bytes * 2)
+            return WOLFSSL_FAILURE;
         if (wolfSSL_DSA_do_verify(tbs, (unsigned char *)sig, ctx->pkey->dsa,
             &dsacheck) != WOLFSSL_SUCCESS || dsacheck != 1)
             return WOLFSSL_FAILURE;