Skip to content

Commit

Permalink
Merge commit from fork
Browse files Browse the repository at this point in the history
* Limit recursion depth for unknown field detection

(cherry picked from commit f038dc731c55be1e1c526e67695acc358631afd6)

* Limit unpack any

(cherry picked from commit 1a2bff56fb7391f9ce87d4fbe9e0367ae991c0b2)

* Update Changelog

* Another limit recursion depth for unknown field detection

* Update changelog
  • Loading branch information
alpe authored Dec 16, 2024
1 parent 679ca5e commit ba7ac45
Show file tree
Hide file tree
Showing 5 changed files with 96 additions and 7 deletions.
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ Ref: https://keepachangelog.com/en/1.0.0/

## [Unreleased]

## [v0.50.11](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.50.11) - 2024-12-04

## [v0.50.11](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.50.11) - 2024-12-16

### Features

Expand All @@ -50,6 +51,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### 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
* (server) [#22564](https://github.com/cosmos/cosmos-sdk/pull/22564) Fix fallback genesis path in server
* (x/group) [#22425](https://github.com/cosmos/cosmos-sdk/pull/22425) Proper address rendering in error
* (sims) [#21906](https://github.com/cosmos/cosmos-sdk/pull/21906) Skip sims test when running dry on validators
Expand Down
57 changes: 55 additions & 2 deletions codec/types/interface_registry.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package types

import (
"errors"
"fmt"
"reflect"

Expand All @@ -12,6 +13,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
)

// AnyUnpacker is an interface which allows safely unpacking types packed
// in Any's against a whitelist of registered types
type AnyUnpacker interface {
Expand Down Expand Up @@ -270,6 +282,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
Expand All @@ -280,6 +331,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")
Expand All @@ -295,7 +348,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)
}
Expand All @@ -315,7 +368,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
}
Expand Down
18 changes: 16 additions & 2 deletions codec/unknownproto/unknown_fields.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}

desc, ok := msg.(descriptorIface)
if !ok {
Expand Down Expand Up @@ -130,7 +144,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
Expand All @@ -153,7 +167,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
Expand Down
6 changes: 6 additions & 0 deletions x/tx/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@ Since v0.13.0, x/tx follows Cosmos SDK semver: https://github.com/cosmos/cosmos-

## [Unreleased]

## [v0.13.7](https://github.com/cosmos/cosmos-sdk/releases/tag/x/tx/v0.13.7) - 2024-12-16

### Bug Fixes

* Fix [ABS-0043](https://github.com/cosmos/cosmos-sdk/security/advisories/GHSA-8wcc-m6j2-qxvm) Limit recursion depth for unknown field detection

## [v0.13.6](https://github.com/cosmos/cosmos-sdk/releases/tag/x/tx/v0.13.6) - 2024-12-12

### Bug Fixes
Expand Down
18 changes: 16 additions & 2 deletions x/tx/decode/unknown.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,23 @@ func RejectUnknownFieldsStrict(bz []byte, msg protoreflect.MessageDescriptor, re
// 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, desc protoreflect.MessageDescriptor, allowUnknownNonCriticals bool, resolver protodesc.Resolver) (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, desc, allowUnknownNonCriticals, resolver, 10_000)
}

func doRejectUnknownFields(
bz []byte,
desc protoreflect.MessageDescriptor,
allowUnknownNonCriticals bool,
resolver protodesc.Resolver,
recursionLimit int,
) (hasUnknownNonCriticals bool, err error) {
if len(bz) == 0 {
return hasUnknownNonCriticals, nil
}
if recursionLimit == 0 {
return false, errors.New("recursion limit reached")
}

fields := desc.Fields()

Expand Down Expand Up @@ -100,7 +114,7 @@ func RejectUnknownFields(bz []byte, desc protoreflect.MessageDescriptor, allowUn

if fieldMessage.FullName() == anyFullName {
// Firstly typecheck types.Any to ensure nothing snuck in.
hasUnknownNonCriticalsChild, err := RejectUnknownFields(fieldBytes, anyDesc, allowUnknownNonCriticals, resolver)
hasUnknownNonCriticalsChild, err := doRejectUnknownFields(fieldBytes, anyDesc, allowUnknownNonCriticals, resolver, recursionLimit-1)
hasUnknownNonCriticals = hasUnknownNonCriticals || hasUnknownNonCriticalsChild
if err != nil {
return hasUnknownNonCriticals, err
Expand All @@ -120,7 +134,7 @@ func RejectUnknownFields(bz []byte, desc protoreflect.MessageDescriptor, allowUn
fieldBytes = a.Value
}

hasUnknownNonCriticalsChild, err := RejectUnknownFields(fieldBytes, fieldMessage, allowUnknownNonCriticals, resolver)
hasUnknownNonCriticalsChild, err := doRejectUnknownFields(fieldBytes, fieldMessage, allowUnknownNonCriticals, resolver, recursionLimit-1)
hasUnknownNonCriticals = hasUnknownNonCriticals || hasUnknownNonCriticalsChild
if err != nil {
return hasUnknownNonCriticals, err
Expand Down

0 comments on commit ba7ac45

Please sign in to comment.