From e19b99112e3fe70cb92b09db9595d3e05ef26f7c Mon Sep 17 00:00:00 2001
From: Sarah Jamie Lewis <sarah@openprivacy.ca>
Date: Tue, 2 Feb 2021 18:20:45 -0800
Subject: [PATCH] Fix for Universal Tag Bug found by Lee Bousfield

---
 Cargo.toml |  2 +-
 README.md  |  3 ++-
 src/lib.rs | 46 ++++++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 47 insertions(+), 4 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index cd10165..2f3f579 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -1,7 +1,7 @@
 [package]
 name = "fuzzytags"
 description = "a probabilistic cryptographic structure for metadata resistant tagging"
-version = "0.2.1"
+version = "0.2.2"
 repository = "https://git.openprivacy.ca/openprivacy/fuzzytags"
 authors = ["Sarah Jamie Lewis <sarah@openprivacy.ca>"]
 edition = "2018"
diff --git a/README.md b/README.md
index b32b394..112aed2 100644
--- a/README.md
+++ b/README.md
@@ -149,4 +149,5 @@ For more guidance on integrating fuzzytags into a privacy preserving application
 ## Credits and Contributions
 
 - Based on [Fuzzy Message Detection](https://eprint.iacr.org/2021/089) by Gabrielle Beck and Julia Len and Ian Miers and Matthew Green
-- Performance & API improvements contributed by Henry de Valence
\ No newline at end of file
+- Performance & API improvements contributed by Henry de Valence
+- Universal Tag Bug found by [Lee Bousfield](https://github.com/PlasmaPower/)
\ No newline at end of file
diff --git a/src/lib.rs b/src/lib.rs
index 2d7a1cb..fbcf2f5 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -189,8 +189,15 @@ impl FuzzyDetectionKey {
     ///    }
     /// ```
     pub fn test_tag(&self, tag: &FuzzyTag) -> bool {
-        let m = FuzzySecretKey::g(tag.u, &tag.ciphertexts);
+        // A few checks to make sure the tag is well formed.
+        // All zeros in u or y can lead to a tag that validates against *all* public keys
+        // That doesn't seem like a great idea, so we return false to be safe.
+        // Zero values should never appear in well generated tags.
+        if tag.u.eq(&RistrettoPoint::default()) || tag.y.eq(&Scalar::zero()) {
+            return false;
+        }
 
+        let m = FuzzySecretKey::g(tag.u, &tag.ciphertexts);
         let g = RISTRETTO_BASEPOINT_POINT;
 
         // Re-derive w = g^z from the public tag.
@@ -402,7 +409,10 @@ impl FuzzyPublicKey {
 
 #[cfg(test)]
 mod tests {
-    use crate::FuzzySecretKey;
+    use crate::{FuzzySecretKey, FuzzyTag};
+    use bit_vec::BitVec;
+    use curve25519_dalek::ristretto::RistrettoPoint;
+    use curve25519_dalek::scalar::Scalar;
 
     #[test]
     fn test_serialization() {
@@ -440,6 +450,38 @@ mod tests {
         }
     }
 
+    fn gen_zero_tag_zero(max_gamma: usize) -> FuzzyTag {
+        let tag = FuzzyTag {
+            u: RistrettoPoint::default(),
+            y: Scalar::default(),
+            ciphertexts: BitVec::from_elem(max_gamma, false),
+        };
+        tag
+    }
+
+    fn gen_zero_tag_one(max_gamma: usize) -> FuzzyTag {
+        let mut tag = FuzzyTag {
+            u: RistrettoPoint::default(),
+            y: Scalar::default(),
+            ciphertexts: BitVec::from_elem(max_gamma, false),
+        };
+        tag.ciphertexts.set_all();
+        tag
+    }
+
+    #[test]
+    // Thanks to Lee Bousfield who noticed an all zeros or all ones tag would
+    // validate against a public key with 50% probability, allowing universal
+    // broadcast, which overall seems like a bad idea...
+    // Test to make sure that doesn't happen.
+    fn test_zero_tag() {
+        let secret_key = FuzzySecretKey::generate(24);
+        let tag = gen_zero_tag_zero(16);
+        assert_eq!(false, secret_key.extract(6).test_tag(&tag));
+        let tag = gen_zero_tag_one(16);
+        assert_eq!(false, secret_key.extract(6).test_tag(&tag));
+    }
+
     #[test]
     fn false_positives() {
         let gamma = 8;