Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a filter for memo and receiver field size #3765

Merged
merged 27 commits into from
Jan 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
05fe222
New configuration for max memo and receiver size
ljoss17 Jan 8, 2024
b7b9096
Do not relay packets which have memo or receiver field too big
ljoss17 Jan 8, 2024
3ea0c6f
Update defaults
ljoss17 Jan 10, 2024
72c32bd
Rename configurations max_memo_size and max_receiver_size to ics20_ma…
ljoss17 Jan 10, 2024
5e4aa5e
Improve max memo and receiver configuration
ljoss17 Jan 10, 2024
8e504b5
Add changelog entry
ljoss17 Jan 10, 2024
43a79a3
Change type of ics20_max_memo_size and ics20_max_receiver_size to byt…
ljoss17 Jan 10, 2024
f2b3a4f
Pass LinkParameters to new method for Link and RelayPath
ljoss17 Jan 10, 2024
b535b44
Refactor memo and receiver field validation for ICS20 packets
ljoss17 Jan 10, 2024
fecc450
Update crates/relayer-types/src/core/ics04_channel/packet.rs
ljoss17 Jan 10, 2024
508cb93
Remove unnecessary errors and error maping
ljoss17 Jan 10, 2024
f950ef9
Apply suggestions from code review
ljoss17 Jan 10, 2024
0fd448c
Change max_memo_size and max_receiver_size fields from u64 to usize
ljoss17 Jan 10, 2024
1f5534d
Improve doc for 'are_fields_valid' method
ljoss17 Jan 10, 2024
9af4e6d
Improve logging when validating memo and receiver fields
ljoss17 Jan 10, 2024
9d0eb86
Apply suggestions from code review
ljoss17 Jan 15, 2024
451b578
Refactor ICS20 memo and receiver field configuration and validation
ljoss17 Jan 15, 2024
dbbc53d
Improve documentation of memo and receiver filter configuration
ljoss17 Jan 15, 2024
c222b79
Merge branch 'master' into luca_joss/memo-and-receiver-filtering
ljoss17 Jan 15, 2024
3401a19
Fix clippy error
ljoss17 Jan 15, 2024
657dde1
Add test for memo filter
ljoss17 Jan 16, 2024
9db94e0
Formatting
romac Jan 16, 2024
76bb343
Merge branch 'master' into luca_joss/memo-and-receiver-filtering
romac Jan 16, 2024
bfe93f8
Revert formatting change to otherwise untouched file
romac Jan 16, 2024
06fe76a
Simplify ICS-20 checks a little bit
romac Jan 16, 2024
3d5687e
Perform ICS-20 checks on all packet events
romac Jan 16, 2024
41fe478
Improve comment in memo filter test
ljoss17 Jan 16, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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))
1 change: 1 addition & 0 deletions Cargo.lock

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

14 changes: 14 additions & 0 deletions config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
2 changes: 2 additions & 0 deletions crates/relayer-cli/src/commands/clear.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions crates/relayer-cli/src/commands/tx/packet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
16 changes: 16 additions & 0 deletions crates/relayer/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)]
Expand Down Expand Up @@ -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 {
Expand All @@ -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(),
}
}
}
Expand Down Expand Up @@ -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())
Expand Down
59 changes: 59 additions & 0 deletions crates/relayer/src/config/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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 {
Expand Down
16 changes: 12 additions & 4 deletions crates/relayer/src/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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> {
Expand All @@ -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)?,
})
}

Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand All @@ -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();
Expand Down
64 changes: 64 additions & 0 deletions crates/relayer/src/link/relay_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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};
Expand All @@ -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;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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,
})
}

Expand Down Expand Up @@ -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)?,
Expand Down Expand Up @@ -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)
Expand All @@ -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 {
Expand Down Expand Up @@ -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
}
}
}
2 changes: 2 additions & 0 deletions crates/relayer/src/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading
Loading