Bound DH agree output and fix FFDHE size guard

doc/dox_comments/header_files/dh.h

@@ -103,6 +103,10 @@ int wc_DhGenerateKeyPair(DhKey* key, WC_RNG* rng, byte* priv,
     communication. On successfully generating a shared secret key, the size of
     the secret key written will be stored in agreeSz.
 
+    The value of agreeSz on input is not checked. The agree buffer must be at
+    least as large as the prime, in bytes. Use wc_DhAgree_ct() to have the
+    buffer size checked.
+
     \return 0 Returned on successfully generating an agreed upon secret key
     \return MP_INIT_E May be returned if there is an error while generating
     the shared secret key
@@ -141,6 +145,7 @@ int wc_DhGenerateKeyPair(DhKey* key, WC_RNG* rng, byte* priv,
     \endcode
 
     \sa wc_DhGenerateKeyPair
+    \sa wc_DhAgree_ct
 */
 int wc_DhAgree(DhKey* key, byte* agree, word32* agreeSz,
                        const byte* priv, word32 privSz, const byte* otherPub,

src/internal.c

@@ -7833,10 +7833,20 @@ int DhAgree(WOLFSSL* ssl, DhKey* dhKey,
         else
 #endif
         {
-            PRIVATE_KEY_UNLOCK();
-            ret = wc_DhAgree(dhKey, agree, agreeSz, priv, privSz, otherPub,
-                    otherPubSz);
-            PRIVATE_KEY_LOCK();
+            /* wc_DhAgree() treats agreeSz as output only and writes up to the
+             * prime size, so bound the buffer here. */
+            if (dhKey != NULL && agreeSz != NULL &&
+                    *agreeSz < (word32)mp_unsigned_bin_size(&dhKey->p)) {
+                WOLFSSL_MSG("DH agree buffer smaller than prime");
+                ret = BUFFER_E;
+                WOLFSSL_ERROR_VERBOSE(ret);
+            }
+            else {
+                PRIVATE_KEY_UNLOCK();
+                ret = wc_DhAgree(dhKey, agree, agreeSz, priv, privSz, otherPub,
+                        otherPubSz);
+                PRIVATE_KEY_LOCK();
+            }
         }
     }
 

tests/api/test_tls13_bounds.c

@@ -927,3 +927,44 @@ int test_tls13_mutual_auth_rsa(void)
     return TEST_SKIPPED;
 #endif
 }
+
+/* A pre-master secret buffer shorter than the FFDHE prime must fail the
+ * handshake with BUFFER_E rather than let wc_DhAgree() write past it. */
+int test_tls13_ffdhe_pms_too_small(void)
+{
+    EXPECT_DECLS;
+#if defined(WOLFSSL_TLS13) && !defined(NO_DH) && defined(HAVE_FFDHE_2048) && \
+    defined(HAVE_SUPPORTED_CURVES) && \
+    defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \
+    !defined(NO_WOLFSSL_CLIENT) && !defined(NO_WOLFSSL_SERVER) && \
+    !defined(NO_CERTS) && !defined(NO_FILESYSTEM)
+    WOLFSSL_CTX* ctx_c = NULL;
+    WOLFSSL_CTX* ctx_s = NULL;
+    WOLFSSL* ssl_c = NULL;
+    WOLFSSL* ssl_s = NULL;
+    struct test_memio_ctx test_ctx;
+    int groups[1] = { WOLFSSL_FFDHE_2048 };
+
+    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_set_groups(ssl_c, groups, 1), WOLFSSL_SUCCESS);
+    ExpectIntEQ(wolfSSL_set_groups(ssl_s, groups, 1), WOLFSSL_SUCCESS);
+    ExpectIntEQ(wolfSSL_UseKeyShare(ssl_c, WOLFSSL_FFDHE_2048),
+        WOLFSSL_SUCCESS);
+
+    /* One byte short of the 2048-bit prime. */
+    if (ssl_s != NULL && ssl_s->arrays != NULL)
+        ssl_s->arrays->preMasterSz = (2048 / 8) - 1;
+
+    ExpectIntNE(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0);
+    ExpectIntEQ(wolfSSL_get_error(ssl_s, WOLFSSL_FATAL_ERROR),
+        WC_NO_ERR_TRACE(BUFFER_E));
+
+    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_bounds.h

@@ -39,6 +39,7 @@ int test_tls13_mutual_auth_ecdsa(void);
 int test_tls13_mutual_auth_ed25519(void);
 int test_tls13_mutual_auth_ed448(void);
 int test_tls13_mutual_auth_rsa(void);
+int test_tls13_ffdhe_pms_too_small(void);
 
 #define TEST_TLS13_BOUNDS_DECLS                                             \
     TEST_DECL_GROUP("tls13", test_tls13_ch_legacy_version_is_tls13),        \
@@ -55,6 +56,7 @@ int test_tls13_mutual_auth_rsa(void);
     TEST_DECL_GROUP("tls13", test_tls13_mutual_auth_ecdsa),                 \
     TEST_DECL_GROUP("tls13", test_tls13_mutual_auth_ed25519),               \
     TEST_DECL_GROUP("tls13", test_tls13_mutual_auth_ed448),                 \
-    TEST_DECL_GROUP("tls13", test_tls13_mutual_auth_rsa)
+    TEST_DECL_GROUP("tls13", test_tls13_mutual_auth_rsa),                   \
+    TEST_DECL_GROUP("tls13", test_tls13_ffdhe_pms_too_small)
 
 #endif /* WOLFCRYPT_TEST_TLS13_BOUNDS_H */

wolfssl/wolfcrypt/settings.h

@@ -4051,6 +4051,7 @@
     #define MIN_FFDHE_BITS 0
 #endif
 #define MIN_FFDHE_FP_MAX_BITS   (MIN_FFDHE_BITS * 2)
+/* Only explicit sizes are checked here; tfm.h and sp_int.h check defaults. */
 #if defined(HAVE_FFDHE) && defined(FP_MAX_BITS)
     #if MIN_FFDHE_FP_MAX_BITS > FP_MAX_BITS
         #error "FFDHE parameters are too large for FP_MAX_BIT as set"

wolfssl/wolfcrypt/sp_int.h

@@ -532,6 +532,11 @@ typedef struct sp_dh_ctx {
         #endif
     #endif
 #endif
+/* settings.h cannot check a default chosen here. */
+#if defined(HAVE_FFDHE) && defined(MIN_FFDHE_BITS) && \
+    (MIN_FFDHE_BITS > SP_INT_BITS)
+    #error "FFDHE parameters are too large for SP_INT_BITS"
+#endif
 
 #ifndef SP_INT_DIGITS
     /* Calculate number of digits to have in an sp_int based on maximum size of

wolfssl/wolfcrypt/tfm.h

@@ -278,6 +278,11 @@
     #undef FP_MAX_BITS
     #define FP_MAX_BITS 16384
 #endif
+/* settings.h cannot check a default chosen here. */
+#if defined(HAVE_FFDHE) && defined(MIN_FFDHE_FP_MAX_BITS) && \
+    (MIN_FFDHE_FP_MAX_BITS > FP_MAX_BITS)
+    #error "FFDHE parameters are too large for FP_MAX_BITS"
+#endif
 #define FP_MAX_SIZE           (FP_MAX_BITS+(8*DIGIT_BIT))
 
 /* will this lib work? */