Merge pull request #10096 from padelsbach/dilithium-oob-shift

douzzer · web-flow · commit d6fa8466153c · 2026-04-02T00:13:06.000-05:00
Fix out of bounds shift in ML-DSA
diff --git a/.github/workflows/pq-all.yml b/.github/workflows/pq-all.yml
@@ -18,6 +18,7 @@ jobs:
       matrix:
         config: [
           # Add new configs here
+          '--disable-shared --enable-dilithium --enable-mlkem CFLAGS="-fsanitize=undefined -fno-sanitize-recover=undefined -fno-omit-frame-pointer" LDFLAGS="-fsanitize=undefined" CPPFLAGS="-DWOLFSSL_DILITHIUM_ALIGNMENT=4"',
           '--enable-intelasm --enable-sp-asm --enable-mlkem=yes,kyber,ml-kem CPPFLAGS="-DWOLFSSL_ML_KEM_USE_OLD_IDS"',
           '--enable-intelasm --enable-sp-asm --enable-all --enable-testcert --enable-acert --enable-dtls13 --enable-dtls-mtu --enable-dtls-frag-ch --enable-dtlscid --enable-quic --with-sys-crypto-policy --enable-experimental --enable-mlkem=yes,kyber,ml-kem --enable-lms --enable-xmss --enable-slhdsa --enable-dilithium --enable-dual-alg-certs --disable-qt CPPFLAGS="-pedantic -Wdeclaration-after-statement -DWOLFCRYPT_TEST_LINT -DNO_WOLFSSL_CIPHER_SUITE_TEST -DTEST_LIBWOLFSSL_SOURCES_INCLUSION_SEQUENCE"',
           '--enable-smallstack --enable-smallstackcache --enable-intelasm --enable-sp-asm --enable-all --enable-testcert --enable-acert --enable-dtls13 --enable-dtls-mtu --enable-dtls-frag-ch --enable-dtlscid --enable-quic --with-sys-crypto-policy --enable-experimental --enable-mlkem=yes,kyber,ml-kem --enable-lms --enable-xmss --enable-slhdsa --enable-dilithium --enable-dual-alg-certs --disable-qt CPPFLAGS="-pedantic -Wdeclaration-after-statement -DWOLFCRYPT_TEST_LINT -DNO_WOLFSSL_CIPHER_SUITE_TEST -DTEST_LIBWOLFSSL_SOURCES_INCLUSION_SEQUENCE"',
diff --git a/tests/api/test_mldsa.c b/tests/api/test_mldsa.c
@@ -24781,6 +24781,135 @@ int test_mldsa_pkcs8_export_import_wolfSSL_form(void)
     return EXPECT_RESULT();
 }
 
+/* Exercise w1 encoding with adversarial inputs to catch undefined behavior.
+ *
+ * The word32-optimized paths in dilithium_encode_w1_88_c and
+ * dilithium_encode_w1_32_c left-shift sword32 values by up to 30 bits.
+ * Large or invalid w1 values trigger signed integer overflow (UB in C).
+ * Under -fsanitize=undefined this test will trap on the offending shifts.
+ *
+ * We also test that encoding the same input twice yields identical output
+ * (determinism check - UB can cause nondeterministic results).
+ */
+int test_wc_dilithium_encode_w1_large_values(void)
+{
+    EXPECT_DECLS;
+#if defined(HAVE_DILITHIUM) && defined(WOLFSSL_WC_DILITHIUM) && \
+    (!defined(WOLFSSL_DILITHIUM_NO_SIGN) || \
+     !defined(WOLFSSL_DILITHIUM_NO_VERIFY))
+
+    sword32 w1[DILITHIUM_N];
+    unsigned int j, k;
+
+    /* Adversarial w1 values:
+     *   - valid boundary values (0, 1, max-for-bit-width)
+     *   - oversize values that exceed the expected bit width
+     *   - negative values (should never appear but might if prior step is buggy)
+     *   - extreme values near INT32 limits
+     *   - alternating patterns that stress cross-element packing
+     */
+    const sword32 patterns[] = {
+        0,                  /* trivial zero */
+        1,                  /* minimal nonzero */
+        0x7F,               /* 7 bits set - wider than both 4-bit and 6-bit */
+        0xFF,               /* full byte */
+        0xFFFF,             /* 16 bits */
+        0x7FFFFFFF,         /* INT32_MAX */
+        -1,                 /* all bits set (negative) */
+        -128,               /* negative */
+        (sword32)0x80000000 /* INT32_MIN */
+    };
+    const int n_patterns = (int)(sizeof(patterns) / sizeof(patterns[0]));
+
+    /* ---- 6-bit encoding (dilithium_encode_w1_88 path) ---- */
+#ifndef WOLFSSL_NO_ML_DSA_44
+    {
+        ALIGN32 byte enc_a[DILITHIUM_N * DILITHIUM_Q_HI_88_ENC_BITS / 8];
+        ALIGN32 byte enc_b[DILITHIUM_N * DILITHIUM_Q_HI_88_ENC_BITS / 8];
+
+        /* Uniform fill with each pattern */
+        for (k = 0; k < (unsigned int)n_patterns; k++) {
+            for (j = 0; j < DILITHIUM_N; j++) {
+                w1[j] = patterns[k];
+            }
+
+            XMEMSET(enc_a, 0, sizeof(enc_a));
+            XMEMSET(enc_b, 0, sizeof(enc_b));
+            wc_dilithium_encode_w1_88(w1, enc_a);
+            wc_dilithium_encode_w1_88(w1, enc_b);
+
+            /* Determinism: same input must produce same output */
+            ExpectIntEQ(XMEMCMP(enc_a, enc_b, sizeof(enc_a)), 0);
+        }
+
+        /* Alternating pattern: adjacent elements get different values */
+        for (j = 0; j < DILITHIUM_N; j++) {
+            w1[j] = (j & 1) ? 43 : 0;
+        }
+        XMEMSET(enc_a, 0, sizeof(enc_a));
+        XMEMSET(enc_b, 0, sizeof(enc_b));
+        wc_dilithium_encode_w1_88(w1, enc_a);
+        wc_dilithium_encode_w1_88(w1, enc_b);
+        ExpectIntEQ(XMEMCMP(enc_a, enc_b, sizeof(enc_a)), 0);
+
+        /* Ascending pattern: each element differs */
+        for (j = 0; j < DILITHIUM_N; j++) {
+            w1[j] = (sword32)(j % 44);  /* 0..43 cycling */
+        }
+        XMEMSET(enc_a, 0, sizeof(enc_a));
+        XMEMSET(enc_b, 0, sizeof(enc_b));
+        wc_dilithium_encode_w1_88(w1, enc_a);
+        wc_dilithium_encode_w1_88(w1, enc_b);
+        ExpectIntEQ(XMEMCMP(enc_a, enc_b, sizeof(enc_a)), 0);
+    }
+#endif /* !WOLFSSL_NO_ML_DSA_44 */
+
+    /* ---- 4-bit encoding (dilithium_encode_w1_32 path) ---- */
+#if !defined(WOLFSSL_NO_ML_DSA_65) || !defined(WOLFSSL_NO_ML_DSA_87)
+    {
+        ALIGN32 byte enc_a[DILITHIUM_N * DILITHIUM_Q_HI_32_ENC_BITS / 8];
+        ALIGN32 byte enc_b[DILITHIUM_N * DILITHIUM_Q_HI_32_ENC_BITS / 8];
+
+        /* Uniform fill with each pattern */
+        for (k = 0; k < (unsigned int)n_patterns; k++) {
+            for (j = 0; j < DILITHIUM_N; j++) {
+                w1[j] = patterns[k];
+            }
+
+            XMEMSET(enc_a, 0, sizeof(enc_a));
+            XMEMSET(enc_b, 0, sizeof(enc_b));
+            wc_dilithium_encode_w1_32(w1, enc_a);
+            wc_dilithium_encode_w1_32(w1, enc_b);
+
+            ExpectIntEQ(XMEMCMP(enc_a, enc_b, sizeof(enc_a)), 0);
+        }
+
+        /* Alternating pattern */
+        for (j = 0; j < DILITHIUM_N; j++) {
+            w1[j] = (j & 1) ? 15 : 0;
+        }
+        XMEMSET(enc_a, 0, sizeof(enc_a));
+        XMEMSET(enc_b, 0, sizeof(enc_b));
+        wc_dilithium_encode_w1_32(w1, enc_a);
+        wc_dilithium_encode_w1_32(w1, enc_b);
+        ExpectIntEQ(XMEMCMP(enc_a, enc_b, sizeof(enc_a)), 0);
+
+        /* Ascending pattern */
+        for (j = 0; j < DILITHIUM_N; j++) {
+            w1[j] = (sword32)(j % 16);  /* 0..15 cycling */
+        }
+        XMEMSET(enc_a, 0, sizeof(enc_a));
+        XMEMSET(enc_b, 0, sizeof(enc_b));
+        wc_dilithium_encode_w1_32(w1, enc_a);
+        wc_dilithium_encode_w1_32(w1, enc_b);
+        ExpectIntEQ(XMEMCMP(enc_a, enc_b, sizeof(enc_a)), 0);
+    }
+#endif /* !WOLFSSL_NO_ML_DSA_65 || !WOLFSSL_NO_ML_DSA_87 */
+
+#endif /* HAVE_DILITHIUM && WOLFSSL_WC_DILITHIUM && sign/verify */
+    return EXPECT_RESULT();
+}
+
 int test_mldsa_pkcs12(void)
 {
     EXPECT_DECLS;
diff --git a/tests/api/test_mldsa.h b/tests/api/test_mldsa.h
@@ -40,6 +40,7 @@ int test_wc_dilithium_verify_kats(void);
 int test_wc_Dilithium_PrivateKeyDecode_OpenSSL_form(void);
 int test_mldsa_pkcs8_import_OpenSSL_form(void);
 int test_mldsa_pkcs8_export_import_wolfSSL_form(void);
+int test_wc_dilithium_encode_w1_large_values(void);
 int test_mldsa_pkcs12(void);
 
 #define TEST_MLDSA_DECLS                                                       \
@@ -59,6 +60,7 @@ int test_mldsa_pkcs12(void);
     TEST_DECL_GROUP("mldsa", test_wc_Dilithium_PrivateKeyDecode_OpenSSL_form), \
     TEST_DECL_GROUP("mldsa", test_mldsa_pkcs8_import_OpenSSL_form),            \
     TEST_DECL_GROUP("mldsa", test_mldsa_pkcs8_export_import_wolfSSL_form),     \
+    TEST_DECL_GROUP("mldsa", test_wc_dilithium_encode_w1_large_values),         \
     TEST_DECL_GROUP("mldsa", test_mldsa_pkcs12)
 
 #endif /* WOLFCRYPT_TEST_MLDSA_H */
diff --git a/wolfcrypt/src/dilithium.c b/wolfcrypt/src/dilithium.c
@@ -2191,15 +2191,24 @@ static void dilithium_encode_w1_88_c(const sword32* w1, byte* w1e)
          * 16 numbers in 12 bytes. (16 * 6 bits = 12 * 8 bits) */
 #if defined(LITTLE_ENDIAN_ORDER) && (WOLFSSL_DILITHIUM_ALIGNMENT <= 4)
         word32* w1e32 = (word32*)w1e;
-        w1e32[0] = (word32)( w1[j+ 0]        | (w1[j+ 1] <<  6) |
-                            (w1[j+ 2] << 12) | (w1[j+ 3] << 18) |
-                            (w1[j+ 4] << 24) | (w1[j+ 5] << 30));
-        w1e32[1] = (word32)((w1[j+ 5] >>  2) | (w1[j+ 6] <<  4) |
-                            (w1[j+ 7] << 10) | (w1[j+ 8] << 16) |
-                            (w1[j+ 9] << 22) | (w1[j+10] << 28));
-        w1e32[2] = (word32)((w1[j+10] >>  4) | (w1[j+11] <<  2) |
-                            (w1[j+12] <<  8) | (w1[j+13] << 14) |
-                            (w1[j+14] << 20) | (w1[j+15] << 26));
+        w1e32[0] = (word32)( (word32)w1[j+ 0]        |
+                            ((word32)w1[j+ 1] <<  6) |
+                            ((word32)w1[j+ 2] << 12) |
+                            ((word32)w1[j+ 3] << 18) |
+                            ((word32)w1[j+ 4] << 24) |
+                            ((word32)w1[j+ 5] << 30));
+        w1e32[1] = (word32)(((word32)w1[j+ 5] >>  2) |
+                            ((word32)w1[j+ 6] <<  4) |
+                            ((word32)w1[j+ 7] << 10) |
+                            ((word32)w1[j+ 8] << 16) |
+                            ((word32)w1[j+ 9] << 22) |
+                            ((word32)w1[j+10] << 28));
+        w1e32[2] = (word32)(((word32)w1[j+10] >>  4) |
+                            ((word32)w1[j+11] <<  2) |
+                            ((word32)w1[j+12] <<  8) |
+                            ((word32)w1[j+13] << 14) |
+                            ((word32)w1[j+14] << 20) |
+                            ((word32)w1[j+15] << 26));
 #else
         w1e[ 0] = (byte)( w1[j+ 0]       | (w1[j+ 1] << 6));
         w1e[ 1] = (byte)((w1[j+ 1] >> 2) | (w1[j+ 2] << 4));
@@ -2238,6 +2247,11 @@ static void dilithium_encode_w1_88(const sword32* w1, byte* w1e)
         dilithium_encode_w1_88_c(w1, w1e);
     }
 }
+
+WOLFSSL_TEST_VIS void wc_dilithium_encode_w1_88(const sword32* w1, byte* w1e)
+{
+    dilithium_encode_w1_88(w1, w1e);
+}
 #endif /* !WOLFSSL_NO_ML_DSA_44 */
 
 #if !defined(WOLFSSL_NO_ML_DSA_65) || !defined(WOLFSSL_NO_ML_DSA_87)
@@ -2263,14 +2277,22 @@ static void dilithium_encode_w1_32_c(const sword32* w1, byte* w1e)
          * 16 numbers in 8 bytes. (16 * 4 bits = 8 * 8 bits) */
 #if defined(LITTLE_ENDIAN_ORDER) && (WOLFSSL_DILITHIUM_ALIGNMENT <= 8)
         word32* w1e32 = (word32*)w1e;
-        w1e32[0] = (word32)((w1[j +  0] <<  0) | (w1[j +  1] <<  4) |
-                            (w1[j +  2] <<  8) | (w1[j +  3] << 12) |
-                            (w1[j +  4] << 16) | (w1[j +  5] << 20) |
-                            (w1[j +  6] << 24) | (w1[j +  7] << 28));
-        w1e32[1] = (word32)((w1[j +  8] <<  0) | (w1[j +  9] <<  4) |
-                            (w1[j + 10] <<  8) | (w1[j + 11] << 12) |
-                            (w1[j + 12] << 16) | (w1[j + 13] << 20) |
-                            (w1[j + 14] << 24) | (w1[j + 15] << 28));
+        w1e32[0] = (word32)(((word32)w1[j +  0] <<  0) |
+                            ((word32)w1[j +  1] <<  4) |
+                            ((word32)w1[j +  2] <<  8) |
+                            ((word32)w1[j +  3] << 12) |
+                            ((word32)w1[j +  4] << 16) |
+                            ((word32)w1[j +  5] << 20) |
+                            ((word32)w1[j +  6] << 24) |
+                            ((word32)w1[j +  7] << 28));
+        w1e32[1] = (word32)(((word32)w1[j +  8] <<  0) |
+                            ((word32)w1[j +  9] <<  4) |
+                            ((word32)w1[j + 10] <<  8) |
+                            ((word32)w1[j + 11] << 12) |
+                            ((word32)w1[j + 12] << 16) |
+                            ((word32)w1[j + 13] << 20) |
+                            ((word32)w1[j + 14] << 24) |
+                            ((word32)w1[j + 15] << 28));
 #else
         w1e[0] = (byte)(w1[j +  0] | (w1[j +  1] << 4));
         w1e[1] = (byte)(w1[j +  2] | (w1[j +  3] << 4));
@@ -2305,6 +2327,11 @@ static void dilithium_encode_w1_32(const sword32* w1, byte* w1e)
         dilithium_encode_w1_32_c(w1, w1e);
     }
 }
+
+WOLFSSL_TEST_VIS void wc_dilithium_encode_w1_32(const sword32* w1, byte* w1e)
+{
+    dilithium_encode_w1_32(w1, w1e);
+}
 #endif
 #endif
 
diff --git a/wolfssl/wolfcrypt/dilithium.h b/wolfssl/wolfcrypt/dilithium.h
@@ -1145,6 +1145,16 @@ WOLFSSL_API int wc_MlDsaKey_GetPrivLen(MlDsaKey* key, int* len);
 WOLFSSL_API int wc_MlDsaKey_GetPubLen(MlDsaKey* key, int* len);
 WOLFSSL_API int wc_MlDsaKey_GetSigLen(MlDsaKey* key, int* len);
 
+#if !defined(WOLFSSL_DILITHIUM_NO_SIGN) || \
+    !defined(WOLFSSL_DILITHIUM_NO_VERIFY)
+#ifndef WOLFSSL_NO_ML_DSA_44
+WOLFSSL_TEST_VIS void wc_dilithium_encode_w1_88(const sword32* w1, byte* w1e);
+#endif
+#if !defined(WOLFSSL_NO_ML_DSA_65) || !defined(WOLFSSL_NO_ML_DSA_87)
+WOLFSSL_TEST_VIS void wc_dilithium_encode_w1_32(const sword32* w1, byte* w1e);
+#endif
+#endif
+
 #ifdef __cplusplus
     }    /* extern "C" */
 #endif
