Skip to content

Commit

Permalink
Remove serde from dkg
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
kayabaNerve committed Dec 10, 2023
1 parent ee84753 commit ef26b66
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 8 deletions.
1 change: 0 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions crypto/dkg/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
Expand All @@ -47,7 +46,6 @@ std = [
"std-shims/std",

"borsh?/std",
"serde?/std",

"transcript/std",
"chacha20/std",
Expand All @@ -61,6 +59,5 @@ std = [
"dleq/serialize"
]
borsh = ["dep:borsh"]
serde = ["dep:serde"]
tests = ["rand_core/getrandom"]
default = ["std"]
24 changes: 20 additions & 4 deletions crypto/dkg/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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>,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)));
Expand Down

0 comments on commit ef26b66

Please sign in to comment.