Add a filter for memo and receiver field size (informalsystems#3765)

* New configuration for max memo and receiver size

* Do not relay packets which have memo or receiver field too big

* Update defaults

* Rename configurations max_memo_size and max_receiver_size to ics20_max_memo_size and ics20_max_receiver_size

* Improve max memo and receiver configuration

* Add changelog entry

* Change type of ics20_max_memo_size and ics20_max_receiver_size to byte_unit Byte type

* Pass LinkParameters to new method for Link and RelayPath

* Refactor memo and receiver field validation for ICS20 packets

* Update crates/relayer-types/src/core/ics04_channel/packet.rs

Co-authored-by: Romain Ruetschi <[email protected]>
Signed-off-by: Luca Joss <[email protected]>

* Remove unnecessary errors and error maping

* Apply suggestions from code review

Co-authored-by: Romain Ruetschi <[email protected]>
Signed-off-by: Luca Joss <[email protected]>

* Change max_memo_size and max_receiver_size fields from u64 to usize

* Improve doc for 'are_fields_valid' method

* Improve logging when validating memo and receiver fields

* Apply suggestions from code review

Co-authored-by: Romain Ruetschi <[email protected]>
Signed-off-by: Luca Joss <[email protected]>

* Refactor ICS20 memo and receiver field configuration and validation

* Improve documentation of memo and receiver filter configuration

* Fix clippy error

* Add test for memo filter

* Formatting

* Revert formatting change to otherwise untouched file

* Simplify ICS-20 checks a little bit

* Perform ICS-20 checks on all packet events

* Improve comment in memo filter test

---------

Signed-off-by: Luca Joss <[email protected]>
Co-authored-by: Romain Ruetschi <[email protected]>

.changelog/unreleased/features/ibc-relayer/3766-max-memo-receiver-config.md

@@ -0,0 +1,6 @@
+- Add two new packet configurations:
+    * `ics20_max_memo_size` which filters ICS20 packets with memo
+      field bigger than the configured value
+    * `ics20_max_receiver_size` which filters ICS20 packets with receiver
+      field bigger than the configured value
+  ([\#3766](https://github.com/informalsystems/hermes/issues/3766))

config.toml

@@ -79,6 +79,20 @@ tx_confirmation = false
 # [Default: false]
 auto_register_counterparty_payee = false
 
+# Set the maximum size for the memo field in ICS20 packets.
+# If the size of the memo field is bigger than the configured
+# one, the packet will not be relayed.
+# The filter can be disabled by setting `enabled = false`.
+# [Default: "32KiB"]
+#ics20_max_memo_size = { enabled = true, size = "32KiB" }
+
+# Set the maximum size for the receiver field in ICS20 packets.
+# If the size of the receiver field is bigger than the configured
+# one, the packet will not be relayed.
+# The filter can be disabled by setting `enabled = false`.
+# [Default: "2KiB"]
+#ics20_max_receiver_size = { enabled = true, size = "2KiB" }
+
 # The REST section defines parameters for Hermes' built-in RESTful API.
 # https://hermes.informal.systems/rest.html
 [rest]

crates/relayer-cli/src/commands/clear.rs

@@ -152,6 +152,8 @@ impl Runnable for ClearPacketsCmd {
         let opts = LinkParameters {
             src_port_id: self.port_id.clone(),
             src_channel_id: self.channel_id.clone(),
+            max_memo_size: config.mode.packets.ics20_max_memo_size,
+            max_receiver_size: config.mode.packets.ics20_max_receiver_size,
         };
 
         let fwd_link = match Link::new_from_opts(chains.src.clone(), chains.dst, opts, false, false)

crates/relayer-cli/src/commands/tx/packet.rs

@@ -86,6 +86,8 @@ impl Runnable for TxPacketRecvCmd {
         let opts = LinkParameters {
             src_port_id: self.src_port_id.clone(),
             src_channel_id: self.src_channel_id.clone(),
+            max_memo_size: config.mode.packets.ics20_max_memo_size,
+            max_receiver_size: config.mode.packets.ics20_max_receiver_size,
         };
         let link = match Link::new_from_opts(chains.src, chains.dst, opts, false, false) {
             Ok(link) => link,
@@ -181,6 +183,8 @@ impl Runnable for TxPacketAckCmd {
         let opts = LinkParameters {
             src_port_id: self.src_port_id.clone(),
             src_channel_id: self.src_channel_id.clone(),
+            max_memo_size: config.mode.packets.ics20_max_memo_size,
+            max_receiver_size: config.mode.packets.ics20_max_receiver_size,
         };
         let link = match Link::new_from_opts(chains.src, chains.dst, opts, false, false) {
             Ok(link) => link,

crates/relayer/src/config.rs

@@ -26,6 +26,7 @@ use ibc_relayer_types::core::ics24_host::identifier::{ChainId, ChannelId, PortId
 use ibc_relayer_types::timestamp::ZERO_DURATION;
 
 use crate::chain::cosmos::config::CosmosSdkConfig;
+use crate::config::types::ics20_field_size_limit::Ics20FieldSizeLimit;
 use crate::config::types::TrustThreshold;
 use crate::error::Error as RelayerError;
 use crate::extension_options::ExtensionOptionDynamicFeeTx;
@@ -230,6 +231,14 @@ pub mod default {
             buckets: 10,
         }
     }
+
+    pub fn ics20_max_memo_size() -> Ics20FieldSizeLimit {
+        Ics20FieldSizeLimit::new(true, Byte::from_bytes(32768))
+    }
+
+    pub fn ics20_max_receiver_size() -> Ics20FieldSizeLimit {
+        Ics20FieldSizeLimit::new(true, Byte::from_bytes(2048))
+    }
 }
 
 #[derive(Clone, Debug, Default, Deserialize, Serialize)]
@@ -400,6 +409,10 @@ pub struct Packets {
     pub tx_confirmation: bool,
     #[serde(default = "default::auto_register_counterparty_payee")]
     pub auto_register_counterparty_payee: bool,
+    #[serde(default = "default::ics20_max_memo_size")]
+    pub ics20_max_memo_size: Ics20FieldSizeLimit,
+    #[serde(default = "default::ics20_max_receiver_size")]
+    pub ics20_max_receiver_size: Ics20FieldSizeLimit,
 }
 
 impl Default for Packets {
@@ -410,6 +423,8 @@ impl Default for Packets {
             clear_on_start: default::clear_on_start(),
             tx_confirmation: default::tx_confirmation(),
             auto_register_counterparty_payee: default::auto_register_counterparty_payee(),
+            ics20_max_memo_size: default::ics20_max_memo_size(),
+            ics20_max_receiver_size: default::ics20_max_receiver_size(),
         }
     }
 }
@@ -742,6 +757,7 @@ impl<E: Into<Error>> From<CosmosConfigDiagnostic<E>> for Diagnostic<Error> {
 }
 
 use crate::chain::cosmos::config::error::Error as CosmosConfigError;
+
 impl From<CosmosConfigError> for Error {
     fn from(error: CosmosConfigError) -> Error {
         Error::cosmos_config_error(error.to_string())

crates/relayer/src/config/types.rs

@@ -125,6 +125,10 @@ pub mod max_tx_size {
             Ok(Self(value))
         }
 
+        pub fn unsafe_new(value: usize) -> Self {
+            Self(value)
+        }
+
         pub fn max() -> Self {
             Self(Self::MAX_BOUND)
         }
@@ -261,6 +265,61 @@ pub mod memo {
     }
 }
 
+pub mod ics20_field_size_limit {
+    use byte_unit::Byte;
+    use serde_derive::{Deserialize, Serialize};
+    use std::fmt::Display;
+
+    pub enum ValidationResult {
+        Valid,
+        Invalid { size: usize, max: usize },
+    }
+
+    impl Display for ValidationResult {
+        fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+            match self {
+                Self::Valid => write!(f, "valid"),
+
+                Self::Invalid { size, max } => {
+                    write!(f, "invalid, size `{size}` is greater than max `{max}`")
+                }
+            }
+        }
+    }
+
+    #[derive(Clone, Copy, Debug, Deserialize, Eq, PartialEq, Serialize)]
+    pub struct Ics20FieldSizeLimit {
+        enabled: bool,
+        size: Byte,
+    }
+
+    impl Ics20FieldSizeLimit {
+        pub fn new(enabled: bool, size: Byte) -> Self {
+            Self { enabled, size }
+        }
+
+        /// If the limit is disabled consider the field as valid.
+        /// If the limit is enabled assert the field is smaller or equal
+        /// to the configured value.
+        pub fn check_field_size(&self, field: &str) -> ValidationResult {
+            if self.enabled {
+                let size_limit = self.size.get_bytes() as usize;
+
+                if field.len() <= size_limit {
+                    ValidationResult::Valid
+                } else {
+                    ValidationResult::Invalid {
+                        size: field.len(),
+                        max: size_limit,
+                    }
+                }
+            } else {
+                ValidationResult::Valid
+            }
+        }
+    }
+}
+
 #[cfg(test)]
 #[allow(dead_code)] // the fields of the structs defined below are never accessed
 mod tests {

crates/relayer/src/link.rs

@@ -5,11 +5,14 @@ use ibc_relayer_types::core::{
 };
 use tracing::info;
 
-use crate::chain::requests::{QueryChannelRequest, QueryHeight};
 use crate::chain::{counterparty::check_channel_counterparty, requests::QueryConnectionRequest};
 use crate::chain::{handle::ChainHandle, requests::IncludeProof};
 use crate::channel::{Channel, ChannelSide};
 use crate::link::error::LinkError;
+use crate::{
+    chain::requests::{QueryChannelRequest, QueryHeight},
+    config::types::ics20_field_size_limit::Ics20FieldSizeLimit,
+};
 
 pub mod cli;
 pub mod error;
@@ -33,6 +36,8 @@ pub use relay_path::{RelayPath, Resubmit};
 pub struct LinkParameters {
     pub src_port_id: PortId,
     pub src_channel_id: ChannelId,
+    pub max_memo_size: Ics20FieldSizeLimit,
+    pub max_receiver_size: Ics20FieldSizeLimit,
 }
 
 pub struct Link<ChainA: ChainHandle, ChainB: ChainHandle> {
@@ -43,9 +48,10 @@ impl<ChainA: ChainHandle, ChainB: ChainHandle> Link<ChainA, ChainB> {
     pub fn new(
         channel: Channel<ChainA, ChainB>,
         with_tx_confirmation: bool,
+        link_parameters: LinkParameters,
     ) -> Result<Self, LinkError> {
         Ok(Self {
-            a_to_b: RelayPath::new(channel, with_tx_confirmation)?,
+            a_to_b: RelayPath::new(channel, with_tx_confirmation, link_parameters)?,
         })
     }
 
@@ -140,7 +146,7 @@ impl<ChainA: ChainHandle, ChainB: ChainHandle> Link<ChainA, ChainB> {
                 a_connection.client_id().clone(),
                 a_connection_id,
                 opts.src_port_id.clone(),
-                Some(opts.src_channel_id),
+                Some(opts.src_channel_id.clone()),
                 None,
             ),
             b_side: ChannelSide::new(
@@ -169,7 +175,7 @@ impl<ChainA: ChainHandle, ChainB: ChainHandle> Link<ChainA, ChainB> {
                 .map_err(LinkError::relayer)?;
         }
 
-        Link::new(channel, with_tx_confirmation)
+        Link::new(channel, with_tx_confirmation, opts)
     }
 
     /// Constructs a link around the channel that is reverse to the channel
@@ -182,6 +188,8 @@ impl<ChainA: ChainHandle, ChainB: ChainHandle> Link<ChainA, ChainB> {
         let opts = LinkParameters {
             src_port_id: self.a_to_b.dst_port_id().clone(),
             src_channel_id: self.a_to_b.dst_channel_id().clone(),
+            max_memo_size: self.a_to_b.max_memo_size,
+            max_receiver_size: self.a_to_b.max_receiver_size,
         };
         let chain_b = self.a_to_b.dst_chain().clone();
         let chain_a = self.a_to_b.src_chain().clone();

crates/relayer/src/link/relay_path.rs

@@ -4,6 +4,7 @@ use std::ops::Sub;
 use std::time::{Duration, Instant};
 
 use ibc_proto::google::protobuf::Any;
+use ibc_proto::ibc::applications::transfer::v2::FungibleTokenPacketData as RawPacketData;
 use itertools::Itertools;
 use tracing::{debug, error, info, span, trace, warn, Level};
 
@@ -42,6 +43,8 @@ use crate::chain::tracking::TrackedMsgs;
 use crate::chain::tracking::TrackingId;
 use crate::channel::error::ChannelError;
 use crate::channel::Channel;
+use crate::config::types::ics20_field_size_limit::Ics20FieldSizeLimit;
+use crate::config::types::ics20_field_size_limit::ValidationResult;
 use crate::event::source::EventBatch;
 use crate::event::IbcEventWithHeight;
 use crate::foreign_client::{ForeignClient, ForeignClientError};
@@ -55,6 +58,7 @@ use crate::link::packet_events::query_write_ack_events;
 use crate::link::pending::PendingTxs;
 use crate::link::relay_sender::{AsyncReply, SubmitReply};
 use crate::link::relay_summary::RelaySummary;
+use crate::link::LinkParameters;
 use crate::link::{pending, relay_sender};
 use crate::path::PathIdentifiers;
 use crate::telemetry;
@@ -108,12 +112,16 @@ pub struct RelayPath<ChainA: ChainHandle, ChainB: ChainHandle> {
     // transactions if [`confirm_txes`] is true.
     pending_txs_src: PendingTxs<ChainA>,
     pending_txs_dst: PendingTxs<ChainB>,
+
+    pub max_memo_size: Ics20FieldSizeLimit,
+    pub max_receiver_size: Ics20FieldSizeLimit,
 }
 
 impl<ChainA: ChainHandle, ChainB: ChainHandle> RelayPath<ChainA, ChainB> {
     pub fn new(
         channel: Channel<ChainA, ChainB>,
         with_tx_confirmation: bool,
+        link_parameters: LinkParameters,
     ) -> Result<Self, LinkError> {
         let src_chain = channel.src_chain().clone();
         let dst_chain = channel.dst_chain().clone();
@@ -152,6 +160,9 @@ impl<ChainA: ChainHandle, ChainB: ChainHandle> RelayPath<ChainA, ChainB> {
             confirm_txes: with_tx_confirmation,
             pending_txs_src: PendingTxs::new(src_chain, src_channel_id, src_port_id, dst_chain_id),
             pending_txs_dst: PendingTxs::new(dst_chain, dst_channel_id, dst_port_id, src_chain_id),
+
+            max_memo_size: link_parameters.max_memo_size,
+            max_receiver_size: link_parameters.max_receiver_size,
         })
     }
 
@@ -549,6 +560,18 @@ impl<ChainA: ChainHandle, ChainB: ChainHandle> RelayPath<ChainA, ChainB> {
         for event_with_height in input {
             trace!(event = %event_with_height, "processing event");
 
+            if let Some(packet) = event_with_height.event.packet() {
+                // If the event is a ICS-04 packet event, and the packet contains ICS-20
+                // packet data, check that the ICS-20 fields are within the configured limits.
+                if !check_ics20_fields_size(
+                    &packet.data,
+                    self.max_memo_size,
+                    self.max_receiver_size,
+                ) {
+                    continue;
+                }
+            }
+
             let (dst_msg, src_msg) = match &event_with_height.event {
                 IbcEvent::CloseInitChannel(_) => (
                     self.build_chan_close_confirm_from_event(event_with_height)?,
@@ -1363,6 +1386,7 @@ impl<ChainA: ChainHandle, ChainB: ChainHandle> RelayPath<ChainA, ChainB> {
         dst_info: &ChainStatus,
     ) -> Result<Option<Any>, LinkError> {
         let packet = event.packet.clone();
+
         if self
             .dst_channel(QueryHeight::Specific(dst_info.height))?
             .state_matches(&ChannelState::Closed)
@@ -1381,7 +1405,16 @@ impl<ChainA: ChainHandle, ChainB: ChainHandle> RelayPath<ChainA, ChainB> {
         dst_info: &ChainStatus,
         height: Height,
     ) -> Result<(Option<Any>, Option<Any>), LinkError> {
+        crate::time!(
+            "build_recv_or_timeout_from_send_packet_event",
+            {
+                "src_channel": event.packet.source_channel,
+                "dst_channel": event.packet.destination_channel,
+            }
+        );
+
         let timeout = self.build_timeout_from_send_packet_event(event, dst_info)?;
+
         if timeout.is_some() {
             Ok((None, timeout))
         } else {
@@ -1886,3 +1919,34 @@ impl<ChainA: ChainHandle, ChainB: ChainHandle> RelayPath<ChainA, ChainB> {
         }
     }
 }
+
+#[tracing::instrument(skip(data))]
+fn check_ics20_fields_size(
+    data: &[u8],
+    memo_limit: Ics20FieldSizeLimit,
+    receiver_limit: Ics20FieldSizeLimit,
+) -> bool {
+    match serde_json::from_slice::<RawPacketData>(data) {
+        Ok(packet_data) => {
+            match (
+                memo_limit.check_field_size(&packet_data.memo),
+                receiver_limit.check_field_size(&packet_data.receiver),
+            ) {
+                (ValidationResult::Valid, ValidationResult::Valid) => true,
+
+                (memo_validity, receiver_validity) => {
+                    debug!("found invalid ICS-20 packet data, not relaying packet!");
+                    debug!("    ICS-20 memo:     {memo_validity}");
+                    debug!("    ICS-20 receiver: {receiver_validity}");
+
+                    false
+                }
+            }
+        }
+        Err(e) => {
+            trace!("failed to decode ICS20 packet data with error `{e}`");
+
+            true
+        }
+    }
+}

crates/relayer/src/worker.rs

@@ -117,6 +117,8 @@ pub fn spawn_worker_tasks<ChainA: ChainHandle, ChainB: ChainHandle>(
                 LinkParameters {
                     src_port_id: path.src_port_id.clone(),
                     src_channel_id: path.src_channel_id.clone(),
+                    max_memo_size: packets_config.ics20_max_memo_size,
+                    max_receiver_size: packets_config.ics20_max_receiver_size,
                 },
                 packets_config.tx_confirmation,
                 packets_config.auto_register_counterparty_payee,