Merge pull request #88 from GaloisInc/checked_shr

Use `checked_shr` instead of `>>` in `Field::random`

Cargo.toml

@@ -20,6 +20,9 @@ ff_derive = { version = "0.12", path = "ff_derive", optional = true }
 rand_core = { version = "0.6", default-features = false }
 subtle = { version = "2.2.1", default-features = false, features = ["i128"] }
 
+[dev-dependencies]
+rand = "0.8"
+
 [features]
 default = ["bits", "std"]
 alloc = []

ff_derive/src/lib.rs

@@ -1233,7 +1233,11 @@ fn prime_field_impl(
                     };
 
                     // Mask away the unused most-significant bits.
-                    tmp.0.as_mut()[#top_limb_index] &= 0xffffffffffffffff >> REPR_SHAVE_BITS;
+                    // Note: In some edge cases, `REPR_SHAVE_BITS` could be 64, in which case
+                    // `0xfff... >> REPR_SHAVE_BITS` overflows. So use `checked_shr` instead.
+                    // This is always sufficient because we will have at most one spare limb
+                    // to accommodate values of up to twice the modulus.
+                    tmp.0.as_mut()[#top_limb_index] &= 0xffffffffffffffffu64.checked_shr(REPR_SHAVE_BITS).unwrap_or(0);
 
                     if tmp.is_valid() {
                         return tmp

tests/derive.rs

@@ -21,6 +21,22 @@ mod fermat {
     struct Fermat65537Field([u64; 1]);
 }
 
+mod full_limbs {
+    #[derive(PrimeField)]
+    #[PrimeFieldModulus = "39402006196394479212279040100143613805079739270465446667948293404245721771496870329047266088258938001861606973112319"]
+    #[PrimeFieldGenerator = "19"]
+    #[PrimeFieldReprEndianness = "little"]
+    struct F384p([u64; 7]);
+
+    #[test]
+    fn random_masking_does_not_overflow() {
+        use ff::Field;
+        use rand::rngs::OsRng;
+
+        let _ = F384p::random(OsRng);
+    }
+}
+
 #[test]
 fn batch_inversion() {
     use ff::{BatchInverter, Field};