Merge pull request #4982 from nymtech/cure53_SI86_SI87

Fix critical issues SI86 and SI87 from Cure53

Cargo.lock

@@ -4868,6 +4868,7 @@ dependencies = [
  "rayon",
  "serde",
  "sha2 0.9.9",
+ "subtle 2.5.0",
  "thiserror",
  "zeroize",
 ]

Cargo.toml

@@ -350,6 +350,7 @@ prometheus = { version = "0.13.0" }
 bls12_381 = { git = "https://github.com/jstuczyn/bls12_381", default-features = false, branch = "temp/experimental-serdect" }
 group = { version = "0.13.0", default-features = false }
 ff = { version = "0.13.0", default-features = false }
+subtle = "2.5.0"
 
 # cosmwasm-related
 cosmwasm-schema = "=1.4.3"

common/nym_offline_compact_ecash/Cargo.toml

@@ -25,6 +25,7 @@ rayon = { workspace = true, optional = true }
 zeroize = { workspace = true, features = ["zeroize_derive"] }
 ff = { workspace = true }
 group = { workspace = true }
+subtle = { workspace = true }
 
 nym-pemstore = { path = "../pemstore" }
 nym-network-defaults = { path = "../network-defaults", default-features = false }

common/nym_offline_compact_ecash/src/common_types.rs

@@ -6,6 +6,7 @@ use crate::error::Result;
 use crate::helpers::{g1_tuple_to_bytes, recover_g1_tuple};
 use bls12_381::{G1Projective, Scalar};
 use serde::{Deserialize, Serialize};
+use subtle::Choice;
 
 pub type SignerIndex = u64;
 
@@ -58,6 +59,11 @@ impl Signature {
         let (h, s) = recover_g1_tuple::<Self>(bytes)?;
         Ok(Signature { h, s })
     }
+
+    /// Checks whether any of the group elements of the signature is an identity element located at infinity
+    pub fn is_at_infinity(&self) -> Choice {
+        self.s.is_identity() | self.h.is_identity()
+    }
 }
 
 #[derive(Debug, Clone, Copy, PartialEq, Serialize, Deserialize)]

common/nym_offline_compact_ecash/src/scheme/aggregation.rs

@@ -50,6 +50,17 @@ where
 
 impl Aggregatable for PartialSignature {
     fn aggregate(sigs: &[PartialSignature], indices: Option<&[u64]>) -> Result<Signature> {
+        // Ensure that we have valid signatures
+        if sigs.is_empty() {
+            return Err(CompactEcashError::AggregationEmptySet);
+        }
+
+        // Check each individual signature for point at infinity
+        for sig in sigs {
+            if bool::from(sig.is_at_infinity()) {
+                return Err(CompactEcashError::IdentitySignature);
+            }
+        }
         let h = sigs
             .first()
             .ok_or(CompactEcashError::AggregationEmptySet)?
@@ -109,7 +120,8 @@ pub fn aggregate_signatures(
         Err(err) => return Err(err),
     };
 
-    if bool::from(signature.h.is_identity()) {
+    // Ensure the aggregated signature is not an infinity point
+    if bool::from(signature.is_at_infinity()) {
         return Err(CompactEcashError::IdentitySignature);
     }
 

common/nymcoconut/src/scheme/aggregation.rs

@@ -253,7 +253,7 @@ mod tests {
 
         let sigs = sks
             .iter()
-            .map(|sk| sign(&params, sk, &attributes).unwrap())
+            .map(|sk| sign(sk, &attributes).unwrap())
             .collect::<Vec<_>>();
 
         // aggregating (any) threshold works

common/nymcoconut/src/scheme/issuance.rs

@@ -452,11 +452,7 @@ pub fn verify_partial_blind_signature(
 }
 
 /// Creates a Coconut Signature under a given secret key on a set of public attributes only.
-pub fn sign(
-    params: &Parameters,
-    secret_key: &SecretKey,
-    public_attributes: &[&Attribute],
-) -> Result<Signature> {
+pub fn sign(secret_key: &SecretKey, public_attributes: &[&Attribute]) -> Result<Signature> {
     if public_attributes.len() > secret_key.ys.len() {
         return Err(CoconutError::IssuanceMaxAttributes {
             max: secret_key.ys.len(),
@@ -464,13 +460,23 @@ pub fn sign(
         });
     }
 
-    // TODO: why in the python implementation this hash onto the curve is present
-    // while it's not used in the paper? the paper uses random exponent instead.
-    // (the python implementation hashes string representation of all attributes onto the curve,
-    // but I think the same can be achieved by just summing the attributes thus avoiding the unnecessary
-    // transformation. If I'm wrong, please correct me.)
-    let attributes_sum = public_attributes.iter().copied().sum::<Scalar>();
-    let h = hash_g1((params.gen1() * attributes_sum).to_bytes());
+    //Serialize the array structure of the public attributes into a byte array
+    let mut serialized_attributes = Vec::new();
+    //Prepend the length of the entire array (in bytes)
+    let array_len = public_attributes.len() as u64;
+    serialized_attributes.extend_from_slice(&array_len.to_le_bytes());
+    //Serialize each attribute with its length
+    for &attribute in public_attributes.iter() {
+        let attr_bytes = attribute.to_bytes();
+        let attr_len = attr_bytes.len() as u64;
+
+        // Prefix the attribute with its length
+        serialized_attributes.extend_from_slice(&attr_len.to_le_bytes());
+        serialized_attributes.extend_from_slice(&attr_bytes);
+    }
+
+    //Hash the resulting byte array to derive the point H
+    let h = hash_g1(serialized_attributes);
 
     // x + m0 * y0 + m1 * y1 + ... mn * yn
     let exponent = secret_key.x

common/nymcoconut/src/scheme/mod.rs

@@ -436,8 +436,8 @@ mod tests {
 
         let keypair1 = keygen(&params);
         let keypair2 = keygen(&params);
-        let sig1 = sign(&params, keypair1.secret_key(), &attributes).unwrap();
-        let sig2 = sign(&params, keypair2.secret_key(), &attributes).unwrap();
+        let sig1 = sign(keypair1.secret_key(), &attributes).unwrap();
+        let sig2 = sign(keypair2.secret_key(), &attributes).unwrap();
 
         assert!(verify(
             &params,

common/nymcoconut/src/scheme/verification.rs

@@ -312,6 +312,7 @@ pub fn verify(
 
 #[cfg(test)]
 mod tests {
+    use crate::scheme::issuance::sign;
     use crate::scheme::keygen::keygen;
     use crate::scheme::setup::setup;
 
@@ -355,4 +356,77 @@ mod tests {
             theta
         );
     }
+
+    #[test]
+    fn reject_forged_signature_via_linear_combination() {
+        // This test checks if the protocol correctly rejects forged signatures created
+        // by linear combinations of valid signatures. The verification for forged
+        // signatures should fail.
+        let params = Parameters::new(4).unwrap();
+
+        let scalar_2 = Scalar::one() + Scalar::one();
+        let scalar_2_inv = Scalar::invert(&scalar_2).unwrap();
+
+        //#1
+        let a = params.random_scalar();
+        let zero = Scalar::zero();
+        let a_zero = vec![&a, &zero];
+        let zero_a = vec![&zero, &a];
+
+        let validator_keypair = keygen(&params);
+
+        //#2
+        let sig_a_zero = sign(validator_keypair.secret_key(), &a_zero).unwrap();
+        let sig_zero_a = sign(validator_keypair.secret_key(), &zero_a).unwrap();
+
+        assert!(verify(
+            &params,
+            validator_keypair.verification_key(),
+            &a_zero,
+            &sig_a_zero
+        ));
+        assert!(verify(
+            &params,
+            validator_keypair.verification_key(),
+            &zero_a,
+            &sig_zero_a
+        ));
+
+        //#3
+        let h0 = sig_a_zero.0;
+        // Removed unnecessary references
+        let h1 = scalar_2_inv * sig_a_zero.1 + scalar_2_inv * sig_zero_a.1;
+        let forged_signature = Signature(h0, h1);
+        let a_half = a * scalar_2_inv;
+        let new_plaintext = vec![&a_half, &a_half];
+
+        // The forged signature should not pass verification
+        assert!(!verify(
+            &params,
+            validator_keypair.verification_key(),
+            &new_plaintext,
+            &forged_signature
+        ));
+
+        //#4
+        let scalar_3 = Scalar::one() + Scalar::one() + Scalar::one();
+        let scalar_4 = Scalar::one() + Scalar::one() + Scalar::one() + Scalar::one();
+        let scalar_4_inv = Scalar::invert(&scalar_4).unwrap();
+        let scalar_3_over_4 = scalar_3 * scalar_4_inv;
+
+        // Removed unnecessary references
+        let h1_2 = scalar_4_inv * sig_a_zero.1 + scalar_3_over_4 * sig_zero_a.1;
+        let forged_signature_2 = Signature(h0, h1_2);
+        let a_quarter = a * scalar_4_inv;
+        let a_3_over_4 = a * scalar_3_over_4;
+        let new_plaintext_2 = vec![&a_quarter, &a_3_over_4];
+
+        // The second forged signature should also not pass verification
+        assert!(!verify(
+            &params,
+            validator_keypair.verification_key(),
+            &new_plaintext_2,
+            &forged_signature_2
+        ));
+    }
 }

nym-wallet/Cargo.lock

@@ -3156,6 +3156,7 @@ dependencies = [
  "rand 0.8.5",
  "serde",
  "sha2 0.9.9",
+ "subtle 2.5.0",
  "thiserror",
  "zeroize",
 ]

wasm/zknym-lib/src/generic_scheme/coconut/mod.rs

@@ -96,17 +96,14 @@ pub fn ttp_keygen(
 pub fn sign_simple(
     attributes: Vec<String>,
     keys: &KeyPairWrapper,
-    parameters: Option<ParametersWrapper>,
 ) -> Result<CredentialWrapper, ZkNymError> {
-    let params = get_params(&parameters);
-
     let public_attributes = attributes
         .into_iter()
         .map(hash_to_scalar)
         .collect::<Vec<_>>();
     let attributes_ref = public_attributes.iter().collect::<Vec<_>>();
 
-    nym_coconut::sign(params, keys.secret_key(), &attributes_ref)
+    nym_coconut::sign(keys.secret_key(), &attributes_ref)
         .map(Into::into)
         .map_err(Into::into)
 }