chore(*): add gosec (#94)

Adds gosec

.github/workflows/ci.yml

@@ -14,6 +14,8 @@ jobs:
      run-unit-tests: true
      run-integration-tests: true
      run-lint: true
+     run-gosec: true
+     gosec-args: "-exclude-generated -exclude-dir=e2etest -exclude-dir=testutil ./..."
      install-dependencies-command: |
        sudo apt-get update
        sudo apt-get install -y libzmq3-dev

.github/workflows/publish.yml

@@ -16,6 +16,8 @@ jobs:
       run-unit-tests: true
       run-integration-tests: true
       run-lint: true
+      run-gosec: true
+      gosec-args: "-exclude-generated -exclude-dir=e2etest -exclude-dir=testutil ./..."
       install-dependencies-command: |
         sudo apt-get update
         sudo apt-get install -y libzmq3-dev

CHANGELOG.md

@@ -37,6 +37,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 
 ## Unreleased
 
+* [#94](https://github.com/babylonlabs-io/vigilante/pull/94) adds gosec and fixes gosec issues
+
 ## v0.15.0
 
 * [#90](https://github.com/babylonlabs-io/vigilante/pull/90) upgrade babylon to v0.15.0

Makefile

@@ -81,6 +81,15 @@ proto-gen:
 .PHONY: proto-gen
 
 
+###############################################################################
+###                                Gosec                                    ###
+###############################################################################
+
+gosec-local: ## Run local security checks
+	gosec -exclude-generated -exclude-dir=$(CURDIR)/testutil -exclude-dir=$(CURDIR)/e2etest $(CURDIR)/...
+
+.PHONY: gosec-local
+
 ###############################################################################
 ###                                Release                                  ###
 ###############################################################################

btcclient/query.go

@@ -2,6 +2,7 @@ package btcclient
 
 import (
 	"fmt"
+	"math"
 
 	"github.com/avast/retry-go/v4"
 	"github.com/btcsuite/btcd/btcjson"
@@ -19,6 +20,10 @@ func (c *Client) GetBestBlock() (uint32, error) {
 		return 0, err
 	}
 
+	if height < 0 || height > int64(math.MaxUint32) {
+		panic(fmt.Errorf("height (%d) is out of uint32 range", height)) //software bug, panic
+	}
+
 	return uint32(height), nil
 }
 
@@ -34,7 +39,11 @@ func (c *Client) GetBlockByHash(blockHash *chainhash.Hash) (*types.IndexedBlock,
 	}
 
 	btcTxs := types.GetWrappedTxs(mBlock)
-	return types.NewIndexedBlock(int32(blockInfo.Height), &mBlock.Header, btcTxs), mBlock, nil
+	height := blockInfo.Height
+	if height < 0 || height > int64(math.MaxUint32) {
+		panic(fmt.Errorf("height (%d) is out of uint32 range", height)) //software bug, panic
+	}
+	return types.NewIndexedBlock(uint32(height), &mBlock.Header, btcTxs), mBlock, nil
 }
 
 // GetBlockByHeight returns a block with the given height
@@ -51,7 +60,7 @@ func (c *Client) GetBlockByHeight(height uint32) (*types.IndexedBlock, *wire.Msg
 
 	btcTxs := types.GetWrappedTxs(mBlock)
 
-	return types.NewIndexedBlock(int32(height), &mBlock.Header, btcTxs), mBlock, nil
+	return types.NewIndexedBlock(height, &mBlock.Header, btcTxs), mBlock, nil
 }
 
 func (c *Client) getBestBlockHashWithRetry() (*chainhash.Hash, error) {
@@ -153,7 +162,7 @@ func (c *Client) getBlockVerboseWithRetry(hash *chainhash.Hash) (*btcjson.GetBlo
 // getChainBlocks returns a chain of indexed blocks from the block at baseHeight to the tipBlock
 // note: the caller needs to ensure that tipBlock is on the blockchain
 func (c *Client) getChainBlocks(baseHeight uint32, tipBlock *types.IndexedBlock) ([]*types.IndexedBlock, error) {
-	tipHeight := uint32(tipBlock.Height)
+	tipHeight := tipBlock.Height
 	if tipHeight < baseHeight {
 		return nil, fmt.Errorf("the tip block height %v is less than the base height %v", tipHeight, baseHeight)
 	}
@@ -201,7 +210,7 @@ func (c *Client) FindTailBlocksByHeight(baseHeight uint32) ([]*types.IndexedBloc
 		return nil, err
 	}
 
-	if baseHeight > uint32(tipIb.Height) {
+	if baseHeight > tipIb.Height {
 		return nil, fmt.Errorf("invalid base height %d, should not be higher than tip block %d", baseHeight, tipIb.Height)
 	}
 

btcstaking-tracker/atomicslasher/routines.go

@@ -1,6 +1,7 @@
 package atomicslasher
 
 import (
+	"fmt"
 	"time"
 
 	bstypes "github.com/babylonlabs-io/babylon/x/btcstaking/types"
@@ -58,6 +59,9 @@ func (as *AtomicSlasher) slashingTxTracker() {
 				return
 			}
 			// record BTC tip
+			if blockEpoch.Height < 0 {
+				panic(fmt.Errorf("received negative block height: %d", blockEpoch.Height))
+			}
 			as.btcTipHeight.Store(uint32(blockEpoch.Height))
 			as.logger.Debug("Received new best btc block", zap.Int32("height", blockEpoch.Height))
 			// get full BTC block

btcstaking-tracker/btcslasher/slasher_utils.go

@@ -3,6 +3,7 @@ package btcslasher
 import (
 	"encoding/hex"
 	"fmt"
+	"math"
 	"strings"
 
 	"github.com/avast/retry-go/v4"
@@ -213,6 +214,10 @@ func BuildUnbondingSlashingTxWithWitness(
 		return nil, fmt.Errorf("failed to convert covenant pks to BTC pks: %v", err)
 	}
 
+	if d.UnbondingTime > uint32(^uint16(0)) {
+		panic(fmt.Errorf("unbondingTime (%d) exceeds maximum for uint16", d.UnbondingTime))
+	}
+
 	// get unbonding info
 	unbondingInfo, err := btcstaking.BuildUnbondingInfo(
 		d.BtcPk.MustToBTCPK(),
@@ -316,7 +321,12 @@ func BuildSlashingTxWithWitness(
 		return nil, fmt.Errorf("failed to convert covenant pks to BTC pks: %v", err)
 	}
 
+	if d.TotalSat > math.MaxInt64 {
+		panic(fmt.Errorf("TotalSat %d exceeds int64 range", d.TotalSat))
+	}
+
 	// get staking info
+	// #nosec G115 -- performed the conversion check above
 	stakingInfo, err := btcstaking.BuildStakingInfo(
 		d.BtcPk.MustToBTCPK(),
 		fpBtcPkList,

btcstaking-tracker/stakingeventwatcher/stakingeventwatcher.go

@@ -119,6 +119,9 @@ func (sew *StakingEventWatcher) Start() error {
 		// we registered for notifications with `nil` so we should receive best block immediately
 		select {
 		case block := <-blockEventNotifier.Epochs:
+			if block.Height < 0 {
+				panic(fmt.Errorf("received negative block height: %d", block.Height))
+			}
 			sew.currentBestBlockHeight.Store(uint32(block.Height))
 		case <-sew.quit:
 			startErr = errors.New("watcher quit before finishing start")
@@ -158,6 +161,9 @@ func (sew *StakingEventWatcher) handleNewBlocks(blockNotifier *notifier.BlockEpo
 			if !ok {
 				return
 			}
+			if block.Height < 0 {
+				panic(fmt.Errorf("received negative block height: %d", block.Height))
+			}
 			sew.currentBestBlockHeight.Store(uint32(block.Height))
 			sew.logger.Debugf("Received new best btc block: %d", block.Height)
 		case <-sew.quit:
@@ -183,7 +189,7 @@ func (sew *StakingEventWatcher) checkBabylonDelegations(status btcstakingtypes.B
 			addDel(delegation)
 		}
 
-		if len(delegations) < int(sew.cfg.NewDelegationsBatchSize) {
+		if uint64(len(delegations)) < sew.cfg.NewDelegationsBatchSize {
 			// we received fewer delegations than we asked for; it means went through all of them
 			return nil
 		}
@@ -451,7 +457,7 @@ func (sew *StakingEventWatcher) buildSpendingTxProof(spendingTx *wire.MsgTx) (*b
 	}
 
 	btcTxs := types.GetWrappedTxs(details.Block)
-	ib := types.NewIndexedBlock(int32(details.BlockHeight), &details.Block.Header, btcTxs)
+	ib := types.NewIndexedBlock(details.BlockHeight, &details.Block.Header, btcTxs)
 
 	proof, err := ib.GenSPVProof(int(details.TxIndex))
 	if err != nil {
@@ -596,7 +602,7 @@ func (sew *StakingEventWatcher) checkBtcForStakingTx() error {
 		}
 
 		btcTxs := types.GetWrappedTxs(details.Block)
-		ib := types.NewIndexedBlock(int32(details.BlockHeight), &details.Block.Header, btcTxs)
+		ib := types.NewIndexedBlock(details.BlockHeight, &details.Block.Header, btcTxs)
 
 		proof, err := ib.GenSPVProof(int(details.TxIndex))
 		if err != nil {

config/monitor.go

@@ -20,13 +20,13 @@ type MonitorConfig struct {
 	// Max number of BTC blocks in the buffer
 	BtcBlockBufferSize uint64 `mapstructure:"btc-block-buffer-size"`
 	// Max number of BTC blocks in the cache
-	BtcCacheSize uint64 `mapstructure:"btc-cache-size"`
+	BtcCacheSize uint32 `mapstructure:"btc-cache-size"`
 	// Intervals between each liveness check in seconds
 	LivenessCheckIntervalSeconds uint64 `mapstructure:"liveness-check-interval-seconds"`
 	// Max lasting BTC heights that a checkpoint is not reported before an alarm is sent
 	MaxLiveBtcHeights uint64 `mapstructure:"max-live-btc-heights"`
 	// the confirmation depth to consider a BTC block as confirmed
-	BtcConfirmationDepth uint64 `mapstructure:"btc-confirmation-depth"`
+	BtcConfirmationDepth uint32 `mapstructure:"btc-confirmation-depth"`
 	// whether to enable liveness checker
 	EnableLivenessChecker bool `mapstructure:"enable-liveness-checker"`
 	// DatabaseConfig stores lates epoch and height used for faster bootstrap

config/reporter.go

@@ -14,7 +14,7 @@ const (
 // ReporterConfig defines configuration for the reporter.
 type ReporterConfig struct {
 	NetParams       string `mapstructure:"netparams"`          // should be mainnet|testnet|simnet|signet
-	BTCCacheSize    uint64 `mapstructure:"btc_cache_size"`     // size of the BTC cache
+	BTCCacheSize    uint32 `mapstructure:"btc_cache_size"`     // size of the BTC cache
 	MaxHeadersInMsg uint32 `mapstructure:"max_headers_in_msg"` // maximum number of headers in a MsgInsertHeaders message
 }
 

config/submitter.go

@@ -22,7 +22,7 @@ type SubmitterConfig struct {
 	// ResubmitFeeMultiplier is used to multiply the estimated bumped fee in resubmission
 	ResubmitFeeMultiplier float64 `mapstructure:"resubmit-fee-multiplier"`
 	// PollingIntervalSeconds defines the intervals (in seconds) between each polling of Babylon checkpoints
-	PollingIntervalSeconds uint `mapstructure:"polling-interval-seconds"`
+	PollingIntervalSeconds int64 `mapstructure:"polling-interval-seconds"`
 	// ResendIntervalSeconds defines the time (in seconds) which the submitter awaits
 	// before resubmitting checkpoints to BTC
 	ResendIntervalSeconds uint `mapstructure:"resend-interval-seconds"`
@@ -39,6 +39,10 @@ func (cfg *SubmitterConfig) Validate() error {
 		return errors.New("invalid resubmit-fee-multiplier, should not be less than 1")
 	}
 
+	if cfg.PollingIntervalSeconds < 0 {
+		return errors.New("invalid polling-interval-seconds, should be positive")
+	}
+
 	return nil
 }
 

metrics/prometheus.go

@@ -2,8 +2,9 @@ package metrics
 
 import (
 	"net/http"
-	_ "net/http/pprof"
+	_ "net/http/pprof" // #nosec G108 we want this
 	"regexp"
+	"time"
 
 	"github.com/babylonlabs-io/vigilante/config"
 	"github.com/prometheus/client_golang/prometheus"
@@ -37,5 +38,12 @@ func start(addr string, reg *prometheus.Registry) {
 	}
 	log := metricsLogger.With(zap.String("module", "metrics")).Sugar()
 	log.Infof("Successfully started Prometheus metrics server at %s", addr)
-	log.Fatal(http.ListenAndServe(addr, nil))
+	srv := &http.Server{
+		Addr:         addr,
+		ReadTimeout:  10 * time.Second,
+		WriteTimeout: 10 * time.Second,
+	}
+	if err := srv.ListenAndServe(); err != nil {
+		log.Fatalf("Failed to start metrics server: %v", err)
+	}
 }

monitor/btcscanner/block_handler.go

@@ -3,6 +3,7 @@ package btcscanner
 import (
 	"errors"
 	"fmt"
+	"math"
 
 	"github.com/btcsuite/btcd/wire"
 	"github.com/lightningnetwork/lnd/chainntnfs"
@@ -17,10 +18,13 @@ func (bs *BtcScanner) bootstrapAndBlockEventHandler() {
 	var blockEpoch *chainntnfs.BlockEpoch
 	bestKnownBlock := bs.unconfirmedBlockCache.Tip()
 	if bestKnownBlock != nil {
+		if bestKnownBlock.Height > math.MaxInt32 {
+			panic(fmt.Errorf("block height exceeds int32 range: %d", bestKnownBlock.Height))
+		}
 		hash := bestKnownBlock.BlockHash()
 		blockEpoch = &chainntnfs.BlockEpoch{
 			Hash:        &hash,
-			Height:      bestKnownBlock.Height,
+			Height:      int32(bestKnownBlock.Height),
 			BlockHeader: bestKnownBlock.Header,
 		}
 	}
@@ -43,7 +47,11 @@ func (bs *BtcScanner) bootstrapAndBlockEventHandler() {
 				return // channel closed
 			}
 
-			if err := bs.handleNewBlock(epoch.Height, epoch.BlockHeader); err != nil {
+			if epoch.Height < 0 {
+				panic(fmt.Errorf("received negative epoch height: %d", epoch.Height)) // software bug, panic
+			}
+
+			if err := bs.handleNewBlock(uint32(epoch.Height), epoch.BlockHeader); err != nil {
 				bs.logger.Warnf("failed to handle block at height %d: %s, "+
 					"need to restart the bootstrapping process", epoch.Height, err.Error())
 				bs.Bootstrap()
@@ -54,7 +62,7 @@ func (bs *BtcScanner) bootstrapAndBlockEventHandler() {
 
 // handleNewBlock handles blocks from the BTC client
 // if new confirmed blocks are found, send them through the channel
-func (bs *BtcScanner) handleNewBlock(height int32, header *wire.BlockHeader) error {
+func (bs *BtcScanner) handleNewBlock(height uint32, header *wire.BlockHeader) error {
 	// get cache tip
 	cacheTip := bs.unconfirmedBlockCache.Tip()
 	if cacheTip == nil {
@@ -91,7 +99,12 @@ func (bs *BtcScanner) handleNewBlock(height int32, header *wire.BlockHeader) err
 	// otherwise, add the block to the cache
 	bs.unconfirmedBlockCache.Add(ib)
 
+	if bs.unconfirmedBlockCache.Size() > math.MaxInt32 {
+		panic(fmt.Errorf("unconfirmedBlockCache exceeds uint32"))
+	}
+
 	// still unconfirmed
+	// #nosec G115 -- performed the conversion check above
 	if uint32(bs.unconfirmedBlockCache.Size()) <= bs.k {
 		return nil
 	}

monitor/btcscanner/btc_scanner.go

@@ -78,7 +78,7 @@ func New(
 		logger:                parentLogger.With(zap.String("module", "btcscanner")).Sugar(),
 		btcClient:             btcClient,
 		btcNotifier:           btcNotifier,
-		k:                     uint32(monitorCfg.BtcConfirmationDepth),
+		k:                     monitorCfg.BtcConfirmationDepth,
 		ckptCache:             ckptCache,
 		unconfirmedBlockCache: unconfirmedBlockCache,
 		confirmedBlocksChan:   confirmedBlocksChan,
@@ -143,9 +143,9 @@ func (bs *BtcScanner) Bootstrap() {
 	)
 
 	if bs.confirmedTipBlock != nil {
-		firstUnconfirmedHeight = uint32(bs.confirmedTipBlock.Height + 1)
+		firstUnconfirmedHeight = bs.confirmedTipBlock.Height + 1
 	} else {
-		firstUnconfirmedHeight = uint32(bs.GetBaseHeight())
+		firstUnconfirmedHeight = bs.GetBaseHeight()
 	}
 
 	bs.logger.Infof("the bootstrapping starts at %d", firstUnconfirmedHeight)
@@ -251,7 +251,7 @@ func (bs *BtcScanner) matchAndPop() (*types.CheckpointRecord, error) {
 
 	return &types.CheckpointRecord{
 		RawCheckpoint:      rawCheckpoint,
-		FirstSeenBtcHeight: uint32(ckptSegments.Segments[0].AssocBlock.Height),
+		FirstSeenBtcHeight: ckptSegments.Segments[0].AssocBlock.Height,
 	}, nil
 }
 

monitor/btcscanner/btc_scanner_test.go

@@ -36,7 +36,7 @@ func FuzzBootStrap(f *testing.F) {
 				Return(chainIndexedBlocks[i], nil, nil).AnyTimes()
 		}
 
-		cache, err := types.NewBTCCache(numBlocks)
+		cache, err := types.NewBTCCache(uint32(numBlocks))
 		require.NoError(t, err)
 		var btcScanner btcscanner.BtcScanner
 		btcScanner.SetBtcClient(mockBtcClient)

monitor/liveness_checker.go

@@ -2,6 +2,7 @@ package monitor
 
 import (
 	"fmt"
+	"math"
 	"time"
 
 	"github.com/pkg/errors"
@@ -12,6 +13,10 @@ import (
 )
 
 func (m *Monitor) runLivenessChecker() {
+	if m.Cfg.LivenessCheckIntervalSeconds > uint64(math.MaxInt64/time.Second) {
+		panic(fmt.Errorf("LivenessCheckIntervalSeconds %d exceeds int64 range when converted to time.Duration", m.Cfg.LivenessCheckIntervalSeconds))
+	}
+	// #nosec G115 -- performed the conversion check above
 	ticker := time.NewTicker(time.Duration(m.Cfg.LivenessCheckIntervalSeconds) * time.Second)
 
 	m.logger.Infof("liveness checker is started, checking liveness every %d seconds", m.Cfg.LivenessCheckIntervalSeconds)
@@ -89,7 +94,7 @@ func (m *Monitor) CheckLiveness(cr *types.CheckpointRecord) error {
 		return fmt.Errorf("the gap %d between two BTC heights should not be negative", gap)
 	}
 
-	if gap > int(m.Cfg.MaxLiveBtcHeights) {
+	if uint64(gap) > m.Cfg.MaxLiveBtcHeights {
 		return fmt.Errorf("%w: the gap BTC height is %d, larger than the threshold %d", types.ErrLivenessAttack, gap, m.Cfg.MaxLiveBtcHeights)
 	}