Merge pull request #10478 from embhorn/zd21821

Fixes in SP int and DH

Author: SparkiDev

wolfcrypt/src/dh.c

@@ -1122,13 +1122,24 @@ static int GeneratePrivateDh186(DhKey* key, WC_RNG* rng, byte* priv,
     byte cBuf[DH_MAX_SIZE + 64 / WOLFSSL_BIT_SIZE];
 #endif
 
-    /* Parameters validated in calling functions. */
+    /* Pointer parameters validated by the public entry wc_DhGenerateKeyPair. */
 
     if (mp_iszero(&key->q) == MP_YES) {
         WOLFSSL_MSG("DH q parameter needed for FIPS 186-4 key generation");
         return BAD_FUNC_ARG;
     }
 
+    /* Bound *privSz so cSz (= *privSz + 8) cannot exceed the cBuf capacity.
+     * Note: DH_MAX_SIZE is documented as a bit count, but the cBuf declaration
+     * above uses it directly as a byte count (cBuf is DH_MAX_SIZE + 8 bytes).
+     * This check matches that convention so *privSz (in bytes) is bounded by
+     * the actual byte capacity of cBuf. The same bound is applied to the
+     * WOLFSSL_SMALL_STACK path to avoid unbounded heap allocation. */
+    if (*privSz > DH_MAX_SIZE) {
+        WOLFSSL_MSG("DH private key size exceeds DH_MAX_SIZE");
+        return BAD_FUNC_ARG;
+    }
+
     qSz = (word32)mp_unsigned_bin_size(&key->q);
     pSz = (word32)mp_unsigned_bin_size(&key->p);
 

wolfcrypt/src/sp_int.c

@@ -6140,8 +6140,10 @@ int sp_leading_bit(const sp_int* a)
 int sp_set_bit(sp_int* a, int i)
 {
     int err = MP_OKAY;
-    /* Get index of word to set. */
-    sp_size_t w = (sp_size_t)(i >> SP_WORD_SHIFT);
+    /* Compute word index in full int width so that bit indices large enough
+     * to make the word index overflow sp_size_t are caught by the bounds
+     * check below rather than wrapping. */
+    int wi = (i < 0) ? 0 : (i >> SP_WORD_SHIFT);
 
 #if SP_INT_DIGITS < (65536 / SP_WORD_SIZEOF)
     /* Check bit index isn't bigger than maximum allowed. */
@@ -6151,10 +6153,11 @@ int sp_set_bit(sp_int* a, int i)
     else
 #endif
     /* Check for valid number and space for bit. */
-    if ((a == NULL) || (i < 0) || (w >= a->size)) {
+    if ((a == NULL) || (i < 0) || (wi >= (int)a->size)) {
         err = MP_VAL;
     }
     if (err == MP_OKAY) {
+        sp_size_t w = (sp_size_t)wi;
         /* Amount to shift up to set bit in word. */
         unsigned int s = (unsigned int)(i & (SP_WORD_SIZE - 1));
         unsigned int j;
@@ -8627,15 +8630,16 @@ void sp_rshd(sp_int* a, int c)
 {
     /* Do shift if we have an SP int. */
     if ((a != NULL) && (c > 0)) {
-        /* Make zero if shift removes all digits. */
-        if ((sp_size_t)c >= a->used) {
+        /* Compare c in int width to avoid narrowing to sp_size_t (which can
+         * be word16) before the bounds check. */
+        if (c >= (int)a->used) {
             _sp_zero(a);
         }
         else {
             sp_size_t i;
 
             /* Update used digits count. */
-            a->used = (sp_size_t)(a->used - c);
+            a->used = (sp_size_t)((int)a->used - c);
             /* Move digits down. */
             for (i = 0; i < a->used; i++, c++) {
                 a->dp[i] = a->dp[c];
@@ -8657,21 +8661,23 @@ void sp_rshd(sp_int* a, int c)
 int sp_rshb(const sp_int* a, int n, sp_int* r)
 {
     int err = MP_OKAY;
-    /* Number of digits to shift down. */
-    sp_size_t i;
+    /* Compute the digit-shift count in full int width to avoid wrapping
+     * when n is large enough that the count would exceed sp_size_t range. */
+    int ni = (n < 0) ? 0 : (n >> SP_WORD_SHIFT);
 
     if ((a == NULL) || (n < 0)) {
         err = MP_VAL;
     }
     /* Handle case where shifting out all digits. */
-    else if ((i = (sp_size_t)(n >> SP_WORD_SHIFT)) >= a->used) {
+    else if (ni >= (int)a->used) {
         _sp_zero(r);
     }
     /* Change callers when more error cases returned. */
-    else if ((err == MP_OKAY) && (a->used - i > r->size)) {
+    else if ((err == MP_OKAY) && ((int)a->used - ni > (int)r->size)) {
         err = MP_VAL;
     }
     else if (err == MP_OKAY) {
+        sp_size_t i = (sp_size_t)ni;
         sp_size_t j;
 
         /* Number of bits to shift in digits. */
@@ -14920,16 +14926,25 @@ int sp_div_2d(const sp_int* a, int e, sp_int* r, sp_int* rem)
 int sp_mod_2d(const sp_int* a, int e, sp_int* r)
 {
     int err = MP_OKAY;
-    sp_size_t digits = (sp_size_t)((e + SP_WORD_SIZE - 1) >> SP_WORD_SHIFT);
+    /* Compute digit count in full int width. Decompose to avoid signed
+     * overflow if e is near INT_MAX: (e + SP_WORD_SIZE - 1) >> SHIFT is
+     * equivalent to (e >> SHIFT) + (e has remainder ? 1 : 0). */
+    int digits_full = 0;
+    sp_size_t digits = 0;
 
     if ((a == NULL) || (r == NULL) || (e < 0)) {
         err = MP_VAL;
     }
-    if ((err == MP_OKAY) && (digits > r->size)) {
-        err = MP_VAL;
+    if (err == MP_OKAY) {
+        digits_full = (e >> SP_WORD_SHIFT) +
+                      (((e & (SP_WORD_SIZE - 1)) != 0) ? 1 : 0);
+        if (digits_full > (int)r->size) {
+            err = MP_VAL;
+        }
     }
 
     if (err == MP_OKAY) {
+        digits = (sp_size_t)digits_full;
         /* Copy a into r if not same pointer. */
         if (a != r) {
             sp_size_t cnt = (a->used < digits) ? a->used : digits;

wolfcrypt/test/test.c

@@ -29702,6 +29702,27 @@ static wc_test_ret_t dh_fips_generate_test(WC_RNG *rng)
     if (ret != 0)
         ERROR_OUT(WC_TEST_RET_ENC_EC(ret), exit_gen_test);
 
+#if !defined(WOLFSSL_NO_DH186) && !defined(HAVE_SELFTEST) && \
+    !defined(HAVE_FIPS)
+    /* Regression: an oversized *privSz must be rejected before
+     * GeneratePrivateDh186 writes (*privSz + 8) bytes of RNG output into the
+     * stack-allocated cBuf (sized DH_MAX_SIZE + 8 in non-WOLFSSL_SMALL_STACK
+     * builds). The key still has q set here, so the call dispatches through
+     * GeneratePrivateDh186. Only exercised when the local src/dh.c bound
+     * check is in play (not HAVE_SELFTEST / HAVE_FIPS builds, which use
+     * separate validated modules). */
+    {
+        word32 hugePrivSz = (word32)DH_MAX_SIZE + 1;
+        word32 outPubSz   = sizeof(pub);
+        ret = wc_DhGenerateKeyPair(key, rng, priv, &hugePrivSz, pub, &outPubSz);
+    #if defined(WOLFSSL_ASYNC_CRYPT)
+        ret = wc_AsyncWait(ret, &key->asyncDev, WC_ASYNC_FLAG_NONE);
+    #endif
+        if (ret != WC_NO_ERR_TRACE(BAD_FUNC_ARG))
+            ERROR_OUT(WC_TEST_RET_ENC_EC(ret), exit_gen_test);
+    }
+#endif /* !WOLFSSL_NO_DH186 && !HAVE_SELFTEST && !HAVE_FIPS */
+
     wc_FreeDhKey(key);
     ret = wc_InitDhKey_ex(key, HEAP_HINT, devId);
     if (ret != 0)