From 7d0f9514d3cd7b09127e6e7ac1b39d190de052dd Mon Sep 17 00:00:00 2001 From: Colin Axner Date: Tue, 10 Nov 2020 17:42:40 +0100 Subject: [PATCH 1/2] fix mismatched signature data type verification --- .../06-solomachine/types/proof.go | 24 +++---- .../06-solomachine/types/proof_test.go | 62 +++++++++++++++++++ 2 files changed, 75 insertions(+), 11 deletions(-) diff --git a/x/ibc/light-clients/06-solomachine/types/proof.go b/x/ibc/light-clients/06-solomachine/types/proof.go index c6f4c8b48f7..418fb192da9 100644 --- a/x/ibc/light-clients/06-solomachine/types/proof.go +++ b/x/ibc/light-clients/06-solomachine/types/proof.go @@ -19,28 +19,30 @@ import ( // verify the signature. An error is returned if signature verification fails // or an invalid SignatureData type is provided. func VerifySignature(pubKey cryptotypes.PubKey, signBytes []byte, sigData signing.SignatureData) error { - switch data := sigData.(type) { - case *signing.SingleSignatureData: - if !pubKey.VerifySignature(signBytes, data.Signature) { - return ErrSignatureVerificationFailed - } - - case *signing.MultiSignatureData: - multiPK, ok := pubKey.(multisig.PubKey) + switch pubKey := pubKey.(type) { + case multisig.PubKey: + data, ok := sigData.(*signing.MultiSignatureData) if !ok { - return sdkerrors.Wrapf(ErrSignatureVerificationFailed, "invalid pubkey type: expected %T, got %T", (multisig.PubKey)(nil), pubKey) + return sdkerrors.Wrapf(ErrSignatureVerificationFailed, "invalid signature data type, expected %T, got %T", (*signing.MultiSignatureData)(nil), data) } // The function supplied fulfills the VerifyMultisignature interface. No special // adjustments need to be made to the sign bytes based on the sign mode. - if err := multiPK.VerifyMultisignature(func(signing.SignMode) ([]byte, error) { + if err := pubKey.VerifyMultisignature(func(signing.SignMode) ([]byte, error) { return signBytes, nil }, data); err != nil { return err } default: - return sdkerrors.Wrapf(ErrSignatureVerificationFailed, "unsupported signature data type %T", data) + data, ok := sigData.(*signing.SingleSignatureData) + if !ok { + return sdkerrors.Wrapf(ErrSignatureVerificationFailed, "invalid signature data type, expected %T, got %T", (*signing.SingleSignatureData)(nil), data) + } + + if !pubKey.VerifySignature(signBytes, data.Signature) { + return ErrSignatureVerificationFailed + } } return nil diff --git a/x/ibc/light-clients/06-solomachine/types/proof_test.go b/x/ibc/light-clients/06-solomachine/types/proof_test.go index 5ecd9c4c5da..e2ba679a5b9 100644 --- a/x/ibc/light-clients/06-solomachine/types/proof_test.go +++ b/x/ibc/light-clients/06-solomachine/types/proof_test.go @@ -1,10 +1,72 @@ package types_test import ( + cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" + "github.com/cosmos/cosmos-sdk/types/tx/signing" "github.com/cosmos/cosmos-sdk/x/ibc/light-clients/06-solomachine/types" + solomachinetypes "github.com/cosmos/cosmos-sdk/x/ibc/light-clients/06-solomachine/types" ibctesting "github.com/cosmos/cosmos-sdk/x/ibc/testing" ) +func (suite *SoloMachineTestSuite) TestVerifySignature() { + cdc := suite.chainA.App.AppCodec() + signBytes := []byte("sign bytes") + + singleSignature := suite.solomachine.GenerateSignature(signBytes) + singleSigData, err := solomachinetypes.UnmarshalSignatureData(cdc, singleSignature) + suite.Require().NoError(err) + + multiSignature := suite.solomachineMulti.GenerateSignature(signBytes) + multiSigData, err := solomachinetypes.UnmarshalSignatureData(cdc, multiSignature) + suite.Require().NoError(err) + + testCases := []struct { + name string + publicKey cryptotypes.PubKey + sigData signing.SignatureData + expPass bool + }{ + { + "single signature with regular public key", + suite.solomachine.PublicKey, + singleSigData, + true, + }, + { + "multi signature with multisig public key", + suite.solomachineMulti.PublicKey, + multiSigData, + true, + }, + { + "single signature with multisig public key", + suite.solomachineMulti.PublicKey, + singleSigData, + false, + }, + { + "multi signature with regular public key", + suite.solomachine.PublicKey, + multiSigData, + false, + }, + } + + for _, tc := range testCases { + tc := tc + + suite.Run(tc.name, func() { + err := solomachinetypes.VerifySignature(tc.publicKey, signBytes, tc.sigData) + + if tc.expPass { + suite.Require().NoError(err) + } else { + suite.Require().Error(err) + } + }) + } +} + func (suite *SoloMachineTestSuite) TestClientStateSignBytes() { cdc := suite.chainA.App.AppCodec() From ae29eab78683542c2e907be788012049aa4c22a0 Mon Sep 17 00:00:00 2001 From: Colin Axner Date: Tue, 10 Nov 2020 17:47:17 +0100 Subject: [PATCH 2/2] update godoc --- x/ibc/light-clients/06-solomachine/types/proof.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/x/ibc/light-clients/06-solomachine/types/proof.go b/x/ibc/light-clients/06-solomachine/types/proof.go index 418fb192da9..6c2e0b84288 100644 --- a/x/ibc/light-clients/06-solomachine/types/proof.go +++ b/x/ibc/light-clients/06-solomachine/types/proof.go @@ -15,9 +15,9 @@ import ( // VerifySignature verifies if the the provided public key generated the signature // over the given data. Single and Multi signature public keys are supported. -// The type of the signature data determines how the public key is used to -// verify the signature. An error is returned if signature verification fails -// or an invalid SignatureData type is provided. +// The signature data type must correspond to the public key type. An error is +// returned if signature verification fails or an invalid SignatureData type is +// provided. func VerifySignature(pubKey cryptotypes.PubKey, signBytes []byte, sigData signing.SignatureData) error { switch pubKey := pubKey.(type) { case multisig.PubKey: