From 35227c463649df0238613944b2238313740a64e1 Mon Sep 17 00:00:00 2001 From: HaoyangLiu Date: Tue, 27 Jun 2023 14:49:42 +0800 Subject: [PATCH] [R4R] - {0.4.2}: Resolve audit feedback from Sigma: MNT-9, MNT-10 and MNT-12 (#1131) * ensure key length is no less than 16 * add signature length checker * return pointer instead of struct and return the relevant err1 or err2 instead of err * ensure sig length is no less than 64 --- tss/manager/sign.go | 11 ++++++--- tss/node/tsslib/keysign.go | 7 +++--- tss/node/tsslib/keysign/tss_keysign.go | 32 +++++++++++++++----------- tss/node/tsslib/storage/shamir_mgr.go | 10 +++++--- tss/ws/server/handler.go | 4 ++++ 5 files changed, 41 insertions(+), 23 deletions(-) diff --git a/tss/manager/sign.go b/tss/manager/sign.go index b2c02e4b7..bb3ab7ea3 100644 --- a/tss/manager/sign.go +++ b/tss/manager/sign.go @@ -6,13 +6,14 @@ import ( "encoding/json" "errors" "fmt" - "github.com/influxdata/influxdb/pkg/slices" - "github.com/mantlenetworkio/mantle/l2geth/crypto" - tmjson "github.com/tendermint/tendermint/libs/json" "strings" "sync" "time" + "github.com/influxdata/influxdb/pkg/slices" + "github.com/mantlenetworkio/mantle/l2geth/crypto" + tmjson "github.com/tendermint/tendermint/libs/json" + "github.com/mantlenetworkio/mantle/l2geth/log" tss "github.com/mantlenetworkio/mantle/tss/common" "github.com/mantlenetworkio/mantle/tss/manager/types" @@ -87,6 +88,10 @@ func (m Manager) sign(ctx types.Context, request interface{}, digestBz []byte, m } poolPubKeyBz, _ := hex.DecodeString(ctx.TssInfos().ClusterPubKey) + if len(signResponse.Signature) < 64 { + log.Error(fmt.Sprintf("invalid signature, expected length is no less than 64, actual length is %d", len(signResponse.Signature))) + return + } if !crypto.VerifySignature(poolPubKeyBz, digestBz, signResponse.Signature[:64]) { log.Error("illegal signature") return diff --git a/tss/node/tsslib/keysign.go b/tss/node/tsslib/keysign.go index 5ae7204ec..0a5800dc3 100644 --- a/tss/node/tsslib/keysign.go +++ b/tss/node/tsslib/keysign.go @@ -4,6 +4,9 @@ import ( "encoding/hex" "errors" "fmt" + "strings" + "time" + "github.com/libp2p/go-libp2p/core/peer" "github.com/mantlenetworkio/mantle/tss/node/tsslib/abnormal" "github.com/mantlenetworkio/mantle/tss/node/tsslib/common" @@ -11,8 +14,6 @@ import ( keysign2 "github.com/mantlenetworkio/mantle/tss/node/tsslib/keysign" "github.com/mantlenetworkio/mantle/tss/node/tsslib/messages" "github.com/mantlenetworkio/mantle/tss/node/tsslib/storage" - "strings" - "time" ) func (t *TssServer) generateSignature(onlinePeers []peer.ID, req keysign2.Request, localStateItem storage.KeygenLocalState, keysignInstance *keysign2.TssKeySign) (keysign2.Response, error) { @@ -56,7 +57,7 @@ func (t *TssServer) generateSignature(onlinePeers []peer.ID, req keysign2.Reques } return keysign2.NewResponse( - &signatureData, + signatureData, common.Success, "", nil, diff --git a/tss/node/tsslib/keysign/tss_keysign.go b/tss/node/tsslib/keysign/tss_keysign.go index 1773bd684..6aa0f2523 100644 --- a/tss/node/tsslib/keysign/tss_keysign.go +++ b/tss/node/tsslib/keysign/tss_keysign.go @@ -4,6 +4,9 @@ import ( "crypto/ecdsa" "errors" "fmt" + "sync" + "time" + tsscommon "github.com/binance-chain/tss-lib/common" "github.com/binance-chain/tss-lib/ecdsa/signing" "github.com/binance-chain/tss-lib/tss" @@ -16,8 +19,6 @@ import ( "github.com/mantlenetworkio/mantle/tss/node/tsslib/storage" "github.com/rs/zerolog" "github.com/rs/zerolog/log" - "sync" - "time" ) type TssKeySign struct { @@ -60,16 +61,15 @@ func (tKeySign *TssKeySign) GetTssCommonStruct() *common2.TssCommon { } // signMessage -func (tKeySign *TssKeySign) SignMessage(msgToSign []byte, localStateItem storage.KeygenLocalState, parties []string) (tsscommon.SignatureData, error) { +func (tKeySign *TssKeySign) SignMessage(msgToSign []byte, localStateItem storage.KeygenLocalState, parties []string) (*tsscommon.SignatureData, error) { partiesID, localPartyID, err := conversion.GetParties(parties, localStateItem.LocalPartyKey) - var emptySignatureData tsscommon.SignatureData if err != nil { - return emptySignatureData, fmt.Errorf("fail to form key sign party: %w", err) + return nil, fmt.Errorf("fail to form key sign party: %w", err) } if !common2.Contains(partiesID, localPartyID) { tKeySign.logger.Info().Msgf("we are not in this rounds key sign") - return emptySignatureData, nil + return nil, nil } outCh := make(chan tss.Message, 2*len(partiesID)) @@ -78,13 +78,13 @@ func (tKeySign *TssKeySign) SignMessage(msgToSign []byte, localStateItem storage m, err := common2.MsgToHashInt(msgToSign) if err != nil { - return emptySignatureData, fmt.Errorf("fail to convert msg to hash int: %w", err) + return nil, fmt.Errorf("fail to convert msg to hash int: %w", err) } moniker := m.String() partiesID, eachLocalPartyID, err := conversion.GetParties(parties, localStateItem.LocalPartyKey) ctx := tss.NewPeerContext(partiesID) if err != nil { - return emptySignatureData, fmt.Errorf("error to create parties in batch signging %w\n", err) + return nil, fmt.Errorf("error to create parties in batch signging %w\n", err) } tKeySign.logger.Info().Msgf("message: (%s) keysign parties: %+v", m.String(), parties) @@ -95,11 +95,15 @@ func (tKeySign *TssKeySign) SignMessage(msgToSign []byte, localStateItem storage abnormalMgr := tKeySign.tssCommonStruct.GetAbnormalMgr() partyIDMap := conversion.SetupPartyIDMap(partiesID) - err1 := conversion.SetupIDMaps(partyIDMap, tKeySign.tssCommonStruct.PartyIDtoP2PID) - err2 := conversion.SetupIDMaps(partyIDMap, abnormalMgr.PartyIDtoP2PID) - if err1 != nil || err2 != nil { + err = conversion.SetupIDMaps(partyIDMap, tKeySign.tssCommonStruct.PartyIDtoP2PID) + if err != nil { + tKeySign.logger.Error().Err(err).Msgf("error in creating mapping between partyID and P2P ID") + return nil, err + } + err = conversion.SetupIDMaps(partyIDMap, abnormalMgr.PartyIDtoP2PID) + if err != nil { tKeySign.logger.Error().Err(err).Msgf("error in creating mapping between partyID and P2P ID") - return emptySignatureData, err + return nil, err } tKeySign.tssCommonStruct.SetPartyInfo(&abnormal.PartyInfo{ @@ -127,7 +131,7 @@ func (tKeySign *TssKeySign) SignMessage(msgToSign []byte, localStateItem storage result, err := tKeySign.processKeySign(errCh, outCh, endCh) if err != nil { close(tKeySign.commStopChan) - return emptySignatureData, fmt.Errorf("fail to process key sign: %w", err) + return nil, fmt.Errorf("fail to process key sign: %w", err) } select { @@ -138,7 +142,7 @@ func (tKeySign *TssKeySign) SignMessage(msgToSign []byte, localStateItem storage } keySignWg.Wait() tKeySign.logger.Info().Msgf("%s successfully sign the message", tKeySign.p2pComm.GetHost().ID().String()) - return result, nil + return &result, nil } func (tKeySign *TssKeySign) processKeySign(errChan chan struct{}, outCh <-chan tss.Message, endCh <-chan tsscommon.SignatureData) (tsscommon.SignatureData, error) { diff --git a/tss/node/tsslib/storage/shamir_mgr.go b/tss/node/tsslib/storage/shamir_mgr.go index d9cb988e0..7a2c5650b 100644 --- a/tss/node/tsslib/storage/shamir_mgr.go +++ b/tss/node/tsslib/storage/shamir_mgr.go @@ -7,6 +7,10 @@ import ( "encoding/json" "errors" "fmt" + "sort" + "strconv" + "strings" + sssas "github.com/SSSaaS/sssa-golang" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/credentials" @@ -19,9 +23,6 @@ import ( "github.com/google/uuid" nodeconfig "github.com/mantlenetworkio/mantle/tss/common" "github.com/rs/zerolog/log" - "sort" - "strconv" - "strings" ) const ( @@ -546,6 +547,9 @@ func NewSession(region, id, secret string) (*session.Session, error) { } func createUUID(key string) (string, error) { + if len(key) < 16 { + return "", fmt.Errorf("key length should not be less than 16") + } uuidBytes := make([]byte, 16) keyBytes := []byte(key) copy(uuidBytes, keyBytes[len(keyBytes)-16:len(keyBytes)]) diff --git a/tss/ws/server/handler.go b/tss/ws/server/handler.go index 13e1766c8..6021e2e0c 100644 --- a/tss/ws/server/handler.go +++ b/tss/ws/server/handler.go @@ -180,6 +180,10 @@ func (wm *WebsocketManager) WebsocketHandler(w http.ResponseWriter, r *http.Requ wm.logger.Error("hex decode error for pubkey or sig", "err", err) return } + if len(sigBytes) < 64 { + wm.logger.Error(fmt.Sprintf("invalid sigBytes, expected length is no less than 64, actual length is %d", len(sigBytes))) + return + } digestBz := crypto.Keccak256Hash([]byte(timeStr)).Bytes() if !crypto.VerifySignature(pubKeyBytes, digestBz, sigBytes[:64]) { wm.logger.Error("illegal signature", "publicKey", pubKey, "time", timeStr, "signature", sig)