Summary of Bug
The IBC v2 MsgAcknowledgement handler indexes Acknowledgement.AppAcknowledgements by payload position, but nothing on the acknowledging side checks that a successful acknowledgement has one entry per payload:
https://github.com/cosmos/ibc-go/blob/622fe8d/modules/core/04-channel/v2/keeper/msg_server.go#L183-L192
recvSuccess := !bytes.Equal(msg.Acknowledgement.AppAcknowledgements[0], types.ErrorAcknowledgement[:])
for i, pd := range msg.Packet.Payloads {
...
if recvSuccess {
ack = msg.Acknowledgement.AppAcknowledgements[i]The length check exists only where the acknowledgement is written (writeAcknowledgement rejects a successful ack whose length differs from the payload count). MsgAcknowledgement.ValidateBasic / Acknowledgement.Validate only check that the list is non-empty and that the error sentinel is used alone, and acknowledgePacket only proves the commitment. If the counterparty commits a successful acknowledgement with fewer entries than the packet has payloads, the handler hits index out of range inside MsgAcknowledgement after the proof verified.
The existing TestMsgAcknowledge_ValidateBasic case "success, multiple payloads" actually builds this message (2 payloads, 1 app ack) and expects it to pass.
Expected Behaviour
A MsgAcknowledgement whose successful acknowledgement doesn't have one app acknowledgement per payload is rejected with ErrInvalidAcknowledgement in ValidateBasic, the same rule writeAcknowledgement applies on the other side, instead of panicking in the handler.
Version
main @ 622fe8d
Steps to Reproduce
Build a MsgAcknowledgement for a packet with two payloads and NewAcknowledgement([]byte("appAck1")): ValidateBasic returns nil, and the handler reads AppAcknowledgements[1] once the (counterparty-provided) proof verifies.
Reaching the handler needs a counterparty that committed such an ack, so this is about handling input from a non-ibc-go / misbehaving counterparty with an error rather than a panic. ibc-go and the Solidity router don't produce it (the Solidity router only accepts single-payload packets).
For Admin Use
Summary of Bug
The IBC v2
MsgAcknowledgementhandler indexesAcknowledgement.AppAcknowledgementsby payload position, but nothing on the acknowledging side checks that a successful acknowledgement has one entry per payload:https://github.com/cosmos/ibc-go/blob/622fe8d/modules/core/04-channel/v2/keeper/msg_server.go#L183-L192
The length check exists only where the acknowledgement is written (
writeAcknowledgementrejects a successful ack whose length differs from the payload count).MsgAcknowledgement.ValidateBasic/Acknowledgement.Validateonly check that the list is non-empty and that the error sentinel is used alone, andacknowledgePacketonly proves the commitment. If the counterparty commits a successful acknowledgement with fewer entries than the packet has payloads, the handler hitsindex out of rangeinsideMsgAcknowledgementafter the proof verified.The existing
TestMsgAcknowledge_ValidateBasiccase "success, multiple payloads" actually builds this message (2 payloads, 1 app ack) and expects it to pass.Expected Behaviour
A
MsgAcknowledgementwhose successful acknowledgement doesn't have one app acknowledgement per payload is rejected withErrInvalidAcknowledgementinValidateBasic, the same rulewriteAcknowledgementapplies on the other side, instead of panicking in the handler.Version
main@ 622fe8dSteps to Reproduce
Build a
MsgAcknowledgementfor a packet with two payloads andNewAcknowledgement([]byte("appAck1")):ValidateBasicreturns nil, and the handler readsAppAcknowledgements[1]once the (counterparty-provided) proof verifies.Reaching the handler needs a counterparty that committed such an ack, so this is about handling input from a non-ibc-go / misbehaving counterparty with an error rather than a panic. ibc-go and the Solidity router don't produce it (the Solidity router only accepts single-payload packets).
For Admin Use