[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

@@ -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

tss/node/tsslib/keysign.go

@@ -4,15 +4,16 @@ 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"
 	"github.com/mantlenetworkio/mantle/tss/node/tsslib/conversion"
 	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,

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) {

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)])

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)