Problem: x/tx not up to date to the released version

x/tx/CHANGELOG.md

@@ -31,6 +31,63 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ## [Unreleased]
 
+## v0.13.1
+
+### Features
+
+* [#19618](https://github.com/cosmos/cosmos-sdk/pull/19618) Add enum as string option to encoder.
+
+### Improvements
+
+* [#18857](https://github.com/cosmos/cosmos-sdk/pull/18857) Moved `FormatCoins` from `core/coins` to this package under `signing/textual`.
+
+### Bug Fixes
+
+* [#19265](https://github.com/cosmos/cosmos-sdk/pull/19265) Reject denoms that contain a comma.
+
+## v0.13.0
+
+### Improvements
+
+* [#18740](https://github.com/cosmos/cosmos-sdk/pull/18740) Support nested messages when fetching signers up to a default depth of 32.
+
+## v0.12.0
+
+### Improvements
+
+* [#18309](https://github.com/cosmos/cosmos-sdk/pull/18309) Update encoder so that amino types default to msg type url.
+
+## v0.11.0
+
+### Improvements
+
+* [#17787](https://github.com/cosmos/cosmos-sdk/pull/17787) Drop tip support.
+
+## v0.10.0
+
+### Features
+
+* [#17681](https://github.com/cosmos/cosmos-sdk/pull/17681) Add encoder `DefineTypeEncoding` method for defining custom type encodings.
+* [#17600](https://github.com/cosmos/cosmos-sdk/pull/17600) Add encoder `DefineScalarEncoding` method for defining custom scalar encodings.
+* [#17600](https://github.com/cosmos/cosmos-sdk/pull/17600) Add indent option to encoder.
+
+## v0.9.1
+
+### Improvements
+
+* [#16936](https://github.com/cosmos/cosmos-sdk/pull/16936) Remove extra whitespace when marshalling module accounts.
+
+## v0.9.0
+
+### Bug Fixes
+
+* [#16681](https://github.com/cosmos/cosmos-sdk/pull/16681): Catch and fix `(*Decoder).Decode` crash from invalid length prefix in Tx bytes.
+
+### Improvements
+
+* [#16846](https://github.com/cosmos/cosmos-sdk/pull/16846): Harmonize interface `signing.TypeResolver` with the rest of the codebase (orm and client/v2).
+* [#16684](https://github.com/cosmos/cosmos-sdk/pull/16684): Use `io.WriteString`+`fmt.Fprintf` to remove unnecessary `string`->`[]byte` roundtrip.
+
 ## v0.8.0
 
 ### Improvements

x/tx/decode/adr027.go

@@ -24,6 +24,14 @@ func rejectNonADR027TxRaw(txBytes []byte) error {
 		if m < 0 {
 			return fmt.Errorf("invalid length; %w", protowire.ParseError(m))
 		}
+
+		// Paranoia from possible varint decoding which can trivially
+		// be wrong due to the precarious nature of the format being tricked:
+		// https://cyber.orijtech.com/advisory/varint-decode-limitless
+		if m > len(txBytes) {
+			return fmt.Errorf("invalid length from decoding (%d) > len(txBytes) (%d)", m, len(txBytes))
+		}
+
 		// TxRaw only has bytes fields.
 		if wireType != protowire.BytesType {
 			return fmt.Errorf("expected %d wire type, got %d", protowire.BytesType, wireType)

x/tx/decode/decode.go

@@ -1,13 +1,13 @@
 package decode
 
 import (
-	"fmt"
+	"errors"
 
 	"github.com/cosmos/cosmos-proto/anyutil"
 	"google.golang.org/protobuf/proto"
 
 	v1beta1 "cosmossdk.io/api/cosmos/tx/v1beta1"
-	"cosmossdk.io/errors"
+	errorsmod "cosmossdk.io/errors"
 	"cosmossdk.io/x/tx/signing"
 )
 
@@ -33,7 +33,7 @@ type Options struct {
 // NewDecoder creates a new Decoder for decoding transactions.
 func NewDecoder(options Options) (*Decoder, error) {
 	if options.SigningContext == nil {
-		return nil, fmt.Errorf("signing context is required")
+		return nil, errors.New("signing context is required")
 	}
 
 	return &Decoder{
@@ -46,7 +46,7 @@ func (d *Decoder) Decode(txBytes []byte) (*DecodedTx, error) {
 	// Make sure txBytes follow ADR-027.
 	err := rejectNonADR027TxRaw(txBytes)
 	if err != nil {
-		return nil, errors.Wrap(ErrTxDecode, err.Error())
+		return nil, errorsmod.Wrap(ErrTxDecode, err.Error())
 	}
 
 	var raw v1beta1.TxRaw
@@ -55,7 +55,7 @@ func (d *Decoder) Decode(txBytes []byte) (*DecodedTx, error) {
 	fileResolver := d.signingCtx.FileResolver()
 	err = RejectUnknownFieldsStrict(txBytes, raw.ProtoReflect().Descriptor(), fileResolver)
 	if err != nil {
-		return nil, errors.Wrap(ErrTxDecode, err.Error())
+		return nil, errorsmod.Wrap(ErrTxDecode, err.Error())
 	}
 
 	err = proto.Unmarshal(txBytes, &raw)
@@ -68,25 +68,25 @@ func (d *Decoder) Decode(txBytes []byte) (*DecodedTx, error) {
 	// allow non-critical unknown fields in TxBody
 	txBodyHasUnknownNonCriticals, err := RejectUnknownFields(raw.BodyBytes, body.ProtoReflect().Descriptor(), true, fileResolver)
 	if err != nil {
-		return nil, errors.Wrap(ErrTxDecode, err.Error())
+		return nil, errorsmod.Wrap(ErrTxDecode, err.Error())
 	}
 
 	err = proto.Unmarshal(raw.BodyBytes, &body)
 	if err != nil {
-		return nil, errors.Wrap(ErrTxDecode, err.Error())
+		return nil, errorsmod.Wrap(ErrTxDecode, err.Error())
 	}
 
 	var authInfo v1beta1.AuthInfo
 
 	// reject all unknown proto fields in AuthInfo
 	err = RejectUnknownFieldsStrict(raw.AuthInfoBytes, authInfo.ProtoReflect().Descriptor(), fileResolver)
 	if err != nil {
-		return nil, errors.Wrap(ErrTxDecode, err.Error())
+		return nil, errorsmod.Wrap(ErrTxDecode, err.Error())
 	}
 
 	err = proto.Unmarshal(raw.AuthInfoBytes, &authInfo)
 	if err != nil {
-		return nil, errors.Wrap(ErrTxDecode, err.Error())
+		return nil, errorsmod.Wrap(ErrTxDecode, err.Error())
 	}
 
 	theTx := &v1beta1.Tx{
@@ -97,17 +97,25 @@ func (d *Decoder) Decode(txBytes []byte) (*DecodedTx, error) {
 
 	var signers [][]byte
 	var msgs []proto.Message
+	seenSigners := map[string]struct{}{}
 	for _, anyMsg := range body.Messages {
 		msg, signerErr := anyutil.Unpack(anyMsg, fileResolver, d.signingCtx.TypeResolver())
 		if signerErr != nil {
-			return nil, errors.Wrap(ErrTxDecode, signerErr.Error())
+			return nil, errorsmod.Wrap(ErrTxDecode, signerErr.Error())
 		}
 		msgs = append(msgs, msg)
 		ss, signerErr := d.signingCtx.GetSigners(msg)
 		if signerErr != nil {
-			return nil, errors.Wrap(ErrTxDecode, signerErr.Error())
+			return nil, errorsmod.Wrap(ErrTxDecode, signerErr.Error())
+		}
+		for _, s := range ss {
+			_, seen := seenSigners[string(s)]
+			if seen {
+				continue
+			}
+			signers = append(signers, s)
+			seenSigners[string(s)] = struct{}{}
 		}
-		signers = append(signers, ss...)
 	}
 
 	return &DecodedTx{

x/tx/decode/decode_test.go

@@ -3,6 +3,7 @@ package decode_test
 import (
 	"encoding/hex"
 	"fmt"
+	"strings"
 	"testing"
 
 	"github.com/cosmos/cosmos-proto/anyutil"
@@ -85,10 +86,6 @@ func TestDecode(t *testing.T) {
 						Payer:    "payer",
 						Granter:  "",
 					},
-					Tip: &txv1beta1.Tip{ //nolint:staticcheck // we still need this deprecated struct
-						Amount: []*basev1beta1.Coin{{Amount: "100", Denom: "denom"}},
-						Tipper: "tipper",
-					},
 				},
 				Signatures: nil,
 			}
@@ -118,3 +115,31 @@ func (d dummyAddressCodec) StringToBytes(text string) ([]byte, error) {
 func (d dummyAddressCodec) BytesToString(bz []byte) (string, error) {
 	return hex.EncodeToString(bz), nil
 }
+
+func TestDecodeTxBodyPanic(t *testing.T) {
+	crashVector := []byte{
+		0x0a, 0x0a, 0x09, 0xe7, 0xbf, 0xba, 0xe6, 0x82, 0x9a, 0xe6, 0xaa, 0x30,
+	}
+
+	cdc := new(dummyAddressCodec)
+	signingCtx, err := signing.NewContext(signing.Options{
+		AddressCodec:          cdc,
+		ValidatorAddressCodec: cdc,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	dec, err := decode.NewDecoder(decode.Options{
+		SigningContext: signingCtx,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	_, err = dec.Decode(crashVector)
+	if err == nil {
+		t.Fatal("expected a non-nil error")
+	}
+	if g, w := err.Error(), "could not consume length prefix"; !strings.Contains(g, w) {
+		t.Fatalf("error mismatch\n%s\nodes not contain\n\t%q", g, w)
+	}
+}

x/tx/decode/fuzz_test.go

@@ -0,0 +1,140 @@
+package decode
+
+import (
+	"encoding/hex"
+	"testing"
+
+	"github.com/cosmos/cosmos-proto/anyutil"
+	fuzz "github.com/google/gofuzz"
+	"google.golang.org/protobuf/proto"
+	"google.golang.org/protobuf/types/known/anypb"
+
+	bankv1beta1 "cosmossdk.io/api/cosmos/bank/v1beta1"
+	basev1beta1 "cosmossdk.io/api/cosmos/base/v1beta1"
+	"cosmossdk.io/api/cosmos/crypto/secp256k1"
+	signingv1beta1 "cosmossdk.io/api/cosmos/tx/signing/v1beta1"
+	txv1beta1 "cosmossdk.io/api/cosmos/tx/v1beta1"
+	"cosmossdk.io/x/tx/signing"
+)
+
+var (
+	accSeq = uint64(2)
+
+	signerInfo = []*txv1beta1.SignerInfo{
+		{
+			PublicKey: pkAny,
+			ModeInfo: &txv1beta1.ModeInfo{
+				Sum: &txv1beta1.ModeInfo_Single_{
+					Single: &txv1beta1.ModeInfo_Single{
+						Mode: signingv1beta1.SignMode_SIGN_MODE_DIRECT,
+					},
+				},
+			},
+			Sequence: accSeq,
+		},
+	}
+
+	anyMsg, _ = anyutil.New(&bankv1beta1.MsgSend{})
+
+	pkAny, _ = anyutil.New(&secp256k1.PubKey{Key: []byte("foo")})
+)
+
+func generateAndAddSeedsFromTx(f *testing.F) {
+	f.Helper()
+	// 1. Add some seeds.
+	tx := &txv1beta1.Tx{
+		Body: &txv1beta1.TxBody{
+			Messages:      []*anypb.Any{anyMsg},
+			Memo:          "memo",
+			TimeoutHeight: 0,
+		},
+		AuthInfo: &txv1beta1.AuthInfo{
+			SignerInfos: signerInfo,
+			Fee: &txv1beta1.Fee{
+				Amount:   []*basev1beta1.Coin{{Amount: "100", Denom: "denom"}},
+				GasLimit: 100,
+				Payer:    "payer",
+				Granter:  "",
+			},
+		},
+		Signatures: nil,
+	}
+	f.Add(mustMarshal(f, tx))
+	fz := fuzz.New()
+	// 1.1. Mutate tx as much and add those as seeds.
+	for i := 0; i < 1e4; i++ {
+		func() {
+			defer func() {
+				_ = recover() // Catch any panics and continue
+			}()
+			fz.Fuzz(tx)
+			f.Add(mustMarshal(f, tx))
+		}()
+	}
+}
+
+func FuzzInternal_rejectNonADR027TxRaw(f *testing.F) {
+	if testing.Short() {
+		f.Skip("Skipping in -short mode")
+	}
+
+	// 1. Add some seeds.
+	generateAndAddSeedsFromTx(f)
+
+	// 2. Now run the fuzzer.
+	f.Fuzz(func(t *testing.T, in []byte) {
+		// Just ensure it doesn't crash.
+		_ = rejectNonADR027TxRaw(in)
+	})
+}
+
+func FuzzDecode(f *testing.F) {
+	if testing.Short() {
+		f.Skip("Skipping in -short mode")
+	}
+
+	// 1. Add some seeds.
+	generateAndAddSeedsFromTx(f)
+
+	// 2. Now fuzz it.
+	cdc := new(asHexCodec)
+	signingCtx, err := signing.NewContext(signing.Options{
+		AddressCodec:          cdc,
+		ValidatorAddressCodec: cdc,
+	})
+	if err != nil {
+		return
+	}
+	dec, err := NewDecoder(Options{
+		SigningContext: signingCtx,
+	})
+	if err != nil {
+		return
+	}
+
+	f.Fuzz(func(t *testing.T, in []byte) {
+		txr, err := dec.Decode(in)
+		if err == nil && txr == nil {
+			t.Fatal("inconsistency: err==nil yet tx==nil")
+		}
+	})
+}
+
+func mustMarshal(f *testing.F, m proto.Message) []byte {
+	f.Helper()
+	blob, err := proto.Marshal(m)
+	if err != nil {
+		f.Fatal(err)
+	}
+	return blob
+}
+
+type asHexCodec int
+
+func (d asHexCodec) StringToBytes(text string) ([]byte, error) {
+	return hex.DecodeString(text)
+}
+
+func (d asHexCodec) BytesToString(bz []byte) (string, error) {
+	return hex.EncodeToString(bz), nil
+}

x/tx/decode/unknown.go

@@ -85,6 +85,15 @@ func RejectUnknownFields(bz []byte, desc protoreflect.MessageDescriptor, allowUn
 
 		// consume length prefix of nested message
 		_, o := protowire.ConsumeVarint(fieldBytes)
+		if o < 0 {
+			err = fmt.Errorf("could not consume length prefix fieldBytes for nested message: %v: %w",
+				fieldMessage, protowire.ParseError(o))
+			return hasUnknownNonCriticals, err
+		} else if o > len(fieldBytes) {
+			err = fmt.Errorf("length prefix > len(fieldBytes) for nested message: %v", fieldMessage)
+			return hasUnknownNonCriticals, err
+		}
+
 		fieldBytes = fieldBytes[o:]
 
 		var err error