From cfeb874fdacdb2ccfbada888f29e5ede09a7727a Mon Sep 17 00:00:00 2001 From: oars Date: Sun, 30 Mar 2025 17:54:33 +0300 Subject: [PATCH] net: configure maximum message name length to prevent possible dos vector --- src/net/channel.rs | 17 ++++++++++++++--- src/net/message.rs | 7 +++++-- src/net/message_publisher.rs | 2 +- src/net/metering.rs | 2 +- 4 files changed, 21 insertions(+), 7 deletions(-) diff --git a/src/net/channel.rs b/src/net/channel.rs index 37ba6052c..573d41663 100644 --- a/src/net/channel.rs +++ b/src/net/channel.rs @@ -29,6 +29,7 @@ use std::{ use darkfi_serial::{ async_trait, AsyncDecodable, AsyncEncodable, SerialDecodable, SerialEncodable, VarInt, }; + use log::{debug, error, info, trace, warn}; use rand::{rngs::OsRng, Rng}; use smol::{ @@ -42,7 +43,7 @@ use super::{ dnet::{self, dnetev, DnetEvent}, hosts::{HostColor, HostsPtr}, message, - message::{SerializedMessage, VersionMessage}, + message::{SerializedMessage, VersionMessage, MAX_COMMAND_LENGTH}, message_publisher::{MessageSubscription, MessageSubsystem}, metering::{MeteringConfiguration, MeteringQueue}, p2p::P2pPtr, @@ -207,7 +208,7 @@ impl Channel { /// Sends the encoded payload of provided `SerializedMessage` across the channel. /// /// We first check if we should apply some throttling, based on the provided - /// `Message` configuration. We always sleep 2x times more that the exepted one, + /// `Message` configuration. We always sleep 2x times more than the expected one, /// so we don't flood the peer. /// Then, calls `send_message` that creates a new payload and sends it over the /// network transport as a packet. @@ -334,6 +335,11 @@ impl Channel { // First extract the length from the stream let cmd_len = VarInt::decode_async(stream).await?.0; + if cmd_len > (MAX_COMMAND_LENGTH as u64) { + error!(target: "net::channel::read_command", + "Error: Command length ({cmd_len}) exceeds configured limit ({MAX_COMMAND_LENGTH}). Dropping..."); + return Err(Error::MessageInvalid); + } // Then extract precisely `cmd_len` items from the stream. let mut take = stream.take(cmd_len); @@ -401,6 +407,11 @@ impl Channel { "[P2P] Channel {} disconnected", self.address() ); + } else if let Error::MessageInvalid = err { + // The command name length has exceeded the limit, this is possibly a malicious attack so ban it + if let BanPolicy::Strict = self.p2p().settings().read().await.ban_policy { + self.ban().await; + } } else if self.session.upgrade().unwrap().type_id() & (SESSION_ALL & !SESSION_REFINE) != 0 @@ -451,7 +462,7 @@ impl Channel { if self.session.upgrade().unwrap().type_id() != SESSION_REFINE { warn!( target: "net::channel::main_receive_loop()", - "MissingDispatcher|MessageInvalid|MeteringLimitExcheeded for command={command}, channel={self:?}" + "MissingDispatcher|MessageInvalid|MeteringLimitExceeded for command={command}, channel={self:?}" ); if let BanPolicy::Strict = self.p2p().settings().read().await.ban_policy { diff --git a/src/net/message.rs b/src/net/message.rs index c0a2c340c..b496323e8 100644 --- a/src/net/message.rs +++ b/src/net/message.rs @@ -63,6 +63,9 @@ macro_rules! impl_p2p_message { }; } +/// Maximum command (message name) length in bytes +pub const MAX_COMMAND_LENGTH: u8 = 255; + /// Outbound keepalive message. #[derive(Debug, Copy, Clone, SerialEncodable, SerialDecodable)] pub struct PingMessage { @@ -77,12 +80,12 @@ pub struct PongMessage { } impl_p2p_message!(PongMessage, "pong", 0, 0, DEFAULT_METERING_CONFIGURATION); -/// Requests address of outbound connecction. +/// Requests address of outbound connection. #[derive(Debug, Clone, SerialEncodable, SerialDecodable)] pub struct GetAddrsMessage { /// Maximum number of addresses with preferred /// transports to receive. Response vector will - /// also containg addresses without the preferred + /// also contain addresses without the preferred /// transports, so its size will be 2 * max. pub max: u32, /// Preferred addresses transports diff --git a/src/net/message_publisher.rs b/src/net/message_publisher.rs index ea88c52ad..8049e7f15 100644 --- a/src/net/message_publisher.rs +++ b/src/net/message_publisher.rs @@ -241,7 +241,7 @@ impl MessageDispatcherInterface for MessageDispatcher { /// from an inbound stream. /// /// We extract the message length from the stream and use `take()` - /// to allocate an appropiately sized buffer as a basic DDOS protection. + /// to allocate an appropriately sized buffer as a basic DDOS protection. async fn trigger( &self, stream: &mut smol::io::ReadHalf>, diff --git a/src/net/metering.rs b/src/net/metering.rs index 9dabd8735..4b0bfb753 100644 --- a/src/net/metering.rs +++ b/src/net/metering.rs @@ -114,7 +114,7 @@ impl MeteringQueue { /// Add new metering value to the queue, after /// prunning expired metering information. - /// If no thresshold has been set, the insert is + /// If no threshold has been set, the insert is /// ignored. pub fn push(&mut self, value: &u64) { // Check if threshold has been set