fix: Prevent signing from wrong key in multisig (#12548)
This commit is contained in:
parent
63d484a1b8
commit
bbf020f530
@ -82,6 +82,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
|
||||
|
||||
### Bug Fixes
|
||||
|
||||
* [#12548](https://github.com/cosmos/cosmos-sdk/pull/12548) Prevent signing from wrong key while using multisig.
|
||||
* [#12416](https://github.com/cosmos/cosmos-sdk/pull/12416) Prevent zero gas transactions in the `DeductFeeDecorator` AnteHandler decorator.
|
||||
* (x/mint) [#12384](https://github.com/cosmos/cosmos-sdk/pull/12384) Ensure `GoalBonded` must be positive when performing `x/mint` parameter validation.
|
||||
* (x/auth) [#12261](https://github.com/cosmos/cosmos-sdk/pull/12261) Deprecate pagination in GetTxsEventRequest/Response in favor of page and limit to align with tendermint `SignClient.TxSearch`
|
||||
|
||||
@ -9,7 +9,8 @@ import (
|
||||
"github.com/cosmos/cosmos-sdk/client"
|
||||
"github.com/cosmos/cosmos-sdk/client/flags"
|
||||
"github.com/cosmos/cosmos-sdk/client/tx"
|
||||
sdk "github.com/cosmos/cosmos-sdk/types"
|
||||
kmultisig "github.com/cosmos/cosmos-sdk/crypto/keys/multisig"
|
||||
|
||||
authclient "github.com/cosmos/cosmos-sdk/x/auth/client"
|
||||
)
|
||||
|
||||
@ -224,7 +225,6 @@ func makeSignCmd() func(cmd *cobra.Command, args []string) error {
|
||||
return err
|
||||
}
|
||||
|
||||
txFactory := tx.NewFactoryCLI(clientCtx, cmd.Flags())
|
||||
txCfg := clientCtx.TxConfig
|
||||
txBuilder, err := txCfg.WrapTxBuilder(newTx)
|
||||
if err != nil {
|
||||
@ -244,14 +244,39 @@ 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(clientCtx, txF.Keybase(), multisig)
|
||||
if err != nil {
|
||||
// Bech32 decode error, maybe it's a name, we try to fetch from keyring
|
||||
multisigAddr, _, _, err = client.GetFromFields(clientCtx, txFactory.Keybase(), multisig)
|
||||
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, err := multisigkey.GetPubKey()
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
multisigLegacyPub := multisigPubKey.(*kmultisig.LegacyAminoPubKey)
|
||||
|
||||
fromRecord, err := clientCtx.Keyring.Key(fromName)
|
||||
if err != nil {
|
||||
return fmt.Errorf("error getting account from keybase: %w", err)
|
||||
}
|
||||
fromPubKey, err := fromRecord.GetPubKey()
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
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 {
|
||||
|
||||
@ -74,6 +74,10 @@ func (s *IntegrationTestSuite) SetupSuite() {
|
||||
pub2, err := account2.GetPubKey()
|
||||
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{pub1, pub2})
|
||||
_, err = kb.SaveMultisig("multi", multi)
|
||||
s.Require().NoError(err)
|
||||
@ -938,6 +942,10 @@ func (s *IntegrationTestSuite) TestCLIMultisignSortSignatures() {
|
||||
multisigRecord, 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)
|
||||
|
||||
addr, err := multisigRecord.GetAddress()
|
||||
s.Require().NoError(err)
|
||||
resp, err := bankcli.QueryBalancesExec(val1.ClientCtx, addr)
|
||||
@ -995,7 +1003,7 @@ func (s *IntegrationTestSuite) TestCLIMultisignSortSignatures() {
|
||||
|
||||
sign1File := testutil.WriteToNewTempFile(s.T(), account1Signature.String())
|
||||
|
||||
// Sign with account1
|
||||
// Sign with account2
|
||||
addr2, err := account2.GetAddress()
|
||||
s.Require().NoError(err)
|
||||
account2Signature, err := TxSignExec(val1.ClientCtx, addr2, multiGeneratedTxFile.Name(), "--multisig", addr.String())
|
||||
@ -1003,6 +1011,13 @@ func (s *IntegrationTestSuite) TestCLIMultisignSortSignatures() {
|
||||
|
||||
sign2File := testutil.WriteToNewTempFile(s.T(), account2Signature.String())
|
||||
|
||||
// Sign with dummy account
|
||||
dummyAddr, err := dummyAcc.GetAddress()
|
||||
s.Require().NoError(err)
|
||||
_, err = TxSignExec(val1.ClientCtx, dummyAddr, multiGeneratedTxFile.Name(), "--multisig", addr.String())
|
||||
s.Require().Error(err)
|
||||
s.Require().Contains(err.Error(), "signing key is not a part of multisig key")
|
||||
|
||||
multiSigWith2Signatures, err := TxMultiSignExec(val1.ClientCtx, multisigRecord.Name, multiGeneratedTxFile.Name(), sign1File.Name(), sign2File.Name())
|
||||
s.Require().NoError(err)
|
||||
|
||||
@ -1057,7 +1072,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() {
|
||||
@ -1124,7 +1139,7 @@ func (s *IntegrationTestSuite) TestCLIMultisign() {
|
||||
|
||||
addr2, err := account2.GetAddress()
|
||||
s.Require().NoError(err)
|
||||
// Sign with account1
|
||||
// Sign with account2
|
||||
account2Signature, err := TxSignExec(val1.ClientCtx, addr2, multiGeneratedTxFile.Name(), "--multisig", addr.String())
|
||||
s.Require().NoError(err)
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user