From ef26b66d982d7e45eef54f0884795e978505372c Mon Sep 17 00:00:00 2001
From: Luke Parker <lukeparker5132@gmail.com>
Date: Sun, 10 Dec 2023 17:42:23 -0500
Subject: [PATCH] Remove serde from dkg

serde could be used to deserialize intenrally inconsistent objects which could
lead to panics or faults.

The BorshDeserialize derives have been replaced with a manual implementation
which won't produce inconsistent objects.
---
 Cargo.lock            |  1 -
 crypto/dkg/Cargo.toml |  3 ---
 crypto/dkg/src/lib.rs | 24 ++++++++++++++++++++----
 3 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 691c86df8..c8301b66b 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1571,7 +1571,6 @@ dependencies = [
  "multiexp",
  "rand_core",
  "schnorr-signatures",
- "serde",
  "std-shims",
  "thiserror",
  "zeroize",
diff --git a/crypto/dkg/Cargo.toml b/crypto/dkg/Cargo.toml
index 2b8cd25db..0eb3f5412 100644
--- a/crypto/dkg/Cargo.toml
+++ b/crypto/dkg/Cargo.toml
@@ -23,7 +23,6 @@ zeroize = { version = "^1.5", default-features = false, features = ["zeroize_der
 std-shims = { version = "0.1", path = "../../common/std-shims", default-features = false }
 
 borsh = { version = "1", default-features = false, features = ["derive", "de_strict_order"], optional = true }
-serde = { version = "1", default-features = false, features = ["derive"], optional = true }
 
 transcript = { package = "flexible-transcript", path = "../transcript", version = "^0.3.2", default-features = false, features = ["recommended"] }
 chacha20 = { version = "0.9", default-features = false, features = ["zeroize"] }
@@ -47,7 +46,6 @@ std = [
   "std-shims/std",
 
   "borsh?/std",
-  "serde?/std",
 
   "transcript/std",
   "chacha20/std",
@@ -61,6 +59,5 @@ std = [
   "dleq/serialize"
 ]
 borsh = ["dep:borsh"]
-serde = ["dep:serde"]
 tests = ["rand_core/getrandom"]
 default = ["std"]
diff --git a/crypto/dkg/src/lib.rs b/crypto/dkg/src/lib.rs
index fd49856f7..eb915e236 100644
--- a/crypto/dkg/src/lib.rs
+++ b/crypto/dkg/src/lib.rs
@@ -31,8 +31,7 @@ pub mod tests;
 
 /// The ID of a participant, defined as a non-zero u16.
 #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Zeroize)]
-#[cfg_attr(feature = "borsh", derive(borsh::BorshSerialize, borsh::BorshDeserialize))]
-#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
+#[cfg_attr(feature = "borsh", derive(borsh::BorshSerialize))]
 pub struct Participant(pub(crate) u16);
 impl Participant {
   /// Create a new Participant identifier from a u16.
@@ -118,6 +117,14 @@ mod lib {
     Ciphersuite,
   };
 
+  #[cfg(feature = "borsh")]
+  impl borsh::BorshDeserialize for Participant {
+    fn deserialize_reader<R: io::Read>(reader: &mut R) -> io::Result<Self> {
+      Participant::new(u16::deserialize_reader(reader)?)
+        .ok_or_else(|| io::Error::other("invalid participant"))
+    }
+  }
+
   // Validate a map of values to have the expected included participants
   pub(crate) fn validate_map<T, B: Clone + PartialEq + Eq + Debug>(
     map: &HashMap<Participant, T>,
@@ -147,8 +154,7 @@ mod lib {
   /// Parameters for a multisig.
   // These fields should not be made public as they should be static
   #[derive(Clone, Copy, PartialEq, Eq, Debug, Zeroize)]
-  #[cfg_attr(feature = "borsh", derive(borsh::BorshSerialize, borsh::BorshDeserialize))]
-  #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
+  #[cfg_attr(feature = "borsh", derive(borsh::BorshSerialize))]
   pub struct ThresholdParams {
     /// Participants needed to sign on behalf of the group.
     pub(crate) t: u16,
@@ -189,6 +195,16 @@ mod lib {
     }
   }
 
+  #[cfg(feature = "borsh")]
+  impl borsh::BorshDeserialize for ThresholdParams {
+    fn deserialize_reader<R: io::Read>(reader: &mut R) -> io::Result<Self> {
+      let t = u16::deserialize_reader(reader)?;
+      let n = u16::deserialize_reader(reader)?;
+      let i = Participant::deserialize_reader(reader)?;
+      ThresholdParams::new(t, n, i).map_err(|e| io::Error::other(format!("{e:?}")))
+    }
+  }
+
   /// Calculate the lagrange coefficient for a signing set.
   pub fn lagrange<F: PrimeField>(i: Participant, included: &[Participant]) -> F {
     let i_f = F::from(u64::from(u16::from(i)));