Merge commit from fork

* Limit recursion depth for unknown field detection

* Limit unpack any

* Update changelog

* Another limit recursion depth for unknown field detection

* Remove x/tx files
This commit is contained in:
Alexander Peters 2024-12-16 17:07:24 +01:00 committed by GitHub
parent 98393bea6b
commit 896911659d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 71 additions and 5 deletions

View File

@ -51,7 +51,7 @@ Every module contains its own CHANGELOG.md. Please refer to the module you are i
* [#22826](https://github.com/cosmos/cosmos-sdk/pull/22826) Simplify testing frameworks by removing `testutil/cmdtest`.
### Bug Fixes
* Fix [ABS-0043/ ABS-0044](https://github.com/cosmos/cosmos-sdk/security/advisories/GHSA-8wcc-m6j2-qxvm) Limit recursion depth for unknown field detection and unpack any
* (sims) [#21906](https://github.com/cosmos/cosmos-sdk/pull/21906) Skip sims test when running dry on validators
* (cli) [#21919](https://github.com/cosmos/cosmos-sdk/pull/21919) Query address-by-acc-num by account_id instead of id.
* (cli) [#22656](https://github.com/cosmos/cosmos-sdk/pull/22656) Prune cmd should disable async pruning.

View File

@ -15,6 +15,17 @@ import (
"cosmossdk.io/x/tx/signing"
)
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
)
// UnpackInterfaces is a convenience function that calls UnpackInterfaces
// on x if x implements UnpackInterfacesMessage
func UnpackInterfaces(x interface{}, unpacker gogoprotoany.AnyUnpacker) error {
@ -230,6 +241,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
@ -240,6 +290,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 errors.New("UnpackAny expects a pointer")
@ -255,7 +307,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)
}
@ -277,7 +329,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
}

View File

@ -40,9 +40,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")
}
fieldDescProtoFromTagNum, _, err := getDescriptorInfo(msg)
if err != nil {
@ -125,7 +139,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
@ -148,7 +162,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