Mitigating high impact issues (#300)

* feat: added unblind username feature

* feat: checking maximum value of converted username

* feat: added warning about possiblity of private data leakage while verifying process

* feat: added warning about 'validateVKPermutationsLength' method

* fix typo

backend/README.md

@@ -144,4 +144,5 @@ The result will display as:
 4. Verifying the proof on contract verifier for User #0: true
 ```
 
-**Note:** In a production environment, users can independently verify their proof using public interfaces, such as Etherscan.
+**Note:** In a production environment, users can independently verify their proof using public interfaces, such as Etherscan. 
+Also, It's crucial for the prover to inform users about the potential risk of private data leakage, specifically their balances, during the proof verification process. This is especially important when using a third-party endpoint to query `verify_inclusion_proof` method.

contracts/src/Summa.sol

@@ -136,6 +136,13 @@ contract Summa is Ownable {
      * @param vkContract The address of the verifying key contract
      * @param numberOfCurrencies The number of cryptocurrencies whose polynomials are committed in the proof
      * @return isValid True if the number of permutations in the verifying key corresponds to the number of cryptocurrencies
+     *
+     * WARNING: The permutation length may not be correctly calculated by this method if the prover deliberately
+     * tries to deceive the process. This issue cannot be resolved even if we change the approach to rely on user input
+     * for the length instead of calculating it within the method. The ultimate solution is to implement a validation process for the
+     * verifying key contract that can be performed by the user themselves. This issue will be addressed in the following: 
+     * https://github.com/summa-dev/summa-solvency/issues/299
+     */
      */
     function validateVKPermutationsLength(
         address vkContract,

prover/Cargo.toml

@@ -9,6 +9,7 @@ edition = "2021"
 dev-graph = ["halo2_proofs/dev-graph", "plotters"]
 profiling = []
 no_range_check = []
+unblind-username = []
 
 
 [dependencies]

prover/src/circuits/univariate_grand_sum.rs

@@ -72,7 +72,15 @@ where
     [(); N_CURRENCIES + 1]:,
 {
     fn configure(meta: &mut ConstraintSystem<Fp>) -> Self {
-        let username = meta.advice_column();
+        // By default, the username is in a blinded column like any other private inputs in common Halo2 circuit.
+        // However, this makes it indistinguishable between fake user data and real user data that has zero values in Summa circuit.
+        // Therefore, we have introduced a feature that allows the username to be unblinded instead of blinded, which may be necessary for the prover in the future.
+        // Refer to this comment for more details: https://github.com/zBlock-2/summa-solvency/issues/9#issuecomment-2143427298
+        let username: Column<Advice> = if cfg!(feature = "unblind-username") {
+            meta.unblinded_advice_column()
+        } else {
+            meta.advice_column()
+        };
 
         let balances = [(); N_CURRENCIES].map(|_| meta.unblinded_advice_column());
 

prover/src/entry.rs

@@ -1,6 +1,7 @@
+use halo2_proofs::halo2curves::bn256::Fr as Fp;
 use num_bigint::BigUint;
 
-use crate::utils::big_intify_username;
+use crate::utils::{big_intify_username, fp_to_big_uint};
 
 /// An entry in the Merkle Sum Tree from the database of the CEX.
 /// It contains the username and the balances of the user.
@@ -13,8 +14,19 @@ pub struct Entry<const N_CURRENCIES: usize> {
 
 impl<const N_CURRENCIES: usize> Entry<N_CURRENCIES> {
     pub fn new(username: String, balances: [BigUint; N_CURRENCIES]) -> Result<Self, &'static str> {
+        let username_as_big_uint = big_intify_username(&username);
+        let max_allowed_value = fp_to_big_uint(Fp::zero() - Fp::one());
+
+        // Ensure the username, when converted to a BigUint, does not exceed the field modulus
+        // This prevents potential overflow issues by asserting that the username's numeric value
+        // is within the allowable range defined by the field modulus
+        // Please refer to https://github.com/zBlock-2/audit-report/blob/main/versionB.md#4-high-missing-username-range-check-in-big_intify_username--big_uint_to_fp
+        if username_as_big_uint > max_allowed_value {
+            return Err("The value that converted username should not exceed field modulus");
+        }
+
         Ok(Entry {
-            username_as_big_uint: big_intify_username(&username),
+            username_as_big_uint,
             balances,
             username,
         })
@@ -42,3 +54,16 @@ impl<const N_CURRENCIES: usize> Entry<N_CURRENCIES> {
         &self.username
     }
 }
+
+#[cfg(test)]
+#[test]
+fn test_entry_new() {
+    let short_username_entry = Entry::new(String::from("userA"), [BigUint::from(0u32)]);
+    assert!(short_username_entry.is_ok());
+
+    let long_username_entry = Entry::new(
+        String::from("userABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"),
+        [BigUint::from(0u32)],
+    );
+    assert!(long_username_entry.is_err())
+}