From d97be2d8ef2302cf9a6f4c5482150bf2c7f35d7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C4=99drzej=20Stuczy=C5=84ski?= Date: Mon, 1 Sep 2025 11:30:35 +0100 Subject: [PATCH] bugfix: Recipient deserialisation for deserialisers missing bytes specialisation (#5991) * bugfix: Recipient deserialisation for deserialisers missing bytes specialisation for example toml or json will just default to visit_seq ignoring bytes related optimisations * clippy --- Cargo.lock | 27 ++----- common/nymsphinx/addressing/Cargo.toml | 3 + common/nymsphinx/addressing/src/clients.rs | 88 +++++++++++++++++++++- common/nymsphinx/types/src/lib.rs | 2 + 4 files changed, 99 insertions(+), 21 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6ae018e429..f7e6e957aa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2150,8 +2150,8 @@ checksum = "2a2330da5de22e8a3cb63252ce2abb30116bf5265e89c0e01bc17015ce30a476" [[package]] name = "defguard_wireguard_rs" -version = "0.7.5" -source = "git+https://github.com/DefGuard/wireguard-rs.git?rev=v0.7.5#d090d2249e5bb3d4154f07de098387e2ab69bfdc" +version = "0.4.7" +source = "git+https://github.com/DefGuard/wireguard-rs.git?rev=v0.4.7#ef1cf3714629bf5016fb38cbb7320451dc69fb09" dependencies = [ "base64 0.22.1", "libc", @@ -2162,10 +2162,9 @@ dependencies = [ "netlink-packet-utils", "netlink-packet-wireguard", "netlink-sys", - "nix 0.30.1", + "nix 0.29.0", "serde", - "thiserror 2.0.12", - "x25519-dalek", + "thiserror 1.0.69", ] [[package]] @@ -4660,9 +4659,9 @@ dependencies = [ [[package]] name = "netlink-packet-route" -version = "0.22.0" +version = "0.20.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fc0e7987b28514adf555dc1f9a5c30dfc3e50750bbaffb1aec41ca7b23dcd8e4" +checksum = "55e5bda7ca0f9ac5e75b5debac3b75e29a8ac8e2171106a2c3bb466389a8dd83" dependencies = [ "anyhow", "bitflags 2.9.1", @@ -4741,18 +4740,6 @@ name = "nix" version = "0.29.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "71e2746dc3a24dd78b3cfcb7be93368c6de9963d30f43a6a73998a9cf4b17b46" -dependencies = [ - "bitflags 2.9.1", - "cfg-if", - "cfg_aliases", - "libc", -] - -[[package]] -name = "nix" -version = "0.30.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "74523f3a35e05aba87a1d978330aef40f67b0304ac79c1c00b294c9830543db6" dependencies = [ "bitflags 2.9.1", "cfg-if", @@ -6902,10 +6889,12 @@ dependencies = [ name = "nym-sphinx-addressing" version = "0.1.0" dependencies = [ + "bincode", "nym-crypto", "nym-sphinx-types", "rand 0.8.5", "serde", + "serde_json", "thiserror 2.0.12", ] diff --git a/common/nymsphinx/addressing/Cargo.toml b/common/nymsphinx/addressing/Cargo.toml index 03ec4d94af..c7f756cb5c 100644 --- a/common/nymsphinx/addressing/Cargo.toml +++ b/common/nymsphinx/addressing/Cargo.toml @@ -16,3 +16,6 @@ thiserror = { workspace = true } [dev-dependencies] rand = { workspace = true } nym-crypto = { path = "../../crypto", features = ["rand"] } +bincode = { workspace = true } +serde_json = { workspace = true } +serde = { workspace = true, features = ["derive"] } \ No newline at end of file diff --git a/common/nymsphinx/addressing/src/clients.rs b/common/nymsphinx/addressing/src/clients.rs index 0b1bcf7623..aa475dc401 100644 --- a/common/nymsphinx/addressing/src/clients.rs +++ b/common/nymsphinx/addressing/src/clients.rs @@ -7,7 +7,7 @@ use crate::nodes::{NodeIdentity, NODE_IDENTITY_SIZE}; use nym_crypto::asymmetric::{ed25519, x25519}; use nym_sphinx_types::Destination; -use serde::de::{Error as SerdeError, Unexpected, Visitor}; +use serde::de::{Error as SerdeError, SeqAccess, Unexpected, Visitor}; use serde::{Deserialize, Deserializer, Serialize, Serializer}; use std::fmt::{self, Formatter}; use std::str::FromStr; @@ -64,7 +64,7 @@ impl<'de> Deserialize<'de> for Recipient { { struct RecipientVisitor; - impl Visitor<'_> for RecipientVisitor { + impl<'de> Visitor<'de> for RecipientVisitor { type Value = Recipient; fn expecting(&self, formatter: &mut Formatter<'_>) -> fmt::Result { @@ -90,6 +90,42 @@ impl<'de> Deserialize<'de> for Recipient { ) }) } + + fn visit_seq(self, mut seq: A) -> Result + where + A: SeqAccess<'de>, + { + // if we know the size hint, check if it matches expectation, + // otherwise return an error + if let Some(size_hint) = seq.size_hint() { + if size_hint != Recipient::LEN { + return Err(SerdeError::invalid_length(size_hint, &self)); + } + } + + let mut recipient_bytes = [0u8; Recipient::LEN]; + + // clippy's suggestion is completely wrong and it iterates wrong sequence + #[allow(clippy::needless_range_loop)] + for i in 0..Recipient::LEN { + let Some(elem) = seq.next_element::()? else { + return Err(SerdeError::invalid_length(i + 1, &self)); + }; + recipient_bytes[i] = elem; + } + + // make sure there are no trailing bytes + if seq.next_element::()?.is_some() { + return Err(SerdeError::invalid_length(Recipient::LEN + 1, &self)); + } + + Recipient::try_from_bytes(recipient_bytes).map_err(|_| { + SerdeError::invalid_value( + Unexpected::Other("At least one of the curve points was malformed"), + &self, + ) + }) + } } deserializer.deserialize_bytes(RecipientVisitor) @@ -245,6 +281,18 @@ impl FromStr for Recipient { mod tests { use super::*; + fn mock_recipient() -> Recipient { + Recipient::try_from_bytes([ + 67, 5, 132, 146, 3, 236, 116, 89, 254, 57, 131, 159, 69, 181, 55, 208, 12, 108, 136, + 83, 58, 76, 171, 195, 31, 98, 92, 64, 68, 53, 156, 184, 100, 189, 73, 3, 238, 103, 156, + 108, 124, 199, 42, 79, 172, 98, 81, 177, 182, 100, 167, 164, 74, 183, 199, 213, 162, + 173, 102, 112, 30, 159, 148, 66, 44, 75, 230, 182, 138, 114, 170, 163, 209, 82, 204, + 100, 118, 91, 57, 150, 212, 147, 151, 135, 148, 16, 213, 223, 182, 164, 242, 37, 40, + 73, 137, 228, + ]) + .unwrap() + } + #[test] fn string_conversion_works() { let mut rng = rand::thread_rng(); @@ -308,4 +356,40 @@ mod tests { recovered_recipient.gateway.to_bytes() ); } + + // calls `visit_bytes` + #[test] + fn bincode_serialisation_works() { + let recipient = mock_recipient(); + + #[derive(Serialize, Deserialize, Clone, Copy, Debug, PartialEq)] + struct MyStruct { + recipient: Recipient, + } + let a = MyStruct { recipient }; + let s = bincode::serialize(&a).unwrap(); + + let b = bincode::deserialize(&s).unwrap(); + + assert_eq!(a, b); + } + + // calls `visit_seq` + #[test] + fn json_serialisation_works() { + use serde::{Deserialize, Serialize}; + + let recipient = mock_recipient(); + + #[derive(Serialize, Deserialize, Clone, Copy, Debug, PartialEq)] + struct MyStruct { + recipient: Recipient, + } + let a = MyStruct { recipient }; + let s = serde_json::to_string(&a).unwrap(); + + let b = serde_json::from_str(&s).unwrap(); + + assert_eq!(a, b); + } } diff --git a/common/nymsphinx/types/src/lib.rs b/common/nymsphinx/types/src/lib.rs index 5ecd5c3fb1..aa1a82b848 100644 --- a/common/nymsphinx/types/src/lib.rs +++ b/common/nymsphinx/types/src/lib.rs @@ -180,6 +180,7 @@ impl NymPacket { } #[cfg(feature = "sphinx")] + #[allow(unreachable_patterns)] pub fn sphinx_packet_ref(&self) -> Option<&SphinxPacket> { match self { NymPacket::Sphinx(packet) => Some(packet), @@ -188,6 +189,7 @@ impl NymPacket { } #[cfg(feature = "sphinx")] + #[allow(unreachable_patterns)] pub fn to_sphinx_packet(self) -> Option { match self { NymPacket::Sphinx(packet) => Some(packet),