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

Fix counterparty channel id unwrap #220

Merged
merged 3 commits into from
Nov 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions .changelog/unreleased/bug-fixes/219-counterparty-unwrap.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Don't panic on user input in channel proof verification
([#219](https://github.com/cosmos/ibc-rs/issues/219))
1 change: 0 additions & 1 deletion crates/ibc/src/core/ics04_channel/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,6 @@ define_error! {
},

InvalidCounterpartyChannelId
[ ValidationError ]
| _ | { "Invalid channel id in counterparty" },

InvalidChannelState
Expand Down
42 changes: 42 additions & 0 deletions crates/ibc/src/core/ics04_channel/handler/chan_open_try.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,4 +283,46 @@ mod tests {
}
}
}

/// Addresses [issue 219](https://github.com/cosmos/ibc-rs/issues/219)
#[test]
fn chan_open_try_invalid_counterparty_channel_id() {
let proof_height = 10;
let conn_id = ConnectionId::new(2);
let client_id = ClientId::new(mock_client_type(), 45).unwrap();

// This is the connection underlying the channel we're trying to open.
let conn_end = ConnectionEnd::new(
ConnectionState::Open,
client_id.clone(),
ConnectionCounterparty::try_from(get_dummy_raw_counterparty()).unwrap(),
get_compatible_versions(),
ZERO_DURATION,
);

// We're going to test message processing against this message.
// Note: we make the counterparty's channel_id `None`.
let mut msg =
MsgChannelOpenTry::try_from(get_dummy_raw_msg_chan_open_try(proof_height)).unwrap();
msg.channel.remote.channel_id = None;

let chan_id = ChannelId::new(24);
let hops = vec![conn_id.clone()];
msg.channel.connection_hops = hops;

let chan_end = ChannelEnd::new(
State::Init,
*msg.channel.ordering(),
msg.channel.counterparty().clone(),
msg.channel.connection_hops().clone(),
msg.channel.version().clone(),
);
let context = MockContext::default()
.with_client(&client_id, Height::new(0, proof_height).unwrap())
.with_connection(conn_id, conn_end)
.with_channel(msg.port_id.clone(), chan_id, chan_end);

// Makes sure we don't crash
let _ = chan_open_try::process(&context, &msg);
}
}
5 changes: 4 additions & 1 deletion crates/ibc/src/core/ics04_channel/handler/verify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,10 @@ pub fn verify_channel_proofs<Ctx: ChannelReader>(
proofs.object_proof(),
consensus_state.root(),
channel_end.counterparty().port_id(),
channel_end.counterparty().channel_id().unwrap(),
channel_end
.counterparty()
.channel_id()
.ok_or_else(Error::invalid_counterparty_channel_id)?,
expected_chan,
)
.map_err(Error::verify_channel_failed)
Expand Down
2 changes: 1 addition & 1 deletion crates/ibc/src/core/ics04_channel/msgs/chan_open_try.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ impl TryFrom<RawMsgChannelOpenTry> for MsgChannelOpenTry {
};

msg.validate_basic()
.map_err(ChannelError::invalid_counterparty_channel_id)?;
.map_err(|_| ChannelError::invalid_counterparty_channel_id())?;

Ok(msg)
}
Expand Down