From f7c6df9c81d84e5a3efa335cfc1e1863b548857a Mon Sep 17 00:00:00 2001
From: Oleg Andreev <oleganza@gmail.com>
Date: Mon, 12 Aug 2019 22:23:55 +0300
Subject: [PATCH] Better validation of proof shares in Range Proof MPC (#296)

Addressing issues raised by the quarkslab audit (to be released soon):

* additional size validation for individual proof shares,
* correct `size_hint` for `AggregatedGensIter`,
* reused computation of powers-of-z in the proof aggregation to save a bit of CPU.
---
 Cargo.toml                  |  2 +-
 src/generators.rs           |  2 +-
 src/range_proof/dealer.rs   | 14 ++++++++++++++
 src/range_proof/messages.rs | 29 +++++++++++++++++++++++++++++
 src/range_proof/party.rs    | 12 +++++-------
 5 files changed, 50 insertions(+), 9 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index 36cdbbbe..5a1b7743 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -12,7 +12,7 @@ keywords = ["cryptography", "crypto", "ristretto", "zero-knowledge", "bulletproo
 description = "A pure-Rust implementation of Bulletproofs using Ristretto"
 
 [dependencies]
-curve25519-dalek = { version = "1.0.3", features = ["serde"] }
+curve25519-dalek = { version = "^1.2.3", features = ["serde"] }
 subtle = "2"
 sha3 = "0.8"
 digest = "0.8"
diff --git a/src/generators.rs b/src/generators.rs
index 10937917..20d4e249 100644
--- a/src/generators.rs
+++ b/src/generators.rs
@@ -251,7 +251,7 @@ impl<'a> Iterator for AggregatedGensIter<'a> {
     }
 
     fn size_hint(&self) -> (usize, Option<usize>) {
-        let size = self.n * self.m;
+        let size = self.n * (self.m - self.party_idx) - self.gen_idx;
         (size, Some(size))
     }
 }
diff --git a/src/range_proof/dealer.rs b/src/range_proof/dealer.rs
index 6023db70..80d6bf63 100644
--- a/src/range_proof/dealer.rs
+++ b/src/range_proof/dealer.rs
@@ -218,6 +218,20 @@ impl<'a, 'b> DealerAwaitingProofShares<'a, 'b> {
             return Err(MPCError::WrongNumProofShares);
         }
 
+        // Validate lengths for each share
+        let mut bad_shares = Vec::<usize>::new(); // no allocations until we append
+        for (j, share) in proof_shares.iter().enumerate() {
+            share
+                .check_size(self.n, &self.bp_gens, j)
+                .unwrap_or_else(|_| {
+                    bad_shares.push(j);
+                });
+        }
+
+        if bad_shares.len() > 0 {
+            return Err(MPCError::MalformedProofShares { bad_shares });
+        }
+
         let t_x: Scalar = proof_shares.iter().map(|ps| ps.t_x).sum();
         let t_x_blinding: Scalar = proof_shares.iter().map(|ps| ps.t_x_blinding).sum();
         let e_blinding: Scalar = proof_shares.iter().map(|ps| ps.e_blinding).sum();
diff --git a/src/range_proof/messages.rs b/src/range_proof/messages.rs
index 775ff0e8..db59f15a 100644
--- a/src/range_proof/messages.rs
+++ b/src/range_proof/messages.rs
@@ -49,6 +49,32 @@ pub struct ProofShare {
 }
 
 impl ProofShare {
+    /// Checks consistency of all sizes in the proof share and returns the size of the l/r vector.
+    pub(super) fn check_size(
+        &self,
+        expected_n: usize,
+        bp_gens: &BulletproofGens,
+        j: usize,
+    ) -> Result<(), ()> {
+        if self.l_vec.len() != expected_n {
+            return Err(());
+        }
+
+        if self.r_vec.len() != expected_n {
+            return Err(());
+        }
+
+        if expected_n > bp_gens.gens_capacity {
+            return Err(());
+        }
+
+        if j >= bp_gens.party_capacity {
+            return Err(());
+        }
+
+        Ok(())
+    }
+
     /// Audit an individual proof share to determine whether it is
     /// malformed.
     pub(super) fn audit_share(
@@ -69,6 +95,9 @@ impl ProofShare {
         use util;
 
         let n = self.l_vec.len();
+
+        self.check_size(n, bp_gens, j)?;
+
         let (y, z) = (&bit_challenge.y, &bit_challenge.z);
         let x = &poly_challenge.x;
 
diff --git a/src/range_proof/party.rs b/src/range_proof/party.rs
index 017ff17e..8bbf8621 100644
--- a/src/range_proof/party.rs
+++ b/src/range_proof/party.rs
@@ -169,7 +169,7 @@ impl<'a> PartyAwaitingBitChallenge<'a> {
         let mut l_poly = util::VecPoly1::zero(n);
         let mut r_poly = util::VecPoly1::zero(n);
 
-        let zz = vc.z * vc.z;
+        let offset_zz = vc.z * vc.z * offset_z;
         let mut exp_y = offset_y; // start at y^j
         let mut exp_2 = Scalar::one(); // start at 2^0 = 1
         for i in 0..n {
@@ -178,7 +178,7 @@ impl<'a> PartyAwaitingBitChallenge<'a> {
 
             l_poly.0[i] = a_L_i - vc.z;
             l_poly.1[i] = self.s_L[i];
-            r_poly.0[i] = exp_y * (a_R_i + vc.z) + zz * offset_z * exp_2;
+            r_poly.0[i] = exp_y * (a_R_i + vc.z) + offset_zz * exp_2;
             r_poly.1[i] = exp_y * self.s_R[i];
 
             exp_y *= vc.y; // y^i -> y^(i+1)
@@ -202,8 +202,7 @@ impl<'a> PartyAwaitingBitChallenge<'a> {
             v_blinding: self.v_blinding,
             a_blinding: self.a_blinding,
             s_blinding: self.s_blinding,
-            z: vc.z,
-            offset_z,
+            offset_zz,
             l_poly,
             r_poly,
             t_poly,
@@ -240,8 +239,7 @@ impl<'a> Drop for PartyAwaitingBitChallenge<'a> {
 /// A party which has committed to their polynomial coefficents
 /// and is waiting for the polynomial challenge from the dealer.
 pub struct PartyAwaitingPolyChallenge {
-    z: Scalar,
-    offset_z: Scalar,
+    offset_zz: Scalar,
     l_poly: util::VecPoly1,
     r_poly: util::VecPoly1,
     t_poly: util::Poly2,
@@ -263,7 +261,7 @@ impl PartyAwaitingPolyChallenge {
         }
 
         let t_blinding_poly = util::Poly2(
-            self.z * self.z * self.offset_z * self.v_blinding,
+            self.offset_zz * self.v_blinding,
             self.t_1_blinding,
             self.t_2_blinding,
         );