fix: prevent simple inclusion of a valid mnemonic (#13670)

CHANGELOG.md

@@ -54,6 +54,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * (x/auth) [#13210](https://github.com/cosmos/cosmos-sdk/pull/13210) Add `Query/AccountInfo` endpoint for simplified access to basic account info.
 * (x/consensus) [#12905](https://github.com/cosmos/cosmos-sdk/pull/12905) Create a new `x/consensus` module that is now responsible for maintaining Tendermint consensus parameters instead of `x/param`. Legacy types remain in order to facilitate parameter migration from the deprecated `x/params`. App developers should ensure that they execute `baseapp.MigrateParams` during their chain upgrade. These legacy types will be removed in a future release.
 * (x/auth) [#13612](https://github.com/cosmos/cosmos-sdk/pull/13612) Add `Query/ModuleAccountByName` endpoint for accessing the module account info by module name.
+* (client/tx) [#13670](https://github.com/cosmos/cosmos-sdk/pull/13670) Add validation in `BuildUnsignedTx` to prevent simple inclusion of valid mnemonics
 
 ### Improvements
 

client/tx/factory.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"fmt"
 	"os"
+	"strings"
 
 	"cosmossdk.io/math"
 	"github.com/spf13/pflag"
@@ -16,6 +17,7 @@ import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/types/tx"
 	"github.com/cosmos/cosmos-sdk/types/tx/signing"
+	"github.com/cosmos/go-bip39"
 )
 
 // Factory defines a client transaction factory that facilitates generating and
@@ -298,6 +300,11 @@ func (f Factory) BuildUnsignedTx(msgs ...sdk.Msg) (client.TxBuilder, error) {
 		}
 	}
 
+	// Prevent simple inclusion of a valid mnemonic in the memo field
+	if f.memo != "" && bip39.IsMnemonicValid(strings.ToLower(f.memo)) {
+		return nil, errors.New("cannot provide a valid mnemonic seed in the memo field")
+	}
+
 	tx := f.txConfig.NewTxBuilder()
 
 	if err := tx.SetMsgs(msgs...); err != nil {

client/tx/tx_test.go

@@ -3,6 +3,7 @@ package tx_test
 import (
 	gocontext "context"
 	"fmt"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -161,6 +162,55 @@ func TestBuildUnsignedTx(t *testing.T) {
 	require.Empty(t, sigs)
 }
 
+func TestMnemonicInMemo(t *testing.T) {
+	txConfig, cdc := newTestTxConfig(t)
+	kb, err := keyring.New(t.Name(), "test", t.TempDir(), nil, cdc)
+	require.NoError(t, err)
+
+	path := hd.CreateHDPath(118, 0, 0).String()
+
+	_, seed, err := kb.NewMnemonic("test_key1", keyring.English, path, keyring.DefaultBIP39Passphrase, hd.Secp256k1)
+	require.NoError(t, err)
+
+	testCases := []struct {
+		name  string
+		memo  string
+		error bool
+	}{
+		{name: "bare seed", memo: seed, error: true},
+		{name: "padding bare seed", memo: fmt.Sprintf("   %s", seed), error: true},
+		{name: "prefixed", memo: fmt.Sprintf("%s: %s", "prefixed: ", seed), error: false},
+		{name: "normal memo", memo: "this is a memo", error: false},
+		{name: "empty memo", memo: "", error: false},
+		{name: "invalid mnemonic", memo: strings.Repeat("egg", 24), error: false},
+		{name: "caps", memo: strings.ToUpper(seed), error: true},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			txf := tx.Factory{}.
+				WithTxConfig(txConfig).
+				WithAccountNumber(50).
+				WithSequence(23).
+				WithFees("50stake").
+				WithMemo(tc.memo).
+				WithChainID("test-chain").
+				WithKeybase(kb)
+
+			msg := banktypes.NewMsgSend(sdk.AccAddress("from"), sdk.AccAddress("to"), nil)
+			tx, err := txf.BuildUnsignedTx(msg)
+			if tc.error {
+				require.Error(t, err)
+				require.ErrorContains(t, err, "mnemonic")
+				require.Nil(t, tx)
+			} else {
+				require.NoError(t, err)
+				require.NotNil(t, tx)
+			}
+		})
+	}
+}
+
 func TestSign(t *testing.T) {
 	txConfig, cdc := newTestTxConfig(t)
 	requireT := require.New(t)