diff --git a/CHANGELOG.md b/CHANGELOG.md index eb197ca3e7..dd9dc08861 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,11 +43,33 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Improvements ### Bug Fixes +<<<<<<< HEAD * (x/auth) [#1281](https://github.com/Finschia/finschia-sdk/pull/1281) `ModuleAccount.Validate` now reports a nil `.BaseAccount` instead of panicking. (backport #1274) * (x/foundation) [\#1283](https://github.com/Finschia/finschia-sdk/pull/1283) add init logic of foundation module accounts to InitGenesis in order to eliminate potential panic (backport #1277) * (x/collection) [\#1282](https://github.com/Finschia/finschia-sdk/pull/1282) eliminates potential risk for Insufficient Sanity Check of tokenID in Genesis (backport #1276) * (x/collection) [\#1290](https://github.com/Finschia/finschia-sdk/pull/1290) export x/collection params into genesis (backport #1268) * (x/foundation) [\#1295](https://github.com/Finschia/finschia-sdk/pull/1295) add missing error handling for migration +======= +* chore(deps) [\#1141](https://github.com/Finschia/finschia-sdk/pull/1141) Bump github.com/cosmos/ledger-cosmos-go from 0.12.2 to 0.13.2 to fix ledger signing issue +* (x/auth, x/slashing) [\#1179](https://github.com/Finschia/finschia-sdk/pull/1179) modify missing changes of converting to tendermint +* (x/auth) [#1274](https://github.com/Finschia/finschia-sdk/pull/1274) `ModuleAccount.Validate` now reports a nil `.BaseAccount` instead of panicking. +* (x/collection) [\#1276](https://github.com/Finschia/finschia-sdk/pull/1276) eliminates potential risk for Insufficient Sanity Check of tokenID in Genesis +* (x/foundation) [\#1277](https://github.com/Finschia/finschia-sdk/pull/1277) add init logic of foundation module accounts to InitGenesis in order to eliminate potential panic +* (x/collection, x/token) [\#1288](https://github.com/Finschia/finschia-sdk/pull/1288) use accAddress to compare in validatebasic function in collection & token modules +* (x/collection) [\#1268](https://github.com/Finschia/finschia-sdk/pull/1268) export x/collection params into genesis +* (x/collection) [\#1294](https://github.com/Finschia/finschia-sdk/pull/1294) reject NFT coins on FT APIs +* (sec) [\#1302](https://github.com/Finschia/finschia-sdk/pull/1302) remove map iteration non-determinism with keys + sorting +* (client) [\#1303](https://github.com/Finschia/finschia-sdk/pull/1303) fix possible overflow in BuildUnsignedTx +* (types) [\#1299](https://github.com/Finschia/finschia-sdk/pull/1299) add missing nil checks +* (x/staking) [\#1301](https://github.com/Finschia/finschia-sdk/pull/1301) Use bytes instead of string comparison in delete validator queue (backport cosmos/cosmos-sdk#12303) +* (x/gov) [\#1304](https://github.com/Finschia/finschia-sdk/pull/1304) fetch a failed proposal tally from proposal.FinalTallyResult in the gprc query +* (x/staking) [\#1306](https://github.com/Finschia/finschia-sdk/pull/1306) add validation for potential slashing evasion during re-delegation +* (client/keys) [#1312](https://github.com/Finschia/finschia-sdk/pull/1312) ignore error when key not found in `keys delete` +* (store) [\#1310](https://github.com/Finschia/finschia-sdk/pull/1310) fix app-hash mismatch if upgrade migration commit is interrupted(backport cosmos/cosmos-sdk#13530) +* (types) [\#1313](https://github.com/Finschia/finschia-sdk/pull/1313) fix correctly coalesce coins even with repeated denominations(backport cosmos/cosmos-sdk#13265) +* (x/crypto) [\#1316](https://github.com/Finschia/finschia-sdk/pull/1316) error if incorrect ledger public key (backport cosmos/cosmos-sdk#14460, cosmos/cosmos-sdk#19691) +* (x/auth) [#1319](https://github.com/Finschia/finschia-sdk/pull/1319) prevent signing from wrong key in multisig +>>>>>>> c051dcc91 (fix: prevent signing from wrong key in multisig (#1319)) ### Removed diff --git a/x/auth/client/cli/tx_multisign.go b/x/auth/client/cli/tx_multisign.go index 907f4f746b..c32e9077db 100644 --- a/x/auth/client/cli/tx_multisign.go +++ b/x/auth/client/cli/tx_multisign.go @@ -413,3 +413,13 @@ func getMultisigInfo(clientCtx client.Context, name string) (keyring.Info, error return multisigInfo, nil } + +func getMultisigRecord(clientCtx client.Context, name string) (keyring.Info, error) { + kb := clientCtx.Keyring + multisigRecord, err := kb.Key(name) + if err != nil { + return nil, errors.Wrap(err, "error getting keybase multisig account") + } + + return multisigRecord, nil +} diff --git a/x/auth/client/cli/tx_sign.go b/x/auth/client/cli/tx_sign.go index af534068dc..c522d3d596 100644 --- a/x/auth/client/cli/tx_sign.go +++ b/x/auth/client/cli/tx_sign.go @@ -9,7 +9,7 @@ import ( "github.com/Finschia/finschia-sdk/client" "github.com/Finschia/finschia-sdk/client/flags" "github.com/Finschia/finschia-sdk/client/tx" - sdk "github.com/Finschia/finschia-sdk/types" + kmultisig "github.com/Finschia/finschia-sdk/crypto/keys/multisig" authclient "github.com/Finschia/finschia-sdk/x/auth/client" ) @@ -258,14 +258,33 @@ func makeSignCmd() func(cmd *cobra.Command, args []string) error { overwrite, _ := f.GetBool(flagOverwrite) if multisig != "" { - multisigAddr, err := sdk.AccAddressFromBech32(multisig) + // Bech32 decode error, maybe it's a name, we try to fetch from keyring + multisigAddr, multisigName, _, err := client.GetFromFields(txFactory.Keybase(), multisig, clientCtx.GenerateOnly) if err != nil { - // Bech32 decode error, maybe it's a name, we try to fetch from keyring - multisigAddr, _, _, err = client.GetFromFields(txFactory.Keybase(), multisig, clientCtx.GenerateOnly) - if err != nil { - return fmt.Errorf("error getting account from keybase: %w", err) + return fmt.Errorf("error getting account from keybase: %w", err) + } + multisigkey, err := getMultisigRecord(clientCtx, multisigName) + if err != nil { + return err + } + multisigPubKey := multisigkey.GetPubKey() + multisigLegacyPub := multisigPubKey.(*kmultisig.LegacyAminoPubKey) + + fromRecord, err := clientCtx.Keyring.Key(fromName) + if err != nil { + return fmt.Errorf("error getting account from keybase: %w", err) + } + fromPubKey := fromRecord.GetPubKey() + + var found bool + for _, pubkey := range multisigLegacyPub.GetPubKeys() { + if pubkey.Equals(fromPubKey) { + found = true } } + if !found { + return fmt.Errorf("signing key is not a part of multisig key") + } err = authclient.SignTxWithSignerAddress( txF, clientCtx, multisigAddr, fromName, txBuilder, clientCtx.Offline, overwrite) if err != nil { diff --git a/x/auth/client/testutil/suite.go b/x/auth/client/testutil/suite.go index 2841f187f5..f163eb1bdf 100644 --- a/x/auth/client/testutil/suite.go +++ b/x/auth/client/testutil/suite.go @@ -62,6 +62,10 @@ func (s *IntegrationTestSuite) SetupSuite() { account2, _, err := kb.NewMnemonic("newAccount2", keyring.English, sdk.FullFundraiserPath, keyring.DefaultBIP39Passphrase, hd.Secp256k1) s.Require().NoError(err) + // Create a dummy account for testing purpose + _, _, err = kb.NewMnemonic("dummyAccount", keyring.English, sdk.FullFundraiserPath, keyring.DefaultBIP39Passphrase, hd.Secp256k1) + s.Require().NoError(err) + multi := kmultisig.NewLegacyAminoPubKey(2, []cryptotypes.PubKey{account1.GetPubKey(), account2.GetPubKey()}) _, err = kb.SaveMultisig("multi", multi) s.Require().NoError(err) @@ -790,6 +794,10 @@ func (s *IntegrationTestSuite) TestCLIMultisignSortSignatures() { multisigInfo, err := val1.ClientCtx.Keyring.Key("multi") s.Require().NoError(err) + // Generate dummy account which is not a part of multisig. + dummyAcc, err := val1.ClientCtx.Keyring.Key("dummyAccount") + s.Require().NoError(err) + resp, err := bankcli.QueryBalancesExec(val1.ClientCtx, multisigInfo.GetAddress()) s.Require().NoError(err) @@ -842,12 +850,18 @@ func (s *IntegrationTestSuite) TestCLIMultisignSortSignatures() { sign1File := testutil.WriteToNewTempFile(s.T(), account1Signature.String()) - // Sign with account1 + // Sign with account2 account2Signature, err := TxSignExec(val1.ClientCtx, account2.GetAddress(), multiGeneratedTxFile.Name(), "--multisig", multisigInfo.GetAddress().String()) s.Require().NoError(err) sign2File := testutil.WriteToNewTempFile(s.T(), account2Signature.String()) + // Sign with dummy account + dummyAddr := dummyAcc.GetAddress() + _, err = TxSignExec(val1.ClientCtx, dummyAddr, multiGeneratedTxFile.Name(), "--multisig", multisigInfo.GetAddress().String()) + s.Require().Error(err) + s.Require().Contains(err.Error(), "signing key is not a part of multisig key") + multiSigWith2Signatures, err := TxMultiSignExec(val1.ClientCtx, multisigInfo.GetName(), multiGeneratedTxFile.Name(), sign1File.Name(), sign2File.Name()) s.Require().NoError(err) @@ -902,7 +916,7 @@ func (s *IntegrationTestSuite) TestSignWithMultisig() { // as the main point of this test is to test the `--multisig` flag with an address // that is not in the keyring. _, err = TxSignExec(val1.ClientCtx, addr1, multiGeneratedTx2File.Name(), "--multisig", multisigAddr.String()) - s.Require().Contains(err.Error(), "tx intended signer does not match the given signer") + s.Require().Contains(err.Error(), "error getting account from keybase") } func (s *IntegrationTestSuite) TestCLIMultisign() {