From ae5c4d683576dae5341e34cf424f9406e567fe01 Mon Sep 17 00:00:00 2001 From: Gjermund Garaba Date: Fri, 7 Feb 2025 16:08:50 -0500 Subject: [PATCH 1/2] fix: limit max size of packet payloads for v1 and v2 --- modules/core/04-channel/types/packet.go | 6 ++++++ modules/core/04-channel/types/packet_test.go | 1 + modules/core/04-channel/v2/types/packet.go | 7 +++++++ modules/core/04-channel/v2/types/packet_test.go | 17 +++++++++++++++++ 4 files changed, 31 insertions(+) diff --git a/modules/core/04-channel/types/packet.go b/modules/core/04-channel/types/packet.go index b585f093e50..87ef8b615bb 100644 --- a/modules/core/04-channel/types/packet.go +++ b/modules/core/04-channel/types/packet.go @@ -12,6 +12,8 @@ import ( "github.com/cosmos/ibc-go/v9/modules/core/exported" ) +const MaxPayloadsSize = 262144 // 256 KiB. This is the maximum size of all payloads combined + // CommitPacket returns the packet commitment bytes. The commitment consists of: // sha256_hash(timeout_timestamp + timeout_height.RevisionNumber + timeout_height.RevisionHeight + sha256_hash(data)) // from a given packet. This results in a fixed length preimage. @@ -110,6 +112,10 @@ func (p Packet) ValidateBasic() error { return errorsmod.Wrap(ErrInvalidPacket, "packet data bytes cannot be empty") } + if len(p.Data) > MaxPayloadsSize { + return errorsmod.Wrapf(ErrInvalidPacket, "packet data bytes cannot exceed %d bytes", MaxPayloadsSize) + } + return nil } diff --git a/modules/core/04-channel/types/packet_test.go b/modules/core/04-channel/types/packet_test.go index 13bbde87598..92ad3c57801 100644 --- a/modules/core/04-channel/types/packet_test.go +++ b/modules/core/04-channel/types/packet_test.go @@ -60,6 +60,7 @@ func TestPacketValidateBasic(t *testing.T) { {types.NewPacket(validPacketData, 1, portid, chanid, invalidPort, cpchanid, timeoutHeight, timeoutTimestamp), errors.New("invalid destination port")}, {types.NewPacket(validPacketData, 1, portid, chanid, cpportid, invalidChannel, timeoutHeight, timeoutTimestamp), errors.New("invalid destination channel")}, {types.NewPacket(validPacketData, 1, portid, chanid, cpportid, cpchanid, disabledTimeout, 0), errors.New("packet timeout height and packet timeout timestamp cannot both be 0: invalid packet")}, + {types.NewPacket(make([]byte, types.MaxPayloadsSize+1), 1, portid, chanid, cpportid, cpchanid, timeoutHeight, timeoutTimestamp), errors.New("packet data bytes cannot exceed 262144 bytes: invalid packet")}, } for _, tc := range testCases { diff --git a/modules/core/04-channel/v2/types/packet.go b/modules/core/04-channel/v2/types/packet.go index 21525033f2a..d8685d613c8 100644 --- a/modules/core/04-channel/v2/types/packet.go +++ b/modules/core/04-channel/v2/types/packet.go @@ -5,6 +5,7 @@ import ( errorsmod "cosmossdk.io/errors" + channeltypesv1 "github.com/cosmos/ibc-go/v9/modules/core/04-channel/types" host "github.com/cosmos/ibc-go/v9/modules/core/24-host" ) @@ -36,10 +37,16 @@ func (p Packet) ValidateBasic() error { return errorsmod.Wrap(ErrInvalidPacket, "payloads must contain exactly one payload") } + totalPayloadsSize := 0 for _, pd := range p.Payloads { if err := pd.ValidateBasic(); err != nil { return errorsmod.Wrap(err, "invalid Payload") } + totalPayloadsSize += len(pd.Value) + } + + if totalPayloadsSize > channeltypesv1.MaxPayloadsSize { + return errorsmod.Wrapf(ErrInvalidPacket, "packet data bytes cannot exceed %d bytes", channeltypesv1.MaxPayloadsSize) } if err := host.ChannelIdentifierValidator(p.SourceClient); err != nil { diff --git a/modules/core/04-channel/v2/types/packet_test.go b/modules/core/04-channel/v2/types/packet_test.go index ec5d5776160..abf17dddf20 100644 --- a/modules/core/04-channel/v2/types/packet_test.go +++ b/modules/core/04-channel/v2/types/packet_test.go @@ -7,6 +7,7 @@ import ( "github.com/stretchr/testify/require" transfertypes "github.com/cosmos/ibc-go/v9/modules/apps/transfer/types" + channeltypesv1 "github.com/cosmos/ibc-go/v9/modules/core/04-channel/types" "github.com/cosmos/ibc-go/v9/modules/core/04-channel/v2/types" host "github.com/cosmos/ibc-go/v9/modules/core/24-host" ibctesting "github.com/cosmos/ibc-go/v9/testing" @@ -26,6 +27,22 @@ func TestValidateBasic(t *testing.T) { func() {}, nil, }, + { + "success, single payload just below MaxPayloadsSize", + func() { + packet.Payloads[0].Value = make([]byte, channeltypesv1.MaxPayloadsSize-1) + }, + nil, + }, + { + "failure: invalid single payloads size", + func() { + // bytes that are larger than MaxPayloadsSize + packet.Payloads[0].Value = make([]byte, channeltypesv1.MaxPayloadsSize+1) + }, + types.ErrInvalidPacket, + }, + // TODO: add test cases for multiple payloads when enabled (#7008) { "failure: payloads is nil", func() { From 4a3a705889b14e1847eda892e596aa688fd2473b Mon Sep 17 00:00:00 2001 From: Gjermund Garaba Date: Fri, 7 Feb 2025 23:20:06 -0500 Subject: [PATCH 2/2] rename --- modules/core/04-channel/types/packet.go | 6 +++--- modules/core/04-channel/types/packet_test.go | 2 +- modules/core/04-channel/v2/types/packet.go | 4 ++-- modules/core/04-channel/v2/types/packet_test.go | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/modules/core/04-channel/types/packet.go b/modules/core/04-channel/types/packet.go index 87ef8b615bb..f3524990331 100644 --- a/modules/core/04-channel/types/packet.go +++ b/modules/core/04-channel/types/packet.go @@ -12,7 +12,7 @@ import ( "github.com/cosmos/ibc-go/v9/modules/core/exported" ) -const MaxPayloadsSize = 262144 // 256 KiB. This is the maximum size of all payloads combined +const MaximumPayloadsSize = 262144 // 256 KiB. This is the maximum size of all payloads combined // CommitPacket returns the packet commitment bytes. The commitment consists of: // sha256_hash(timeout_timestamp + timeout_height.RevisionNumber + timeout_height.RevisionHeight + sha256_hash(data)) @@ -112,8 +112,8 @@ func (p Packet) ValidateBasic() error { return errorsmod.Wrap(ErrInvalidPacket, "packet data bytes cannot be empty") } - if len(p.Data) > MaxPayloadsSize { - return errorsmod.Wrapf(ErrInvalidPacket, "packet data bytes cannot exceed %d bytes", MaxPayloadsSize) + if len(p.Data) > MaximumPayloadsSize { + return errorsmod.Wrapf(ErrInvalidPacket, "packet data bytes cannot exceed %d bytes", MaximumPayloadsSize) } return nil diff --git a/modules/core/04-channel/types/packet_test.go b/modules/core/04-channel/types/packet_test.go index 92ad3c57801..60566bcae8c 100644 --- a/modules/core/04-channel/types/packet_test.go +++ b/modules/core/04-channel/types/packet_test.go @@ -60,7 +60,7 @@ func TestPacketValidateBasic(t *testing.T) { {types.NewPacket(validPacketData, 1, portid, chanid, invalidPort, cpchanid, timeoutHeight, timeoutTimestamp), errors.New("invalid destination port")}, {types.NewPacket(validPacketData, 1, portid, chanid, cpportid, invalidChannel, timeoutHeight, timeoutTimestamp), errors.New("invalid destination channel")}, {types.NewPacket(validPacketData, 1, portid, chanid, cpportid, cpchanid, disabledTimeout, 0), errors.New("packet timeout height and packet timeout timestamp cannot both be 0: invalid packet")}, - {types.NewPacket(make([]byte, types.MaxPayloadsSize+1), 1, portid, chanid, cpportid, cpchanid, timeoutHeight, timeoutTimestamp), errors.New("packet data bytes cannot exceed 262144 bytes: invalid packet")}, + {types.NewPacket(make([]byte, types.MaximumPayloadsSize+1), 1, portid, chanid, cpportid, cpchanid, timeoutHeight, timeoutTimestamp), errors.New("packet data bytes cannot exceed 262144 bytes: invalid packet")}, } for _, tc := range testCases { diff --git a/modules/core/04-channel/v2/types/packet.go b/modules/core/04-channel/v2/types/packet.go index d8685d613c8..d3fd79d01c5 100644 --- a/modules/core/04-channel/v2/types/packet.go +++ b/modules/core/04-channel/v2/types/packet.go @@ -45,8 +45,8 @@ func (p Packet) ValidateBasic() error { totalPayloadsSize += len(pd.Value) } - if totalPayloadsSize > channeltypesv1.MaxPayloadsSize { - return errorsmod.Wrapf(ErrInvalidPacket, "packet data bytes cannot exceed %d bytes", channeltypesv1.MaxPayloadsSize) + if totalPayloadsSize > channeltypesv1.MaximumPayloadsSize { + return errorsmod.Wrapf(ErrInvalidPacket, "packet data bytes cannot exceed %d bytes", channeltypesv1.MaximumPayloadsSize) } if err := host.ChannelIdentifierValidator(p.SourceClient); err != nil { diff --git a/modules/core/04-channel/v2/types/packet_test.go b/modules/core/04-channel/v2/types/packet_test.go index abf17dddf20..015ca83812f 100644 --- a/modules/core/04-channel/v2/types/packet_test.go +++ b/modules/core/04-channel/v2/types/packet_test.go @@ -30,7 +30,7 @@ func TestValidateBasic(t *testing.T) { { "success, single payload just below MaxPayloadsSize", func() { - packet.Payloads[0].Value = make([]byte, channeltypesv1.MaxPayloadsSize-1) + packet.Payloads[0].Value = make([]byte, channeltypesv1.MaximumPayloadsSize-1) }, nil, }, @@ -38,7 +38,7 @@ func TestValidateBasic(t *testing.T) { "failure: invalid single payloads size", func() { // bytes that are larger than MaxPayloadsSize - packet.Payloads[0].Value = make([]byte, channeltypesv1.MaxPayloadsSize+1) + packet.Payloads[0].Value = make([]byte, channeltypesv1.MaximumPayloadsSize+1) }, types.ErrInvalidPacket, },