From e40d16ee899aa0ec757a6c664d7d9fe98c51bf0c Mon Sep 17 00:00:00 2001 From: Callum Waters Date: Mon, 16 Dec 2024 18:14:59 +0100 Subject: [PATCH 1/3] Merge commit from fork (#422) * Limit recursion depth for unknown field detection * Limit unpack any (cherry picked from commit 1a2bff56fb7391f9ce87d4fbe9e0367ae991c0b2) * Update changelog Co-authored-by: Alexander Peters --- codec/types/interface_registry.go | 57 +++++++++++++++++++++++++++- codec/unknownproto/unknown_fields.go | 18 ++++++++- 2 files changed, 71 insertions(+), 4 deletions(-) diff --git a/codec/types/interface_registry.go b/codec/types/interface_registry.go index 5d7e72e890c0..ac210ade07f3 100644 --- a/codec/types/interface_registry.go +++ b/codec/types/interface_registry.go @@ -1,6 +1,7 @@ package types import ( + "errors" "fmt" "reflect" @@ -9,6 +10,17 @@ import ( "github.com/gogo/protobuf/proto" ) +var ( + + // MaxUnpackAnySubCalls extension point that defines the maximum number of sub-calls allowed during the unpacking + // process of protobuf Any messages. + MaxUnpackAnySubCalls = 100 + + // MaxUnpackAnyRecursionDepth extension point that defines the maximum allowed recursion depth during protobuf Any + // message unpacking. + MaxUnpackAnyRecursionDepth = 10 +) + // AnyUnpacker is an interface which allows safely unpacking types packed // in Any's against a whitelist of registered types type AnyUnpacker interface { @@ -194,6 +206,45 @@ func (registry *interfaceRegistry) ListImplementations(ifaceName string) []strin } func (registry *interfaceRegistry) UnpackAny(any *Any, iface interface{}) error { + unpacker := &statefulUnpacker{ + registry: registry, + maxDepth: MaxUnpackAnyRecursionDepth, + maxCalls: &sharedCounter{count: MaxUnpackAnySubCalls}, + } + return unpacker.UnpackAny(any, iface) +} + +// sharedCounter is a type that encapsulates a counter value +type sharedCounter struct { + count int +} + +// statefulUnpacker is a struct that helps in deserializing and unpacking +// protobuf Any messages while maintaining certain stateful constraints. +type statefulUnpacker struct { + registry *interfaceRegistry + maxDepth int + maxCalls *sharedCounter +} + +// cloneForRecursion returns a new statefulUnpacker instance with maxDepth reduced by one, preserving the registry and maxCalls. +func (r statefulUnpacker) cloneForRecursion() *statefulUnpacker { + return &statefulUnpacker{ + registry: r.registry, + maxDepth: r.maxDepth - 1, + maxCalls: r.maxCalls, + } +} + +// UnpackAny deserializes a protobuf Any message into the provided interface, ensuring the interface is a pointer. +// It applies stateful constraints such as max depth and call limits, and unpacks interfaces if required. +func (r *statefulUnpacker) UnpackAny(any *Any, iface interface{}) error { + if r.maxDepth == 0 { + return errors.New("max depth exceeded") + } + if r.maxCalls.count == 0 { + return errors.New("call limit exceeded") + } // here we gracefully handle the case in which `any` itself is `nil`, which may occur in message decoding if any == nil { return nil @@ -204,6 +255,8 @@ func (registry *interfaceRegistry) UnpackAny(any *Any, iface interface{}) error return nil } + r.maxCalls.count-- + rv := reflect.ValueOf(iface) if rv.Kind() != reflect.Ptr { return fmt.Errorf("UnpackAny expects a pointer") @@ -219,7 +272,7 @@ func (registry *interfaceRegistry) UnpackAny(any *Any, iface interface{}) error } } - imap, found := registry.interfaceImpls[rt] + imap, found := r.registry.interfaceImpls[rt] if !found { return fmt.Errorf("no registered implementations of type %+v", rt) } @@ -239,7 +292,7 @@ func (registry *interfaceRegistry) UnpackAny(any *Any, iface interface{}) error return err } - err = UnpackInterfaces(msg, registry) + err = UnpackInterfaces(msg, r.cloneForRecursion()) if err != nil { return err } diff --git a/codec/unknownproto/unknown_fields.go b/codec/unknownproto/unknown_fields.go index 3af40ffed15b..7332a76e5c57 100644 --- a/codec/unknownproto/unknown_fields.go +++ b/codec/unknownproto/unknown_fields.go @@ -39,9 +39,23 @@ func RejectUnknownFieldsStrict(bz []byte, msg proto.Message, resolver jsonpb.Any // This function traverses inside of messages nested via google.protobuf.Any. It does not do any deserialization of the proto.Message. // An AnyResolver must be provided for traversing inside google.protobuf.Any's. func RejectUnknownFields(bz []byte, msg proto.Message, allowUnknownNonCriticals bool, resolver jsonpb.AnyResolver) (hasUnknownNonCriticals bool, err error) { + // recursion limit with same default as https://github.com/protocolbuffers/protobuf-go/blob/v1.35.2/encoding/protowire/wire.go#L28 + return doRejectUnknownFields(bz, msg, allowUnknownNonCriticals, resolver, 10_000) +} + +func doRejectUnknownFields( + bz []byte, + msg proto.Message, + allowUnknownNonCriticals bool, + resolver jsonpb.AnyResolver, + recursionLimit int, +) (hasUnknownNonCriticals bool, err error) { if len(bz) == 0 { return hasUnknownNonCriticals, nil } + if recursionLimit == 0 { + return false, errors.New("recursion limit reached") + } desc, ok := msg.(descriptorIface) if !ok { @@ -129,7 +143,7 @@ func RejectUnknownFields(bz []byte, msg proto.Message, allowUnknownNonCriticals if protoMessageName == ".google.protobuf.Any" { // Firstly typecheck types.Any to ensure nothing snuck in. - hasUnknownNonCriticalsChild, err := RejectUnknownFields(fieldBytes, (*types.Any)(nil), allowUnknownNonCriticals, resolver) + hasUnknownNonCriticalsChild, err := doRejectUnknownFields(fieldBytes, (*types.Any)(nil), allowUnknownNonCriticals, resolver, recursionLimit-1) hasUnknownNonCriticals = hasUnknownNonCriticals || hasUnknownNonCriticalsChild if err != nil { return hasUnknownNonCriticals, err @@ -152,7 +166,7 @@ func RejectUnknownFields(bz []byte, msg proto.Message, allowUnknownNonCriticals } } - hasUnknownNonCriticalsChild, err := RejectUnknownFields(fieldBytes, msg, allowUnknownNonCriticals, resolver) + hasUnknownNonCriticalsChild, err := doRejectUnknownFields(fieldBytes, msg, allowUnknownNonCriticals, resolver, recursionLimit-1) hasUnknownNonCriticals = hasUnknownNonCriticals || hasUnknownNonCriticalsChild if err != nil { return hasUnknownNonCriticals, err From d2163cecdef80591e86d0e5c8c7e6b8f70e2d0ad Mon Sep 17 00:00:00 2001 From: Rootul P Date: Fri, 20 Dec 2024 10:30:22 -0500 Subject: [PATCH 2/3] chore(deps): upgrade to cosmos-ledger-go v0.14.0 (#424) * feat: cherry-pick c226e8b * chore(deps): upgrade to cosmos-ledger-go v0.14.0 --- crypto/keyring/keyring.go | 2 +- crypto/ledger/ledger_mock.go | 2 +- crypto/ledger/ledger_secp256k1.go | 30 ++++++++++++++++++++++++------ crypto/types/types.go | 11 +++++++++++ go.mod | 2 +- go.sum | 4 ++-- 6 files changed, 40 insertions(+), 11 deletions(-) diff --git a/crypto/keyring/keyring.go b/crypto/keyring/keyring.go index bfcd473ff50f..831cc693c378 100644 --- a/crypto/keyring/keyring.go +++ b/crypto/keyring/keyring.go @@ -594,7 +594,7 @@ func SignWithLedger(k *Record, msg []byte) (sig []byte, pub types.PubKey, err er return nil, nil, fmt.Errorf("the public key that the user attempted to sign with does not match the public key on the ledger device. %v does not match %v", pubKey.String(), ledgerPubKey.String()) } - sig, err = priv.Sign(msg) + sig, err = priv.SignLedgerAminoJSON(msg) if err != nil { return nil, nil, err } diff --git a/crypto/ledger/ledger_mock.go b/crypto/ledger/ledger_mock.go index 21e18cc6c99a..60d6471cef0b 100644 --- a/crypto/ledger/ledger_mock.go +++ b/crypto/ledger/ledger_mock.go @@ -84,7 +84,7 @@ func (mock LedgerSECP256K1Mock) GetAddressPubKeySECP256K1(derivationPath []uint3 return pk, addr, err } -func (mock LedgerSECP256K1Mock) SignSECP256K1(derivationPath []uint32, message []byte) ([]byte, error) { +func (mock LedgerSECP256K1Mock) SignSECP256K1(derivationPath []uint32, message []byte, p2 byte) ([]byte, error) { path := hd.NewParams(derivationPath[0], derivationPath[1], derivationPath[2], derivationPath[3] != 0, derivationPath[4]) seed, err := bip39.NewSeedWithErrorChecking(testdata.TestMnemonic, "") if err != nil { diff --git a/crypto/ledger/ledger_secp256k1.go b/crypto/ledger/ledger_secp256k1.go index 29f50ad4e212..fac4e1716605 100644 --- a/crypto/ledger/ledger_secp256k1.go +++ b/crypto/ledger/ledger_secp256k1.go @@ -32,7 +32,10 @@ type ( // Returns a compressed pubkey and bech32 address (requires user confirmation) GetAddressPubKeySECP256K1([]uint32, string) ([]byte, string, error) // Signs a message (requires user confirmation) - SignSECP256K1([]uint32, []byte) ([]byte, error) + // The last byte denotes the SIGN_MODE to be used by Ledger: 0 for + // LEGACY_AMINO_JSON, 1 for TEXTUAL. It corresponds to the P2 value + // in https://github.com/cosmos/ledger-cosmos/blob/main/docs/APDUSPEC.md + SignSECP256K1([]uint32, []byte, byte) ([]byte, error) } // PrivKeyLedgerSecp256k1 implements PrivKey, calling the ledger nano we @@ -51,7 +54,7 @@ type ( // This function is marked as unsafe as it will retrieve a pubkey without user verification. // It can only be used to verify a pubkey but never to create new accounts/keys. In that case, // please refer to NewPrivKeySecp256k1 -func NewPrivKeySecp256k1Unsafe(path hd.BIP44Params) (types.LedgerPrivKey, error) { +func NewPrivKeySecp256k1Unsafe(path hd.BIP44Params) (types.LedgerPrivKeyAminoJSON, error) { device, err := getDevice() if err != nil { return nil, err @@ -88,7 +91,8 @@ func (pkl PrivKeyLedgerSecp256k1) PubKey() types.PubKey { return pkl.CachedPubKey } -// Sign returns a secp256k1 signature for the corresponding message +// Sign returns a secp256k1 signature for the corresponding message using +// SIGN_MODE_TEXTUAL. func (pkl PrivKeyLedgerSecp256k1) Sign(message []byte) ([]byte, error) { device, err := getDevice() if err != nil { @@ -96,7 +100,19 @@ func (pkl PrivKeyLedgerSecp256k1) Sign(message []byte) ([]byte, error) { } defer warnIfErrors(device.Close) - return sign(device, pkl, message) + return sign(device, pkl, message, 1) +} + +// SignLedgerAminoJSON returns a secp256k1 signature for the corresponding message using +// SIGN_MODE_LEGACY_AMINO_JSON. +func (pkl PrivKeyLedgerSecp256k1) SignLedgerAminoJSON(message []byte) ([]byte, error) { + device, err := getDevice() + if err != nil { + return nil, err + } + defer warnIfErrors(device.Close) + + return sign(device, pkl, message, 0) } // ShowAddress triggers a ledger device to show the corresponding address. @@ -228,13 +244,15 @@ func validateKey(device SECP256K1, pkl PrivKeyLedgerSecp256k1) error { // Communication is checked on NewPrivKeyLedger and PrivKeyFromBytes, returning // an error, so this should only trigger if the private key is held in memory // for a while before use. -func sign(device SECP256K1, pkl PrivKeyLedgerSecp256k1, msg []byte) ([]byte, error) { +// +// Last byte P2 is 0 for LEGACY_AMINO_JSON, and 1 for TEXTUAL. +func sign(device SECP256K1, pkl PrivKeyLedgerSecp256k1, msg []byte, p2 byte) ([]byte, error) { err := validateKey(device, pkl) if err != nil { return nil, err } - sig, err := device.SignSECP256K1(pkl.Path.DerivationPath(), msg) + sig, err := device.SignSECP256K1(pkl.Path.DerivationPath(), msg, p2) if err != nil { return nil, err } diff --git a/crypto/types/types.go b/crypto/types/types.go index eccdba73813d..296900776563 100644 --- a/crypto/types/types.go +++ b/crypto/types/types.go @@ -29,6 +29,17 @@ type LedgerPrivKey interface { Type() string } +// LedgerPrivKeyAminoJSON is a Ledger PrivKey type that supports signing with +// SIGN_MODE_LEGACY_AMINO_JSON. It is added as a non-breaking change, instead of directly +// on the LedgerPrivKey interface (whose Sign method will sign with TEXTUAL), +// and will be deprecated/removed once LEGACY_AMINO_JSON is removed. +type LedgerPrivKeyAminoJSON interface { + LedgerPrivKey + // SignLedgerAminoJSON signs a messages on the Ledger device using + // SIGN_MODE_LEGACY_AMINO_JSON. + SignLedgerAminoJSON(msg []byte) ([]byte, error) +} + // PrivKey defines a private key and extends proto.Message. For now, it extends // LedgerPrivKey (see godoc for LedgerPrivKey). Ultimately, we should remove // LedgerPrivKey and add its methods here directly. diff --git a/go.mod b/go.mod index 5b0cdb1fc4f9..d31437c2f392 100644 --- a/go.mod +++ b/go.mod @@ -18,7 +18,7 @@ require ( github.com/cosmos/cosmos-sdk/db v1.0.0-beta.1 github.com/cosmos/go-bip39 v1.0.0 github.com/cosmos/iavl v0.19.6 - github.com/cosmos/ledger-cosmos-go v0.12.4 + github.com/cosmos/ledger-cosmos-go v0.14.0 github.com/gogo/gateway v1.1.0 github.com/gogo/protobuf v1.3.2 github.com/golang/mock v1.6.0 diff --git a/go.sum b/go.sum index 412cdcabfb69..81b7b0277806 100644 --- a/go.sum +++ b/go.sum @@ -213,8 +213,8 @@ github.com/cosmos/iavl v0.19.6 h1:XY78yEeNPrEYyNCKlqr9chrwoeSDJ0bV2VjocTk//OU= github.com/cosmos/iavl v0.19.6/go.mod h1:X9PKD3J0iFxdmgNLa7b2LYWdsGd90ToV5cAONApkEPw= github.com/cosmos/keyring v1.2.0 h1:8C1lBP9xhImmIabyXW4c3vFjjLiBdGCmfLUfeZlV1Yo= github.com/cosmos/keyring v1.2.0/go.mod h1:fc+wB5KTk9wQ9sDx0kFXB3A0MaeGHM9AwRStKOQ5vOA= -github.com/cosmos/ledger-cosmos-go v0.12.4 h1:drvWt+GJP7Aiw550yeb3ON/zsrgW0jgh5saFCr7pDnw= -github.com/cosmos/ledger-cosmos-go v0.12.4/go.mod h1:fjfVWRf++Xkygt9wzCsjEBdjcf7wiiY35fv3ctT+k4M= +github.com/cosmos/ledger-cosmos-go v0.14.0 h1:WfCHricT3rPbkPSVKRH+L4fQGKYHuGOK9Edpel8TYpE= +github.com/cosmos/ledger-cosmos-go v0.14.0/go.mod h1:E07xCWSBl3mTGofZ2QnL4cIUzMbbGVyik84QYKbX3RA= github.com/cpuguy83/go-md2man v1.0.10/go.mod h1:SmD6nW6nTyfqj6ABTjUi3V3JVMnlJmwcJI5acqYI6dE= github.com/cpuguy83/go-md2man/v2 v2.0.0-20190314233015-f79a8a8ca69d/go.mod h1:maD7wRr/U5Z6m/iR4s+kqSMx2CaBsrgA7czyZG/E6dU= github.com/cpuguy83/go-md2man/v2 v2.0.4/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= From fc94c0e2e549ab0a89bfd8241ed90005bfce5ca2 Mon Sep 17 00:00:00 2001 From: Rootul P Date: Mon, 13 Jan 2025 19:57:52 -0700 Subject: [PATCH 3/3] fix: use default send rate configured by celestia-app (#425) --- server/util.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/server/util.go b/server/util.go index 28532a1c5821..9ef11549ff6f 100644 --- a/server/util.go +++ b/server/util.go @@ -213,8 +213,6 @@ func interceptConfigs(rootViper *viper.Viper, customAppTemplate string, customCo } conf.RPC.PprofListenAddress = "localhost:6060" - conf.P2P.RecvRate = 5120000 - conf.P2P.SendRate = 5120000 tmcfg.WriteConfigFile(tmCfgFile, conf) case err != nil: