Skip to content

Commit

Permalink
fix: prevent simple inclusion of a valid mnemonic
Browse files Browse the repository at this point in the history
  • Loading branch information
arirubinstein committed Oct 26, 2022
1 parent 6b4db20 commit a5fe7df
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
7 changes: 7 additions & 0 deletions client/tx/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"errors"
"fmt"
"os"
"strings"

"cosmossdk.io/math"
"github.com/spf13/pflag"
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
50 changes: 50 additions & 0 deletions client/tx/tx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package tx_test
import (
gocontext "context"
"fmt"
"strings"
"testing"

"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit a5fe7df

Please sign in to comment.