Merge pull request #656 from Mr-Leshiy/tally-share-validation

chain-impl-mockchain/benches/tally.rs

@@ -265,8 +265,10 @@ fn tally_benchmark(
         })
         .collect();
 
-    let decrypted_tally =
-        VoteTally::new_private(vote_plan.to_id(), DecryptedPrivateTally::new(shares));
+    let decrypted_tally = VoteTally::new_private(
+        vote_plan.to_id(),
+        DecryptedPrivateTally::new(shares).unwrap(),
+    );
     let fragment =
         controller
             .fragment_factory()

chain-impl-mockchain/src/certificate/mod.rs

@@ -17,7 +17,8 @@ pub use self::vote_plan::{
     VotePlan, VotePlanId, VotePlanProof,
 };
 pub use self::vote_tally::{
-    DecryptedPrivateTally, DecryptedPrivateTallyProposal, TallyProof, VoteTally, VoteTallyPayload,
+    DecryptedPrivateTally, DecryptedPrivateTallyError, DecryptedPrivateTallyProposal, TallyProof,
+    VoteTally, VoteTallyPayload,
 };
 pub use delegation::{OwnerStakeDelegation, StakeDelegation};
 pub use pool::{

chain-impl-mockchain/src/certificate/vote_tally.rs

@@ -12,6 +12,7 @@ use chain_core::{
 };
 use chain_crypto::Verification;
 use chain_vote::TallyDecryptShare;
+use thiserror::Error;
 use typed_bytes::{ByteArray, ByteBuilder};
 
 #[derive(Debug, Eq, PartialEq, Hash, Clone)]
@@ -39,6 +40,10 @@ pub enum TallyProof {
     },
 }
 
+#[derive(Debug, Error)]
+#[error("decrypt_shares in the proposal should have the same options amount")]
+pub struct DecryptedPrivateTallyError {}
+
 #[derive(Debug, Eq, PartialEq, Hash, Clone)]
 pub struct DecryptedPrivateTally {
     inner: Box<[DecryptedPrivateTallyProposal]>,
@@ -167,9 +172,21 @@ impl TallyProof {
 }
 
 impl DecryptedPrivateTally {
-    pub fn new(proposals: Vec<DecryptedPrivateTallyProposal>) -> Self {
-        Self {
-            inner: proposals.into_boxed_slice(),
+    pub fn new(
+        proposals: Vec<DecryptedPrivateTallyProposal>,
+    ) -> Result<Self, DecryptedPrivateTallyError> {
+        if proposals.iter().all(|proposal| {
+            let mut shares = proposal.decrypt_shares.iter();
+            match shares.next() {
+                Some(first_share) => shares.all(|share| share.options() == first_share.options()),
+                None => true,
+            }
+        }) {
+            Ok(Self {
+                inner: proposals.into_boxed_slice(),
+            })
+        } else {
+            Err(DecryptedPrivateTallyError {})
         }
     }
 
@@ -280,7 +297,8 @@ impl Readable for VoteTally {
                 }
 
                 VoteTallyPayload::Private {
-                    inner: DecryptedPrivateTally::new(proposals),
+                    inner: DecryptedPrivateTally::new(proposals)
+                        .map_err(|err| ReadError::InvalidData(err.to_string()))?,
                 }
             }
         };

chain-impl-mockchain/src/testing/builders/vote.rs

@@ -1,5 +1,7 @@
 use crate::{
-    certificate::{DecryptedPrivateTally, DecryptedPrivateTallyProposal},
+    certificate::{
+        DecryptedPrivateTally, DecryptedPrivateTallyError, DecryptedPrivateTallyProposal,
+    },
     testing::data::CommitteeMembersManager,
     vote::VotePlanStatus,
 };
@@ -9,7 +11,7 @@ use rand::thread_rng;
 pub fn decrypt_tally(
     vote_plan_status: &VotePlanStatus,
     members: &CommitteeMembersManager,
-) -> DecryptedPrivateTally {
+) -> Result<DecryptedPrivateTally, DecryptedPrivateTallyError> {
     let encrypted_tally = vote_plan_status
         .proposals
         .iter()

chain-impl-mockchain/src/testing/e2e/vote_private.rs

@@ -88,7 +88,7 @@ pub fn private_vote_cast_action_transfer_to_rewards_all_shares() {
         })
         .unwrap();
 
-    let shares = decrypt_tally(vote_plan_status, &members);
+    let shares = decrypt_tally(vote_plan_status, &members).unwrap();
 
     controller
         .tally_vote_private(&alice, &vote_plan, shares, &mut ledger)

chain-vote/src/tally.rs

@@ -278,6 +278,11 @@ impl TallyDecryptShare {
     /// Given the member's public key `MemberPublicKey`, and the `EncryptedTally`, verifies the
     /// correctness of the `TallyDecryptShare`.
     pub fn verify(&self, encrypted_tally: &EncryptedTally, pk: &MemberPublicKey) -> bool {
+        // elements amount of the tally share should be equal to the number of the existing options of the tally
+        if self.options() != encrypted_tally.r.len() {
+            return false;
+        }
+
         for (element, r) in self.elements.iter().zip(encrypted_tally.r.iter()) {
             if !element.pi.verify(r, &element.r1, &pk.0) {
                 return false;
@@ -647,6 +652,43 @@ mod tests {
         );
     }
 
+    #[test]
+    fn tally_wrong_elements_size() {
+        let mut rng = ChaCha20Rng::from_seed([0u8; 32]);
+
+        let shared_string =
+            b"Example of a shared string. This should be VotePlan.to_id()".to_owned();
+        let h = Crs::from_hash(&shared_string);
+
+        let mc = MemberCommunicationKey::new(&mut rng);
+        let mc = [mc.to_public()];
+
+        let threshold = 1;
+
+        let m = MemberState::new(&mut rng, threshold, &h, &mc, 0);
+
+        let participants = vec![m.public_key()];
+        let ek = ElectionPublicKey::from_participants(&participants);
+
+        println!("encrypting vote");
+
+        let vote_options = 2;
+        let e = get_encrypted_ballot(&mut rng, &ek, &h, Vote::new(vote_options, 0));
+
+        println!("tallying");
+
+        let mut encrypted_tally = EncryptedTally::new(vote_options, ek, h);
+        encrypted_tally.add(&e, 1);
+
+        let mut tds = encrypted_tally.partial_decrypt(&mut rng, m.secret_key());
+
+        println!("corrupt tally share, add extra element");
+
+        tds.elements.push(tds.elements.last().unwrap().clone());
+
+        assert!(!tds.verify(&encrypted_tally, &m.public_key()))
+    }
+
     #[test]
     fn zero_encrypted_tally_serialization_sanity() {
         let election_key = ElectionPublicKey(PublicKey {