LaconicNetwork/cosmos-sdk
9
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 2 Wiki Activity

fix: Prevent signing from wrong key in multisig (#12446)

Browse Source
This commit is contained in:
likhita-809 2022-07-12 19:12:33 +05:30 committed by GitHub
parent 055989339d
commit 28f4fb9c39
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 108 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
client/keys/show_test.go
View File

@ -31,7 +31,7 @@ func Test_multiSigKey_Properties(t *testing.T) {
require.Equal(t, "myMultisig", k.Name)
require.Equal(t, keyring.TypeMulti, k.GetType())
pub, err := k.GetPubKey()
pub, err := k.GetMultisigPubKey()
require.NoError(t, err)
require.Equal(t, "D3923267FA8A3DD367BB768FA8BDC8FF7F89DA3F", pub.Address().String())

2
crypto/keyring/output_test.go
View File

@ -20,7 +20,7 @@ func TestBech32KeysOutput(t *testing.T) {
k, err := NewMultiRecord("multisig", multisigPk)
require.NotNil(t, k)
require.NoError(t, err)
pubKey, err := k.GetPubKey()
pubKey, err := k.GetMultisigPubKey()
require.NoError(t, err)
accAddr := sdk.AccAddress(pubKey.Address())
expectedOutput, err := NewKeyOutput(k.Name, k.GetType(), accAddr, multisigPk)

11
crypto/keyring/record.go
View File

@ -5,6 +5,7 @@ import (
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
"github.com/cosmos/cosmos-sdk/crypto/hd"
"github.com/cosmos/cosmos-sdk/crypto/keys/multisig"
cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
"github.com/cosmos/cosmos-sdk/types"
)
@ -59,6 +60,16 @@ func NewMultiRecord(name string, pk cryptotypes.PubKey) (*Record, error) {
return newRecord(name, pk, recordMultiItem)
}
// GetMultisigPubKey fetches a public key of the multi type record
func (k *Record) GetMultisigPubKey() (*multisig.LegacyAminoPubKey, error) {
pk, ok := k.PubKey.GetCachedValue().(*multisig.LegacyAminoPubKey)
if !ok {
return nil, errors.New("unable to cast any to multisig.LegacyAminoPubKey")
}
return pk, nil
}
// GetPubKey fetches a public key of the record
func (k *Record) GetPubKey() (cryptotypes.PubKey, error) {
pk, ok := k.PubKey.GetCachedValue().(cryptotypes.PubKey)

45
crypto/keyring/record_test.go
View File

@ -10,6 +10,8 @@ import (
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
"github.com/cosmos/cosmos-sdk/crypto/hd"
"github.com/cosmos/cosmos-sdk/crypto/keys/ed25519"
"github.com/cosmos/cosmos-sdk/crypto/keys/multisig"
"github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1"
cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
)
@ -17,10 +19,11 @@ import (
type RecordTestSuite struct {
suite.Suite
appName string
cdc codec.Codec
priv cryptotypes.PrivKey
pub cryptotypes.PubKey
appName string
cdc codec.Codec
priv cryptotypes.PrivKey
pub cryptotypes.PubKey
multiPub *multisig.LegacyAminoPubKey
}
func (s *RecordTestSuite) SetupSuite() {
@ -28,6 +31,14 @@ func (s *RecordTestSuite) SetupSuite() {
s.cdc = getCodec()
s.priv = cryptotypes.PrivKey(ed25519.GenPrivKey())
s.pub = s.priv.PubKey()
multiKey := secp256k1.GenPrivKeyFromSecret([]byte("myMultiKey"))
pk := multisig.NewLegacyAminoPubKey(
1,
[]cryptotypes.PubKey{multiKey.PubKey()},
)
s.multiPub = pk
}
func (s *RecordTestSuite) TestOfflineRecordMarshaling() {
@ -47,6 +58,32 @@ func (s *RecordTestSuite) TestOfflineRecordMarshaling() {
s.Require().True(s.pub.Equals(pk2))
}
func (s *RecordTestSuite) TestMultiRecordMarshaling() {
dir := s.T().TempDir()
mockIn := strings.NewReader("")
kb, err := New(s.appName, BackendTest, dir, mockIn, s.cdc)
s.Require().NoError(err)
k, err := NewMultiRecord("testrecord", s.multiPub)
s.Require().NoError(err)
ks, ok := kb.(keystore)
s.Require().True(ok)
bz, err := ks.cdc.Marshal(k)
s.Require().NoError(err)
k2, err := ks.protoUnmarshalRecord(bz)
s.Require().NoError(err)
s.Require().Equal(k.Name, k2.Name)
s.Require().True(k.PubKey.Equal(k2.PubKey))
pub2, err := k2.GetMultisigPubKey()
s.Require().NoError(err)
s.Require().True(s.multiPub.Equals(pub2))
}
func (s *RecordTestSuite) TestLocalRecordMarshaling() {
dir := s.T().TempDir()
mockIn := strings.NewReader("")

44
x/auth/client/cli/tx_sign.go
View File

@ -224,7 +224,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 {
@ -232,7 +231,7 @@ func makeSignCmd() func(cmd *cobra.Command, args []string) error {
}
printSignatureOnly, _ := cmd.Flags().GetBool(flagSigOnly)
multisig, _ := cmd.Flags().GetString(flagMultisig)
multisigkey, _ := cmd.Flags().GetString(flagMultisig)
if err != nil {
return err
}
@ -243,15 +242,44 @@ func makeSignCmd() func(cmd *cobra.Command, args []string) error {
}
overwrite, _ := f.GetBool(flagOverwrite)
if multisig != "" {
multisigAddr, err := sdk.AccAddressFromBech32(multisig)
if multisigkey != "" {
var (
multisigName string
multisigAddr sdk.AccAddress
)
// Bech32 decode error, maybe it's a name, we try to fetch from keyring
multisigAddr, multisigName, _, err = client.GetFromFields(clientCtx, txF.Keybase(), multisigkey)
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)
}
multisigRecord, err := clientCtx.Keyring.Key(multisigName)
if err != nil {
return err
}
multisigPub, err := multisigRecord.GetMultisigPubKey()
if err != nil {
return err
}
fromRecord, err := clientCtx.Keyring.Key(fromName)
if err != nil {
return err
}
fromPubKey, err := fromRecord.GetPubKey()
if err != nil {
return err
}
var found bool
for _, pubkey := range multisigPub.GetPubKeys() {
if pubkey.Equals(fromPubKey) {
found = true
}
}
if !found {
return fmt.Errorf("from key is not a part of multisig key")
}
err = authclient.SignTxWithSignerAddress(
txF, clientCtx, multisigAddr, fromName, txBuilder, clientCtx.Offline, overwrite)
if err != nil {

21
x/auth/client/testutil/suite.go
View File

@ -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(), "from 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)