From 7fda713a8224b893d93c26108d6f4d7feff22040 Mon Sep 17 00:00:00 2001
From: AriaNaraghi <arianari97@gmail.com>
Date: Mon, 6 Jul 2026 00:30:46 +0300
Subject: [PATCH] fix: reject non-canonical field elements in FieldArray SSZ
 decoding

FieldArray::from_ssz_bytes decoded each limb with F::new(u32), which reduces
its argument mod p. Because 2p - 1 < 2^32, every canonical limb value v has a
non-canonical alias v + p that fits in a u32 and decodes to the same element,
while encoding always emits the canonical form. This made the SSZ encoding of
signatures, public keys, and secret keys malleable, contradicting the canonical
encoding contract required for consensus-layer compatibility.

Reject any limb >= F::ORDER_U32 with DecodeError::BytesInvalid and add a
regression test. Mirrors the canonical-deserialization check already enforced
on the field type in leanVM (audit finding F-01).
---
 src/array.rs | 55 ++++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 49 insertions(+), 6 deletions(-)

diff --git a/src/array.rs b/src/array.rs
index 0ef0b3a..3950a32 100644
--- a/src/array.rs
+++ b/src/array.rs
@@ -91,13 +91,24 @@ impl<const N: usize> Decode for FieldArray<N> {
             });
         }
 
-        let arr = std::array::from_fn(|i| {
-            let start = i * F::NUM_BYTES;
-            let chunk = bytes[start..start + F::NUM_BYTES].try_into().unwrap();
-            F::new(u32::from_le_bytes(chunk))
-        });
+        let mut elements = [F::ZERO; N];
+        for (element, chunk) in elements.iter_mut().zip(bytes.chunks_exact(F::NUM_BYTES)) {
+            let limb = u32::from_le_bytes(chunk.try_into().unwrap());
+            // Reject non-canonical encodings. `F::new` reduces its argument mod p,
+            // so without this check a limb `v` and its alias `v + p` (which fits in
+            // a u32, since 2p - 1 < 2^32) would decode to the same field element,
+            // making the SSZ encoding of signatures and keys malleable. The SSZ
+            // contract requires the canonical representative in `[0, p)`.
+            if limb >= F::ORDER_U32 {
+                return Err(DecodeError::BytesInvalid(format!(
+                    "non-canonical field element {limb} (>= field modulus {})",
+                    F::ORDER_U32
+                )));
+            }
+            *element = F::new(limb);
+        }
 
-        Ok(Self(arr))
+        Ok(Self(elements))
     }
 }
 
@@ -193,6 +204,38 @@ mod tests {
         assert_eq!(original, decoded, "Round-trip failed for max values");
     }
 
+    #[test]
+    fn test_ssz_decode_rejects_non_canonical() {
+        // Encode a canonical array, then tamper a limb with a non-canonical
+        // representation. The decoder must reject it: otherwise two distinct byte
+        // strings decode to the same value, making serialized signatures and keys
+        // malleable.
+        let original = FieldArray([F::new(1), F::new(2), F::new(3)]);
+        let mut encoded = original.as_ssz_bytes();
+
+        // Sanity: the canonical encoding round-trips.
+        assert_eq!(FieldArray::<3>::from_ssz_bytes(&encoded).unwrap(), original);
+
+        // A limb equal to the modulus p is non-canonical and must be rejected.
+        encoded[0..F::NUM_BYTES].copy_from_slice(&F::ORDER_U32.to_le_bytes());
+        assert!(
+            FieldArray::<3>::from_ssz_bytes(&encoded).is_err(),
+            "decoder accepted a field element equal to the modulus"
+        );
+
+        // The concrete malleability vector: p + 1 is the alias of the first limb
+        // (value 1) and, before the fix, decoded to the same element.
+        encoded[0..F::NUM_BYTES].copy_from_slice(&(F::ORDER_U32 + 1).to_le_bytes());
+        assert!(
+            FieldArray::<3>::from_ssz_bytes(&encoded).is_err(),
+            "decoder accepted a non-canonical alias of a field element"
+        );
+
+        // The largest canonical value, p - 1, is still accepted.
+        encoded[0..F::NUM_BYTES].copy_from_slice(&(F::ORDER_U32 - 1).to_le_bytes());
+        assert!(FieldArray::<3>::from_ssz_bytes(&encoded).is_ok());
+    }
+
     #[test]
     fn test_ssz_roundtrip_specific_values() {
         // Create an array with sequential values for easy verification