cosmos / ibc-rs

Rust implementation of the Inter-Blockchain Communication (IBC) protocol.
Apache License 2.0
182 stars 73 forks source link

Packet timeout isn’t checked when sending #1198

Closed mina86 closed 2 months ago

mina86 commented 2 months ago

According to ICS 04, sendPacket function must check whether either timeout height or timestamp is set. Currently neither send_packet nor send_transfer performs that check.

The condition is verified when converting RawPacket (RawMsgTransfer) into a Packet (a MsgTranfer) but that only happens when decoding a Protocol Message. JSON or Borsh encoded object won’t have this check performed. Not to mention that a Packet (a MsgTransfer) can be constructed directly with both of the timeouts cleared.

Solution appears to be to add the checks to send_packet_validate and send_transfer_validate functions. Note that even if the check is added to those functions the check in TryFrom<RawPacket> and TryFrom<RawMsgTransfer> should remain there since there may be existing code that relies on the conversions performing the verification.

rnbguy commented 2 months ago

JSON or Borsh encoded object won’t have this check performed.

This is a very nice find. I always wanted to add a Validatable trait that allows data validation of all domain types directly.

trait Validatable {
  fn validate(&self) -> bool;
}

Thanks for the report. :raised_hands: We will fix this soon.