chore: Enable gosec and fix findings (#83)

This update involves:
- Enabling gosec in the github workflow and updating the github workflow
versions.
- Update the codebase to fix findings from gosec. Highlights:
   - Some unhandled errors
- Config types allowed for very large values. For example, the retry
intervals and the commit gaps allowed for uint64. All values related to
block sizes were left to uint64, but the others were reduced to uint32.
   - Prometheus server protection against Slowloris

.github/workflows/ci.yml

@@ -7,14 +7,19 @@ on:
 
 jobs:
   lint_test:
-    uses: babylonlabs-io/.github/.github/workflows/reusable_go_lint_test.yml@v0.1.0
+    uses: babylonlabs-io/.github/.github/workflows/reusable_go_lint_test.yml@v0.6.0
     with:
       run-unit-tests: true
       run-integration-tests: true
       run-lint: true
+      run-build: true
+      run-gosec: true
+      gosec-args: "-exclude-generated -exclude-dir=itest -exclude-dir=testutil ./..."
 
   docker_pipeline:
-    uses: babylonlabs-io/.github/.github/workflows/reusable_docker_pipeline.yml@v0.1.0
+    uses: babylonlabs-io/.github/.github/workflows/reusable_docker_pipeline.yml@v0.6.0
     secrets: inherit
     with:
       publish: false
+      dockerfile: ./Dockerfile
+      repoName: finality-provider

.github/workflows/publish.yml

@@ -9,15 +9,20 @@ on:
 
 jobs:
   lint_test:
-    uses: babylonlabs-io/.github/.github/workflows/reusable_go_lint_test.yml@v0.1.0
+    uses: babylonlabs-io/.github/.github/workflows/reusable_go_lint_test.yml@v0.6.0
     with:
       run-unit-tests: true
       run-integration-tests: true
       run-lint: true
+      run-build: true
+      run-gosec: true
+      gosec-args: "-exclude-generated -exclude-dir=itest -exclude-dir=testutil ./..."
 
   docker_pipeline:
     needs: ["lint_test"]
-    uses: babylonlabs-io/.github/.github/workflows/reusable_docker_pipeline.yml@v0.1.0
+    uses: babylonlabs-io/.github/.github/workflows/reusable_docker_pipeline.yml@v0.6.0
     secrets: inherit
     with:
       publish: true
+      dockerfile: ./Dockerfile
+      repoName: finality-provider

clientcontroller/babylon.go

@@ -6,7 +6,7 @@ import (
 	"time"
 
 	sdkErr "cosmossdk.io/errors"
-	"cosmossdk.io/math"
+	sdkmath "cosmossdk.io/math"
 	bbnclient "github.com/babylonlabs-io/babylon/client/client"
 	bbntypes "github.com/babylonlabs-io/babylon/types"
 	btcctypes "github.com/babylonlabs-io/babylon/x/btccheckpoint/types"
@@ -55,7 +55,7 @@ func NewBabylonController(
 		return nil, fmt.Errorf("failed to create Babylon client: %w", err)
 	}
 
-	// makes sure that the key in config really exists and it is a valid bech 32 addr
+	// makes sure that the key in config really exists and is a valid bech32 addr
 	// to allow using mustGetTxSigner
 	if _, err := bc.GetAddr(); err != nil {
 		return nil, err
@@ -112,7 +112,7 @@ func (bc *BabylonController) reliablySendMsgs(msgs []sdk.Msg, expectedErrs []*sd
 func (bc *BabylonController) RegisterFinalityProvider(
 	fpPk *btcec.PublicKey,
 	pop []byte,
-	commission *math.LegacyDec,
+	commission *sdkmath.LegacyDec,
 	description []byte,
 ) (*types.TxResponse, error) {
 	var bbnPop btcstakingtypes.ProofOfPossessionBTC
@@ -323,13 +323,13 @@ func (bc *BabylonController) QueryLastCommittedPublicRand(fpPk *btcec.PublicKey,
 	return res.PubRandCommitMap, nil
 }
 
-func (bc *BabylonController) QueryBlocks(startHeight, endHeight, limit uint64) ([]*types.BlockInfo, error) {
+func (bc *BabylonController) QueryBlocks(startHeight, endHeight uint64, limit uint32) ([]*types.BlockInfo, error) {
 	if endHeight < startHeight {
 		return nil, fmt.Errorf("the startHeight %v should not be higher than the endHeight %v", startHeight, endHeight)
 	}
 	count := endHeight - startHeight + 1
-	if count > limit {
-		count = limit
+	if count > uint64(limit) {
+		count = uint64(limit)
 	}
 	return bc.queryLatestBlocks(sdk.Uint64ToBigEndian(startHeight), count, finalitytypes.QueriedBlockStatus_ANY, false)
 }
@@ -405,10 +405,14 @@ func (bc *BabylonController) queryCometBestBlock() (*types.BlockInfo, error) {
 		return nil, err
 	}
 
+	headerHeightInt64 := chainInfo.BlockMetas[0].Header.Height
+	if headerHeightInt64 < 0 {
+		return nil, fmt.Errorf("block height %v should be positive", headerHeightInt64)
+	}
 	// Returning response directly, if header with specified number did not exist
 	// at request will contain nil header
 	return &types.BlockInfo{
-		Height: uint64(chainInfo.BlockMetas[0].Header.Height),
+		Height: uint64(headerHeightInt64),
 		Hash:   chainInfo.BlockMetas[0].Header.AppHash,
 	}, nil
 }

clientcontroller/interface.go

@@ -59,7 +59,7 @@ type ClientController interface {
 	QueryBlock(height uint64) (*types.BlockInfo, error)
 
 	// QueryBlocks returns a list of blocks from startHeight to endHeight
-	QueryBlocks(startHeight, endHeight, limit uint64) ([]*types.BlockInfo, error)
+	QueryBlocks(startHeight, endHeight uint64, limit uint32) ([]*types.BlockInfo, error)
 
 	// QueryBestBlock queries the tip block of the consumer chain
 	QueryBestBlock() (*types.BlockInfo, error)

eotsmanager/cmd/eotsd/daemon/sign.go

@@ -186,6 +186,7 @@ func SignSchnorr(ctx *cli.Context) error {
 func hashFromFile(inputFilePath string) ([]byte, error) {
 	h := sha256.New()
 
+	// #nosec G304 - The log file path is provided by the user and not externally
 	f, err := os.Open(inputFilePath)
 	if err != nil {
 		return nil, fmt.Errorf("failed to open the file %s: %w", inputFilePath, err)

eotsmanager/service/server.go

@@ -66,8 +66,11 @@ func (s *Server) RunUntilShutdown() error {
 
 	defer func() {
 		s.logger.Info("Closing database...")
-		s.db.Close()
-		s.logger.Info("Database closed")
+		if err := s.db.Close(); err != nil {
+			s.logger.Error(fmt.Sprintf("Failed to close database: %v", err)) // Log the error
+		} else {
+			s.logger.Info("Database closed")
+		}
 		metricsServer.Stop(context.Background())
 		s.logger.Info("Metrics server stopped")
 	}()
@@ -79,7 +82,11 @@ func (s *Server) RunUntilShutdown() error {
 	if err != nil {
 		return fmt.Errorf("failed to listen on %s: %w", listenAddr, err)
 	}
-	defer lis.Close()
+	defer func() {
+		if err := lis.Close(); err != nil {
+			s.logger.Error(fmt.Sprintf("Failed to close network listener: %v", err))
+		}
+	}()
 
 	grpcServer := grpc.NewServer()
 	defer grpcServer.Stop()

finality-provider/cmd/fpd/daemon/daemon_commands.go

@@ -50,7 +50,11 @@ func runCommandGetDaemonInfo(cmd *cobra.Command, args []string) error {
 	if err != nil {
 		return err
 	}
-	defer cleanUp()
+	defer func() {
+		if err := cleanUp(); err != nil {
+			fmt.Printf("Failed to clean up grpc client: %v\n", err)
+		}
+	}()
 
 	info, err := client.GetInfo(context.Background())
 	if err != nil {
@@ -126,7 +130,11 @@ func runCommandCreateFP(ctx client.Context, cmd *cobra.Command, _ []string) erro
 	if err != nil {
 		return err
 	}
-	defer cleanUp()
+	defer func() {
+		if err := cleanUp(); err != nil {
+			fmt.Printf("Failed to clean up grpc client: %v\n", err)
+		}
+	}()
 
 	chainId, err := flags.GetString(chainIdFlag)
 	if err != nil {
@@ -202,7 +210,11 @@ func runCommandUnjailFP(_ client.Context, cmd *cobra.Command, args []string) err
 	if err != nil {
 		return err
 	}
-	defer cleanUp()
+	defer func() {
+		if err := cleanUp(); err != nil {
+			fmt.Printf("Failed to clean up grpc client: %v\n", err)
+		}
+	}()
 
 	_, err = client.UnjailFinalityProvider(context.Background(), args[0])
 	if err != nil {
@@ -263,7 +275,11 @@ func runCommandLsFP(cmd *cobra.Command, args []string) error {
 	if err != nil {
 		return err
 	}
-	defer cleanUp()
+	defer func() {
+		if err := cleanUp(); err != nil {
+			fmt.Printf("Failed to clean up grpc client: %v\n", err)
+		}
+	}()
 
 	resp, err := client.QueryFinalityProviderList(context.Background())
 	if err != nil {
@@ -303,7 +319,11 @@ func runCommandInfoFP(cmd *cobra.Command, args []string) error {
 	if err != nil {
 		return err
 	}
-	defer cleanUp()
+	defer func() {
+		if err := cleanUp(); err != nil {
+			fmt.Printf("Failed to clean up grpc client: %v\n", err)
+		}
+	}()
 
 	resp, err := client.QueryFinalityProviderInfo(context.Background(), fpPk)
 	if err != nil {
@@ -346,7 +366,11 @@ func runCommandRegisterFP(cmd *cobra.Command, args []string) error {
 	if err != nil {
 		return err
 	}
-	defer cleanUp()
+	defer func() {
+		if err := cleanUp(); err != nil {
+			fmt.Printf("Failed to clean up grpc client: %v\n", err)
+		}
+	}()
 
 	passphrase, err := flags.GetString(passphraseFlag)
 	if err != nil {
@@ -402,7 +426,11 @@ func runCommandAddFinalitySig(cmd *cobra.Command, args []string) error {
 	if err != nil {
 		return err
 	}
-	defer cleanUp()
+	defer func() {
+		if err := cleanUp(); err != nil {
+			fmt.Printf("Failed to clean up grpc client: %v\n", err)
+		}
+	}()
 
 	appHash, err := hex.DecodeString(appHashHex)
 	if err != nil {

finality-provider/cmd/fpd/daemon/export.go

@@ -64,7 +64,11 @@ func runCommandExportFP(ctx client.Context, cmd *cobra.Command, args []string) e
 	if err != nil {
 		return fmt.Errorf("failled to connect to daemon addr %s: %w", daemonAddress, err)
 	}
-	defer cleanUp()
+	defer func() {
+		if err := cleanUp(); err != nil {
+			fmt.Printf("Failed to clean up grpc client: %v\n", err)
+		}
+	}()
 
 	fpBtcPkHex := args[0]
 	fpPk, err := bbn.NewBIP340PubKeyFromHex(fpBtcPkHex)

finality-provider/config/config.go

@@ -58,15 +58,15 @@ type Config struct {
 	LogLevel string `long:"loglevel" description:"Logging level for all subsystems" choice:"trace" choice:"debug" choice:"info" choice:"warn" choice:"error" choice:"fatal"`
 	// ChainName and ChainID (if any) of the chain config identify a consumer chain
 	ChainName                string        `long:"chainname" description:"the name of the consumer chain" choice:"babylon"`
-	NumPubRand               uint64        `long:"numPubRand" description:"The number of Schnorr public randomness for each commitment"`
-	NumPubRandMax            uint64        `long:"numpubrandmax" description:"The upper bound of the number of Schnorr public randomness for each commitment"`
-	MinRandHeightGap         uint64        `long:"minrandheightgap" description:"The minimum gap between the last committed rand height and the current Babylon block height"`
+	NumPubRand               uint32        `long:"numPubRand" description:"The number of Schnorr public randomness for each commitment"`
+	NumPubRandMax            uint32        `long:"numpubrandmax" description:"The upper bound of the number of Schnorr public randomness for each commitment"`
+	MinRandHeightGap         uint32        `long:"minrandheightgap" description:"The minimum gap between the last committed rand height and the current Babylon block height"`
 	StatusUpdateInterval     time.Duration `long:"statusupdateinterval" description:"The interval between each update of finality-provider status"`
 	RandomnessCommitInterval time.Duration `long:"randomnesscommitinterval" description:"The interval between each attempt to commit public randomness"`
 	SubmissionRetryInterval  time.Duration `long:"submissionretryinterval" description:"The interval between each attempt to submit finality signature or public randomness after a failure"`
-	MaxSubmissionRetries     uint64        `long:"maxsubmissionretries" description:"The maximum number of retries to submit finality signature or public randomness"`
+	MaxSubmissionRetries     uint32        `long:"maxsubmissionretries" description:"The maximum number of retries to submit finality signature or public randomness"`
 	FastSyncInterval         time.Duration `long:"fastsyncinterval" description:"The interval between each try of fast sync, which is disabled if the value is 0"`
-	FastSyncLimit            uint64        `long:"fastsynclimit" description:"The maximum number of blocks to catch up for each fast sync"`
+	FastSyncLimit            uint32        `long:"fastsynclimit" description:"The maximum number of blocks to catch up for each fast sync"`
 	FastSyncGap              uint64        `long:"fastsyncgap" description:"The block gap that will trigger the fast sync"`
 	EOTSManagerAddress       string        `long:"eotsmanageraddress" description:"The address of the remote EOTS manager; Empty if the EOTS manager is running locally"`
 	MaxNumFinalityProviders  uint32        `long:"maxnumfinalityproviders" description:"The maximum number of finality-provider instances running concurrently within the daemon"`

finality-provider/service/client/rpcclient.go

@@ -18,14 +18,14 @@ type FinalityProviderServiceGRpcClient struct {
 }
 
 // NewFinalityProviderServiceGRpcClient creates a new GRPC connection with finality provider daemon.
-func NewFinalityProviderServiceGRpcClient(remoteAddr string) (client *FinalityProviderServiceGRpcClient, cleanUp func(), err error) {
+func NewFinalityProviderServiceGRpcClient(remoteAddr string) (client *FinalityProviderServiceGRpcClient, cleanUp func() error, err error) {
 	conn, err := grpc.Dial(remoteAddr, grpc.WithTransportCredentials(insecure.NewCredentials()))
 	if err != nil {
 		return nil, nil, fmt.Errorf("failed to build gRPC connection to %s: %w", remoteAddr, err)
 	}
 
-	cleanUp = func() {
-		conn.Close()
+	cleanUp = func() error {
+		return conn.Close()
 	}
 
 	return &FinalityProviderServiceGRpcClient{

finality-provider/service/eots_manager_adapter.go

@@ -11,12 +11,12 @@ import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 )
 
-func (fp *FinalityProviderInstance) getPubRandList(startHeight uint64, numPubRand uint64) ([]*btcec.FieldVal, error) {
+func (fp *FinalityProviderInstance) getPubRandList(startHeight uint64, numPubRand uint32) ([]*btcec.FieldVal, error) {
 	pubRandList, err := fp.em.CreateRandomnessPairList(
 		fp.btcPk.MustMarshal(),
 		fp.GetChainID(),
 		startHeight,
-		uint32(numPubRand),
+		numPubRand,
 		fp.passphrase,
 	)
 	if err != nil {

finality-provider/service/fastsync_test.go

@@ -52,7 +52,7 @@ func FuzzFastSync_SufficientRandomness(f *testing.F) {
 		expectedTxHash := testutil.GenRandomHexStr(r, 32)
 		finalizedBlock := &types.BlockInfo{Height: finalizedHeight, Hash: testutil.GenRandomByteArray(r, 32)}
 		mockClientController.EXPECT().QueryLatestFinalizedBlocks(uint64(1)).Return([]*types.BlockInfo{finalizedBlock}, nil).AnyTimes()
-		mockClientController.EXPECT().QueryBlocks(finalizedHeight+1, currentHeight, uint64(10)).
+		mockClientController.EXPECT().QueryBlocks(finalizedHeight+1, currentHeight, uint32(10)).
 			Return(catchUpBlocks, nil)
 		mockClientController.EXPECT().SubmitBatchFinalitySigs(fpIns.GetBtcPk(), catchUpBlocks, gomock.Any(), gomock.Any(), gomock.Any()).
 			Return(&types.TxResponse{TxHash: expectedTxHash}, nil).AnyTimes()
@@ -110,7 +110,7 @@ func FuzzFastSync_NoRandomness(f *testing.F) {
 		expectedTxHash := testutil.GenRandomHexStr(r, 32)
 		finalizedBlock := &types.BlockInfo{Height: finalizedHeight, Hash: testutil.GenRandomByteArray(r, 32)}
 		mockClientController.EXPECT().QueryLatestFinalizedBlocks(uint64(1)).Return([]*types.BlockInfo{finalizedBlock}, nil).AnyTimes()
-		mockClientController.EXPECT().QueryBlocks(finalizedHeight+1, currentHeight, uint64(10)).
+		mockClientController.EXPECT().QueryBlocks(finalizedHeight+1, currentHeight, uint32(10)).
 			Return(catchUpBlocks, nil)
 		mockClientController.EXPECT().SubmitBatchFinalitySigs(fpIns.GetBtcPk(), catchUpBlocks[:lastHeightWithPubRand-finalizedHeight], gomock.Any(), gomock.Any(), gomock.Any()).
 			Return(&types.TxResponse{TxHash: expectedTxHash}, nil).AnyTimes()

finality-provider/service/fp_instance.go

@@ -3,6 +3,7 @@ package service
 import (
 	"errors"
 	"fmt"
+	"math"
 	"strings"
 	"sync"
 	"time"
@@ -468,7 +469,7 @@ func (fp *FinalityProviderInstance) retrySubmitFinalitySignatureUntilBlockFinali
 			}
 
 			failedCycles += 1
-			if failedCycles > uint32(fp.cfg.MaxSubmissionRetries) {
+			if failedCycles > fp.cfg.MaxSubmissionRetries {
 				return nil, fmt.Errorf("reached max failed cycles with err: %w", err)
 			}
 		} else {
@@ -538,7 +539,7 @@ func (fp *FinalityProviderInstance) retryCommitPubRandUntilBlockFinalized(target
 			)
 
 			failedCycles += 1
-			if failedCycles > uint32(fp.cfg.MaxSubmissionRetries) {
+			if failedCycles > fp.cfg.MaxSubmissionRetries {
 				return nil, fmt.Errorf("reached max failed cycles with err: %w", err)
 			}
 		} else {
@@ -583,7 +584,7 @@ func (fp *FinalityProviderInstance) CommitPubRand(tipHeight uint64) (*types.TxRe
 	if lastCommittedHeight == uint64(0) {
 		// the finality-provider has never submitted public rand before
 		startHeight = tipHeight + 1
-	} else if lastCommittedHeight < fp.cfg.MinRandHeightGap+tipHeight {
+	} else if lastCommittedHeight < uint64(fp.cfg.MinRandHeightGap)+tipHeight {
 		// (should not use subtraction because they are in the type of uint64)
 		// we are running out of the randomness
 		startHeight = lastCommittedHeight + 1
@@ -683,8 +684,13 @@ func (fp *FinalityProviderInstance) SubmitBatchFinalitySignatures(blocks []*type
 		return nil, fmt.Errorf("should not submit batch finality signature with zero block")
 	}
 
+	if len(blocks) > math.MaxUint32 {
+		return nil, fmt.Errorf("should not submit batch finality signature with too many blocks")
+	}
+
 	// get public randomness list
-	prList, err := fp.getPubRandList(blocks[0].Height, uint64(len(blocks)))
+	// #nosec G115 -- performed the conversion check above
+	prList, err := fp.getPubRandList(blocks[0].Height, uint32(len(blocks)))
 	if err != nil {
 		return nil, fmt.Errorf("failed to get public randomness list: %v", err)
 	}