From 71cea7db097c1d0bcc9e560ee2387c071610b13b Mon Sep 17 00:00:00 2001
From: Shoaib Ahmed <sufialhussaini@gmail.com>
Date: Mon, 17 Jan 2022 21:02:53 +0530
Subject: [PATCH] Disallow empty `CommitmentPrefix` and `CommitmentProofBytes`
 (#1761)

* Disallow empty CommitmentPrefix and CommitmentProofBytes

* Fix errors in tests

* Fix failing tests

* Fix clippy errors

* Add .changelog entry

Co-authored-by: Romain Ruetschi <romain@informal.systems>
---
 ...allow-empty-commitment-prefix-and-proof.md |  2 ++
 relayer/src/chain.rs                          | 27 +++++++++++++------
 relayer/src/chain/cosmos.rs                   |  6 ++---
 3 files changed, 24 insertions(+), 11 deletions(-)
 create mode 100644 .changelog/unreleased/improvements/ibc/1761-disallow-empty-commitment-prefix-and-proof.md

diff --git a/.changelog/unreleased/improvements/ibc/1761-disallow-empty-commitment-prefix-and-proof.md b/.changelog/unreleased/improvements/ibc/1761-disallow-empty-commitment-prefix-and-proof.md
new file mode 100644
index 0000000000..d59818fd55
--- /dev/null
+++ b/.changelog/unreleased/improvements/ibc/1761-disallow-empty-commitment-prefix-and-proof.md
@@ -0,0 +1,2 @@
+- Disallow empty `CommitmentPrefix` and `CommitmentProofBytes`
+  ([#1761](https://github.com/informalsystems/ibc-rs/issues/1761))
\ No newline at end of file
diff --git a/relayer/src/chain.rs b/relayer/src/chain.rs
index 635a171bd7..b5b4ee2724 100644
--- a/relayer/src/chain.rs
+++ b/relayer/src/chain.rs
@@ -1,4 +1,7 @@
 use alloc::sync::Arc;
+use core::convert::TryFrom;
+
+use prost_types::Any;
 use tendermint::block::Height;
 use tokio::runtime::Runtime as TokioRuntime;
 
@@ -372,7 +375,10 @@ pub trait ChainEndpoint: Sized {
                 let (client_state_value, client_state_proof) =
                     self.proven_client_state(client_id, height)?;
 
-                client_proof = Some(CommitmentProofBytes::from(client_state_proof));
+                client_proof = Some(
+                    CommitmentProofBytes::try_from(client_state_proof)
+                        .map_err(Error::malformed_proof)?,
+                );
 
                 let consensus_state_proof = self
                     .proven_client_consensus(client_id, client_state_value.latest_height(), height)?
@@ -380,7 +386,8 @@ pub trait ChainEndpoint: Sized {
 
                 consensus_proof = Option::from(
                     ConsensusProof::new(
-                        CommitmentProofBytes::from(consensus_state_proof),
+                        CommitmentProofBytes::try_from(consensus_state_proof)
+                            .map_err(Error::malformed_proof)?,
                         client_state_value.latest_height(),
                     )
                     .map_err(Error::consensus_proof)?,
@@ -394,7 +401,7 @@ pub trait ChainEndpoint: Sized {
         Ok((
             client_state,
             Proofs::new(
-                CommitmentProofBytes::from(connection_proof),
+                CommitmentProofBytes::try_from(connection_proof).map_err(Error::malformed_proof)?,
                 client_proof,
                 consensus_proof,
                 None,
@@ -413,7 +420,8 @@ pub trait ChainEndpoint: Sized {
     ) -> Result<Proofs, Error> {
         // Collect all proofs as required
         let channel_proof =
-            CommitmentProofBytes::from(self.proven_channel(port_id, channel_id, height)?.1);
+            CommitmentProofBytes::try_from(self.proven_channel(port_id, channel_id, height)?.1)
+                .map_err(Error::malformed_proof)?;
 
         Proofs::new(channel_proof, None, None, None, height.increment())
             .map_err(Error::malformed_proof)
@@ -429,9 +437,12 @@ pub trait ChainEndpoint: Sized {
         height: ICSHeight,
     ) -> Result<(Vec<u8>, Proofs), Error> {
         let channel_proof = if packet_type == PacketMsgType::TimeoutOnClose {
-            Some(CommitmentProofBytes::from(
-                self.proven_channel(&port_id, &channel_id, height)?.1,
-            ))
+            Some(
+                CommitmentProofBytes::try_from(
+                    self.proven_channel(&port_id, &channel_id, height)?.1,
+                )
+                .map_err(Error::malformed_proof)?,
+            )
         } else {
             None
         };
@@ -440,7 +451,7 @@ pub trait ChainEndpoint: Sized {
             self.proven_packet(packet_type, port_id, channel_id, sequence, height)?;
 
         let proofs = Proofs::new(
-            CommitmentProofBytes::from(packet_proof),
+            CommitmentProofBytes::try_from(packet_proof).map_err(Error::malformed_proof)?,
             None,
             None,
             channel_proof,
diff --git a/relayer/src/chain/cosmos.rs b/relayer/src/chain/cosmos.rs
index a6e87e5118..4acf5145b0 100644
--- a/relayer/src/chain/cosmos.rs
+++ b/relayer/src/chain/cosmos.rs
@@ -39,6 +39,7 @@ use ibc::core::ics02_client::client_consensus::{
 };
 use ibc::core::ics02_client::client_state::{AnyClientState, IdentifiedAnyClientState};
 use ibc::core::ics02_client::client_type::ClientType;
+use ibc::core::ics02_client::error::Error as ClientError;
 use ibc::core::ics02_client::events as ClientEvents;
 use ibc::core::ics03_connection::connection::{ConnectionEnd, IdentifiedConnectionEnd};
 use ibc::core::ics04_channel;
@@ -1191,9 +1192,8 @@ impl ChainEndpoint for CosmosSdkChain {
         crate::time!("query_commitment_prefix");
 
         // TODO - do a real chain query
-        Ok(CommitmentPrefix::from(
-            self.config().store_prefix.as_bytes().to_vec(),
-        ))
+        CommitmentPrefix::try_from(self.config().store_prefix.as_bytes().to_vec())
+            .map_err(|_| Error::ics02(ClientError::empty_prefix()))
     }
 
     /// Query the chain status