From 58f7e942f226840ca7a0ee8955c621cd56519dd2 Mon Sep 17 00:00:00 2001 From: Nishant Das Date: Mon, 14 Mar 2022 07:30:11 +0800 Subject: [PATCH] Fix Mnemonic Validation (#10354) * fix mnemonic validation * potuz's review * Add fuzz test for wallet recover Co-authored-by: prestonvanloon --- validator/accounts/BUILD.bazel | 2 + ...9fd97f9e0d0c6ac83eb528677d918f0ac9be5f4b8d | 2 + validator/accounts/wallet_recover.go | 17 ++++-- .../accounts/wallet_recover_fuzz_test.go | 19 +++++++ validator/accounts/wallet_recover_test.go | 54 +++++++++++++++++++ 5 files changed, 89 insertions(+), 5 deletions(-) create mode 100644 validator/accounts/testdata/fuzz/FuzzValidateMnemonic/0258716f94e00f9df0da869fd97f9e0d0c6ac83eb528677d918f0ac9be5f4b8d create mode 100644 validator/accounts/wallet_recover_fuzz_test.go diff --git a/validator/accounts/BUILD.bazel b/validator/accounts/BUILD.bazel index 71abec0c3..7b2cffc79 100644 --- a/validator/accounts/BUILD.bazel +++ b/validator/accounts/BUILD.bazel @@ -70,8 +70,10 @@ go_test( "accounts_list_test.go", "wallet_create_test.go", "wallet_edit_test.go", + "wallet_recover_fuzz_test.go", "wallet_recover_test.go", ], + data = glob(["testdata/**"]), embed = [":go_default_library"], deps = [ "//async/event:go_default_library", diff --git a/validator/accounts/testdata/fuzz/FuzzValidateMnemonic/0258716f94e00f9df0da869fd97f9e0d0c6ac83eb528677d918f0ac9be5f4b8d b/validator/accounts/testdata/fuzz/FuzzValidateMnemonic/0258716f94e00f9df0da869fd97f9e0d0c6ac83eb528677d918f0ac9be5f4b8d new file mode 100644 index 000000000..d4075f475 --- /dev/null +++ b/validator/accounts/testdata/fuzz/FuzzValidateMnemonic/0258716f94e00f9df0da869fd97f9e0d0c6ac83eb528677d918f0ac9be5f4b8d @@ -0,0 +1,2 @@ +go test fuzz v1 +string("0 ") diff --git a/validator/accounts/wallet_recover.go b/validator/accounts/wallet_recover.go index 2be5724ba..e2dce6763 100644 --- a/validator/accounts/wallet_recover.go +++ b/validator/accounts/wallet_recover.go @@ -34,6 +34,11 @@ const ( mnemonicPassphrasePromptText = "(Advanced) Enter the '25th word' passphrase for your mnemonic" ) +var ( + ErrIncorrectWordNumber = errors.New("incorrect number of words provided") + ErrEmptyMnemonic = errors.New("phrase cannot be empty") +) + // RecoverWalletConfig to run the recover wallet function. type RecoverWalletConfig struct { WalletDir string @@ -228,16 +233,18 @@ func inputNumAccounts(cliCtx *cli.Context) (int64, error) { // as specified(currently 24). func ValidateMnemonic(mnemonic string) error { if strings.Trim(mnemonic, " ") == "" { - return errors.New("phrase cannot be empty") + return ErrEmptyMnemonic } words := strings.Split(mnemonic, " ") - for i, word := range words { + validWordCount := 0 + for _, word := range words { if strings.Trim(word, " ") == "" { - words = append(words[:i], words[i+1:]...) + continue } + validWordCount += 1 } - if len(words) != phraseWordCount { - return fmt.Errorf("phrase must be %d words, entered %d", phraseWordCount, len(words)) + if validWordCount != phraseWordCount { + return errors.Wrapf(ErrIncorrectWordNumber, "phrase must be %d words, entered %d", phraseWordCount, validWordCount) } return nil } diff --git a/validator/accounts/wallet_recover_fuzz_test.go b/validator/accounts/wallet_recover_fuzz_test.go new file mode 100644 index 000000000..a393be48f --- /dev/null +++ b/validator/accounts/wallet_recover_fuzz_test.go @@ -0,0 +1,19 @@ +//go:build go1.18 +// +build go1.18 + +package accounts_test + +import ( + "testing" + + "github.com/prysmaticlabs/prysm/validator/accounts" +) + +func FuzzValidateMnemonic(f *testing.F) { + f.Add("bag wagon luxury iron swarm yellow course merge trumpet wide decide cash idea disagree deny teach canvas profit slice beach iron sting warfare slide") + f.Add("bag wagon luxury iron swarm yellow course merge trumpet wide cash idea disagree deny teach canvas profit iron sting warfare slide") + f.Add("bag wagon luxury iron swarm yellow course merge trumpet cash wide decide profit cash idea disagree deny teach canvas profit slice beach iron sting warfare slide") + f.Fuzz(func(t *testing.T, input string) { + accounts.ValidateMnemonic(input) + }) +} diff --git a/validator/accounts/wallet_recover_test.go b/validator/accounts/wallet_recover_test.go index b2cecc00d..edffab8d3 100644 --- a/validator/accounts/wallet_recover_test.go +++ b/validator/accounts/wallet_recover_test.go @@ -9,6 +9,7 @@ import ( "strconv" "testing" + "github.com/pkg/errors" "github.com/prysmaticlabs/prysm/cmd/validator/flags" "github.com/prysmaticlabs/prysm/testing/assert" "github.com/prysmaticlabs/prysm/testing/require" @@ -106,3 +107,56 @@ func TestRecoverDerivedWallet_AlreadyExists(t *testing.T) { // Trying to recover an HD wallet into a directory that already exists should give an error require.ErrorContains(t, "a wallet already exists at this location", RecoverWalletCli(cliCtx)) } + +func TestValidateMnemonic(t *testing.T) { + tests := []struct { + name string + mnemonic string + wantErr bool + err error + }{ + { + name: "Valid Mnemonic", + mnemonic: "bag wagon luxury iron swarm yellow course merge trumpet wide decide cash idea disagree deny teach canvas profit slice beach iron sting warfare slide", + wantErr: false, + }, + { + name: "Invalid Mnemonic with too few words", + mnemonic: "bag wagon luxury iron swarm yellow course merge trumpet wide cash idea disagree deny teach canvas profit iron sting warfare slide", + wantErr: true, + err: ErrIncorrectWordNumber, + }, + { + name: "Invalid Mnemonic with too many words", + mnemonic: "bag wagon luxury iron swarm yellow course merge trumpet cash wide decide profit cash idea disagree deny teach canvas profit slice beach iron sting warfare slide", + wantErr: true, + err: ErrIncorrectWordNumber, + }, + { + name: "Empty Mnemonic", + mnemonic: "", + wantErr: true, + err: ErrEmptyMnemonic, + }, + { + name: "Junk Mnemonic", + mnemonic: " a ", + wantErr: true, + err: ErrIncorrectWordNumber, + }, + { + name: "Junk Mnemonic ", + mnemonic: " evilpacket ", + wantErr: true, + err: ErrIncorrectWordNumber, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateMnemonic(tt.mnemonic) + if (err != nil) != tt.wantErr || (tt.wantErr && !errors.Is(err, tt.err)) { + t.Errorf("ValidateMnemonic() error = %v, wantErr %v", err, tt.err) + } + }) + } +}