From 2d2468cd07f10db45acc1a08546ab5ada7d77471 Mon Sep 17 00:00:00 2001 From: Lazar <12626340+Lazar955@users.noreply.github.com> Date: Tue, 5 Nov 2024 14:49:16 +0100 Subject: [PATCH] chore(*): add gosec (#94) Adds gosec --- .github/workflows/ci.yml | 2 ++ .github/workflows/publish.yml | 2 ++ CHANGELOG.md | 2 ++ Makefile | 9 ++++++ btcclient/query.go | 17 +++++++--- btcstaking-tracker/atomicslasher/routines.go | 4 +++ .../btcslasher/slasher_utils.go | 10 ++++++ .../stakingeventwatcher.go | 12 +++++-- config/monitor.go | 4 +-- config/reporter.go | 2 +- config/submitter.go | 6 +++- metrics/prometheus.go | 12 +++++-- monitor/btcscanner/block_handler.go | 19 +++++++++-- monitor/btcscanner/btc_scanner.go | 8 ++--- monitor/btcscanner/btc_scanner_test.go | 2 +- monitor/liveness_checker.go | 7 +++- monitor/monitor.go | 4 +++ reporter/block_handler.go | 8 +++-- reporter/bootstrapping.go | 2 +- reporter/utils.go | 2 +- reporter/utils_test.go | 4 +-- rpcserver/tls.go | 2 +- submitter/relayer/change_address.go | 8 ++--- submitter/relayer/relayer.go | 14 +++++--- testutil/datagen/reporter.go | 4 +-- types/btccache.go | 32 +++++++++---------- types/btccache_test.go | 16 +++++----- types/genesis_info.go | 6 +++- types/indexed_block.go | 8 ++--- types/indexed_block_test.go | 2 +- 30 files changed, 161 insertions(+), 69 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c97e4e31..62640f33 100644 --- a/.github/workflows/ci.yml +++ b/.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 diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index d3ebff4f..138728e5 100644 --- a/.github/workflows/publish.yml +++ b/.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 diff --git a/CHANGELOG.md b/CHANGELOG.md index aec5708c..54ce6384 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/Makefile b/Makefile index b8eb2990..a519fb29 100644 --- a/Makefile +++ b/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 ### ############################################################################### diff --git a/btcclient/query.go b/btcclient/query.go index c7a4225d..5cfa99ee 100644 --- a/btcclient/query.go +++ b/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) } diff --git a/btcstaking-tracker/atomicslasher/routines.go b/btcstaking-tracker/atomicslasher/routines.go index af46651a..22a73d06 100644 --- a/btcstaking-tracker/atomicslasher/routines.go +++ b/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 diff --git a/btcstaking-tracker/btcslasher/slasher_utils.go b/btcstaking-tracker/btcslasher/slasher_utils.go index db6a98b5..d3734c06 100644 --- a/btcstaking-tracker/btcslasher/slasher_utils.go +++ b/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, diff --git a/btcstaking-tracker/stakingeventwatcher/stakingeventwatcher.go b/btcstaking-tracker/stakingeventwatcher/stakingeventwatcher.go index c241da20..6eb90dee 100644 --- a/btcstaking-tracker/stakingeventwatcher/stakingeventwatcher.go +++ b/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 { diff --git a/config/monitor.go b/config/monitor.go index aace1b77..4332fa76 100644 --- a/config/monitor.go +++ b/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 diff --git a/config/reporter.go b/config/reporter.go index 292bf219..a883a116 100644 --- a/config/reporter.go +++ b/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 } diff --git a/config/submitter.go b/config/submitter.go index 6a7b6d83..6a4986b3 100644 --- a/config/submitter.go +++ b/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 } diff --git a/metrics/prometheus.go b/metrics/prometheus.go index 4033339c..e623cc91 100644 --- a/metrics/prometheus.go +++ b/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) + } } diff --git a/monitor/btcscanner/block_handler.go b/monitor/btcscanner/block_handler.go index 4f0c569c..e9ccea42 100644 --- a/monitor/btcscanner/block_handler.go +++ b/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 } diff --git a/monitor/btcscanner/btc_scanner.go b/monitor/btcscanner/btc_scanner.go index 40096ebd..b58bd870 100644 --- a/monitor/btcscanner/btc_scanner.go +++ b/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 } diff --git a/monitor/btcscanner/btc_scanner_test.go b/monitor/btcscanner/btc_scanner_test.go index fb5c0653..800d2c98 100644 --- a/monitor/btcscanner/btc_scanner_test.go +++ b/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) diff --git a/monitor/liveness_checker.go b/monitor/liveness_checker.go index 658aad86..1f0debb3 100644 --- a/monitor/liveness_checker.go +++ b/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) } diff --git a/monitor/monitor.go b/monitor/monitor.go index d9f8656c..a913396c 100644 --- a/monitor/monitor.go +++ b/monitor/monitor.go @@ -3,6 +3,7 @@ package monitor import ( "encoding/hex" "fmt" + "math" "sort" "sync" @@ -143,6 +144,9 @@ func (m *Monitor) Start(baseHeight uint32) { } else if !exists { startHeight = baseHeight } else { + if dbHeight > math.MaxUint32 { + panic(fmt.Errorf("dbHeight %d exceeds uint32 range", dbHeight)) + } startHeight = uint32(dbHeight) + 1 } diff --git a/reporter/block_handler.go b/reporter/block_handler.go index f14dea32..6c6ddd5c 100644 --- a/reporter/block_handler.go +++ b/reporter/block_handler.go @@ -22,7 +22,11 @@ func (r *Reporter) blockEventHandler(blockNotifier *chainntnfs.BlockEpochEvent) return // channel closed } - if err := r.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 := r.handleNewBlock(uint32(epoch.Height), epoch.BlockHeader); err != nil { r.logger.Warnf("Due to error in event processing: %v, bootstrap process need to be restarted", err) r.bootstrapWithRetries() } @@ -34,7 +38,7 @@ func (r *Reporter) blockEventHandler(blockNotifier *chainntnfs.BlockEpochEvent) } // handleNewBlock processes a new block, checking if it connects to the cache or requires bootstrapping. -func (r *Reporter) handleNewBlock(height int32, header *wire.BlockHeader) error { +func (r *Reporter) handleNewBlock(height uint32, header *wire.BlockHeader) error { cacheTip := r.btcCache.Tip() if cacheTip == nil { return fmt.Errorf("cache is empty, restart bootstrap process") diff --git a/reporter/bootstrapping.go b/reporter/bootstrapping.go index 58bed291..f2bfc0c5 100644 --- a/reporter/bootstrapping.go +++ b/reporter/bootstrapping.go @@ -106,7 +106,7 @@ func (r *Reporter) bootstrap() error { // trim cache to the latest k+w blocks on BTC (which are same as in BBN) maxEntries := r.btcConfirmationDepth + r.checkpointFinalizationTimeout - if err = r.btcCache.Resize(uint64(maxEntries)); err != nil { + if err = r.btcCache.Resize(maxEntries); err != nil { r.logger.Errorf("Failed to resize BTC cache: %v", err) panic(err) } diff --git a/reporter/utils.go b/reporter/utils.go index ba489424..73373f38 100644 --- a/reporter/utils.go +++ b/reporter/utils.go @@ -200,7 +200,7 @@ func (r *Reporter) matchAndSubmitCheckpoints(signer string) (int, error) { tx1Block := ckpt.Segments[0].AssocBlock tx2Block := ckpt.Segments[1].AssocBlock r.metrics.NewReportedCheckpointGaugeVec.WithLabelValues( - strconv.Itoa(int(ckpt.Epoch)), + strconv.FormatUint(ckpt.Epoch, 10), strconv.Itoa(int(tx1Block.Height)), tx1Block.Txs[ckpt.Segments[0].TxIdx].Hash().String(), tx2Block.Txs[ckpt.Segments[1].TxIdx].Hash().String(), diff --git a/reporter/utils_test.go b/reporter/utils_test.go index ec114121..5f4b060a 100644 --- a/reporter/utils_test.go +++ b/reporter/utils_test.go @@ -67,7 +67,7 @@ func FuzzProcessHeaders(f *testing.F) { blocks, _, _ := vdatagen.GenRandomBlockchainWithBabylonTx(r, numBlocks, 0, 0) ibs := []*types.IndexedBlock{} for _, block := range blocks { - ibs = append(ibs, types.NewIndexedBlockFromMsgBlock(r.Int31(), block)) + ibs = append(ibs, types.NewIndexedBlockFromMsgBlock(r.Uint32(), block)) } _, mockBabylonClient, mockReporter := newMockReporter(t, ctrl) @@ -111,7 +111,7 @@ func FuzzProcessCheckpoints(f *testing.F) { ibs := []*types.IndexedBlock{} numMatchedCkptsExpected := 0 for i, block := range blocks { - ibs = append(ibs, types.NewIndexedBlockFromMsgBlock(r.Int31(), block)) + ibs = append(ibs, types.NewIndexedBlockFromMsgBlock(r.Uint32(), block)) if rawCkpts[i] != nil { numMatchedCkptsExpected++ } diff --git a/rpcserver/tls.go b/rpcserver/tls.go index c7250a6b..6e9c94f2 100644 --- a/rpcserver/tls.go +++ b/rpcserver/tls.go @@ -73,7 +73,7 @@ func generateRPCKeyPair(RPCKeyFile string, RPCCertFile string, writeKey bool) (t } err = os.WriteFile(RPCKeyFile, key, 0600) if err != nil { - os.Remove(RPCCertFile) //nolint: errcheck + _ = os.Remove(RPCCertFile) //nolint: errcheck return tls.Certificate{}, err } } diff --git a/submitter/relayer/change_address.go b/submitter/relayer/change_address.go index 512ead53..1123a4db 100644 --- a/submitter/relayer/change_address.go +++ b/submitter/relayer/change_address.go @@ -3,7 +3,7 @@ package relayer import ( "errors" "github.com/btcsuite/btcd/btcutil" - "math/rand" + "math/rand/v2" ) // GetChangeAddress randomly picks one address from local addresses that have received funds @@ -35,8 +35,8 @@ func (rl *Relayer) GetChangeAddress() (btcutil.Address, error) { } if len(segwitBech32Addrs) != 0 { - return segwitBech32Addrs[rand.Intn(len(segwitBech32Addrs))], nil + return segwitBech32Addrs[(len(segwitBech32Addrs))-1], nil } - - return legacyAddrs[rand.Intn(len(legacyAddrs))], nil + // #nosec G404 -- using math/rand/v2 + return legacyAddrs[rand.IntN(len(legacyAddrs))], nil } diff --git a/submitter/relayer/relayer.go b/submitter/relayer/relayer.go index d6301eb5..fc03ebb4 100644 --- a/submitter/relayer/relayer.go +++ b/submitter/relayer/relayer.go @@ -203,7 +203,7 @@ func (rl *Relayer) MaybeResubmitSecondCheckpointTx(ckpt *ckpttypes.RawCheckpoint // record the metrics of the resent tx2 rl.metrics.NewSubmittedCheckpointSegmentGaugeVec.WithLabelValues( - strconv.Itoa(int(ckptEpoch)), + strconv.FormatUint(ckptEpoch, 10), "1", resubmittedTx2.TxId.String(), strconv.Itoa(int(resubmittedTx2.Fee)), @@ -374,7 +374,7 @@ func (rl *Relayer) logAndRecordCheckpointMetrics(tx1, tx2 *types.BtcTxInfo, epoc // Record metrics for the first transaction rl.metrics.NewSubmittedCheckpointSegmentGaugeVec.WithLabelValues( - strconv.Itoa(int(epochNum)), + strconv.FormatUint(epochNum, 10), "0", tx1.Tx.TxHash().String(), strconv.Itoa(int(tx1.Fee)), @@ -382,7 +382,7 @@ func (rl *Relayer) logAndRecordCheckpointMetrics(tx1, tx2 *types.BtcTxInfo, epoc // Record metrics for the second transaction rl.metrics.NewSubmittedCheckpointSegmentGaugeVec.WithLabelValues( - strconv.Itoa(int(epochNum)), + strconv.FormatUint(epochNum, 10), "1", tx2.Tx.TxHash().String(), strconv.Itoa(int(tx2.Fee)), @@ -612,7 +612,13 @@ func (rl *Relayer) buildTxWithData(data []byte, firstTx *wire.MsgTx) (*types.Btc // getFeeRate returns the estimated fee rate, ensuring it within [tx-fee-max, tx-fee-min] func (rl *Relayer) getFeeRate() chainfee.SatPerKVByte { - fee, err := rl.EstimateFeePerKW(uint32(rl.GetBTCConfig().TargetBlockNum)) + targetBlockNum := rl.GetBTCConfig().TargetBlockNum + + // check we are within the uint32 range + if targetBlockNum < 0 || targetBlockNum > int64(^uint32(0)) { + panic(fmt.Errorf("targetBlockNum (%d) is out of uint32 range", targetBlockNum)) //software bug, panic + } + fee, err := rl.EstimateFeePerKW(uint32(targetBlockNum)) if err != nil { defaultFee := rl.GetBTCConfig().DefaultFee rl.logger.Errorf("failed to estimate transaction fee. Using default fee %v: %s", defaultFee, err.Error()) diff --git a/testutil/datagen/reporter.go b/testutil/datagen/reporter.go index 54c244a7..e8ab9b56 100644 --- a/testutil/datagen/reporter.go +++ b/testutil/datagen/reporter.go @@ -199,7 +199,7 @@ func GetRandomIndexedBlocks(r *rand.Rand, numBlocks uint64) []*types.IndexedBloc block, _ := GenRandomBlock(r, 1, nil) prevHeight := r.Int31n(math.MaxInt32 - int32(numBlocks)) - ib := types.NewIndexedBlockFromMsgBlock(prevHeight, block) + ib := types.NewIndexedBlockFromMsgBlock(uint32(prevHeight), block) prevHash := ib.Header.BlockHash() ibs = GetRandomIndexedBlocksFromHeight(r, numBlocks-1, prevHeight, prevHash) @@ -212,7 +212,7 @@ func GetRandomIndexedBlocksFromHeight(r *rand.Rand, numBlocks uint64, rootHeight var ( ibs []*types.IndexedBlock prevHash = rootHash - prevHeight = rootHeight + prevHeight = uint32(rootHeight) ) for i := 0; i < int(numBlocks); i++ { diff --git a/types/btccache.go b/types/btccache.go index 9515016d..973c7930 100644 --- a/types/btccache.go +++ b/types/btccache.go @@ -8,12 +8,12 @@ import ( type BTCCache struct { blocks []*IndexedBlock - maxEntries uint64 + maxEntries uint32 sync.RWMutex } -func NewBTCCache(maxEntries uint64) (*BTCCache, error) { +func NewBTCCache(maxEntries uint32) (*BTCCache, error) { // if maxEntries is 0, it means that the cache is disabled if maxEntries == 0 { return nil, ErrInvalidMaxEntries @@ -58,10 +58,10 @@ func (b *BTCCache) Add(ib *IndexedBlock) { // Thread-unsafe version of Add func (b *BTCCache) add(ib *IndexedBlock) { - if b.size() > b.maxEntries { + if b.size() > int(b.maxEntries) { panic(ErrTooManyEntries) } - if b.size() == b.maxEntries { + if b.size() == int(b.maxEntries) { // dereference the 0-th block to ensure it will be garbage-collected // see https://stackoverflow.com/questions/55045402/memory-leak-in-golang-slice b.blocks[0] = nil @@ -117,7 +117,7 @@ func (b *BTCCache) RemoveAll() { } // Size returns the size of the cache. Thread-safe. -func (b *BTCCache) Size() uint64 { +func (b *BTCCache) Size() int { b.RLock() defer b.RUnlock() @@ -125,24 +125,24 @@ func (b *BTCCache) Size() uint64 { } // thread-unsafe version of Size -func (b *BTCCache) size() uint64 { - return uint64(len(b.blocks)) +func (b *BTCCache) size() int { + return len(b.blocks) } -// GetLastBlocks returns list of blocks between the given stopHeight and the tip of the chain in cache +// GetLastBlocks return list of blocks between the given stopHeight and the tip of the chain in cache func (b *BTCCache) GetLastBlocks(stopHeight uint32) ([]*IndexedBlock, error) { b.RLock() defer b.RUnlock() firstHeight := b.blocks[0].Height lastHeight := b.blocks[len(b.blocks)-1].Height - if int32(stopHeight) < firstHeight || lastHeight < int32(stopHeight) { + if (stopHeight) < firstHeight || lastHeight < (stopHeight) { return []*IndexedBlock{}, fmt.Errorf("the given stopHeight %d is out of range [%d, %d] of BTC cache", stopHeight, firstHeight, lastHeight) } var j int for i := len(b.blocks) - 1; i >= 0; i-- { - if b.blocks[i].Height == int32(stopHeight) { + if b.blocks[i].Height == (stopHeight) { j = i break } @@ -184,21 +184,21 @@ func (b *BTCCache) FindBlock(blockHeight uint32) *IndexedBlock { firstHeight := b.blocks[0].Height lastHeight := b.blocks[len(b.blocks)-1].Height - if int32(blockHeight) < firstHeight || lastHeight < int32(blockHeight) { + if (blockHeight) < firstHeight || lastHeight < (blockHeight) { return nil } - leftBound := uint64(0) + leftBound := 0 rightBound := b.size() - 1 for leftBound <= rightBound { midPoint := leftBound + (rightBound-leftBound)/2 - if b.blocks[midPoint].Height == int32(blockHeight) { + if b.blocks[midPoint].Height == (blockHeight) { return b.blocks[midPoint] } - if b.blocks[midPoint].Height > int32(blockHeight) { + if b.blocks[midPoint].Height > (blockHeight) { rightBound = midPoint - 1 } else { leftBound = midPoint + 1 @@ -208,7 +208,7 @@ func (b *BTCCache) FindBlock(blockHeight uint32) *IndexedBlock { return nil } -func (b *BTCCache) Resize(maxEntries uint64) error { +func (b *BTCCache) Resize(maxEntries uint32) error { b.Lock() defer b.Unlock() @@ -225,7 +225,7 @@ func (b *BTCCache) Trim() { defer b.Unlock() // cache size is smaller than maxEntries, can't trim - if b.size() < b.maxEntries { + if b.size() < int(b.maxEntries) { return } diff --git a/types/btccache_test.go b/types/btccache_test.go index 08f265e8..c218e068 100644 --- a/types/btccache_test.go +++ b/types/btccache_test.go @@ -33,7 +33,7 @@ func FuzzBtcCache(f *testing.F) { invalidMaxEntries = true } - cache, err := types.NewBTCCache(maxEntries) + cache, err := types.NewBTCCache(uint32(maxEntries)) if err != nil { if !invalidMaxEntries { t.Fatalf("NewBTCCache returned error %s", err) @@ -64,7 +64,7 @@ func FuzzBtcCache(f *testing.F) { t.Skip("Skipping test with invalid numBlocks") } - require.Equal(t, numBlocks, cache.Size()) + require.Equal(t, numBlocks, uint64(cache.Size())) // Find a random block in the cache randIdx := datagen.RandomInt(r, int(numBlocks)) @@ -76,19 +76,19 @@ func FuzzBtcCache(f *testing.F) { // Add random blocks to the cache addCount := datagen.RandomIntOtherThan(r, 0, 1000) - prevCacheHeight := cache.Tip().Height + prevCacheHeight := int32(cache.Tip().Height) cacheBlocksBeforeAddition := cache.GetAllBlocks() - blocksToAdd := vdatagen.GetRandomIndexedBlocksFromHeight(r, addCount, cache.Tip().Height, cache.Tip().BlockHash()) + blocksToAdd := vdatagen.GetRandomIndexedBlocksFromHeight(r, addCount, int32(cache.Tip().Height), cache.Tip().BlockHash()) for _, ib := range blocksToAdd { cache.Add(ib) } - require.Equal(t, prevCacheHeight+int32(addCount), cache.Tip().Height) + require.Equal(t, prevCacheHeight+int32(addCount), int32(cache.Tip().Height)) require.Equal(t, blocksToAdd[addCount-1], cache.Tip()) // ensure block heights in cache are in increasing order var heights []int32 for _, ib := range cache.GetAllBlocks() { - heights = append(heights, ib.Height) + heights = append(heights, int32(ib.Height)) } require.IsIncreasing(t, heights) @@ -118,7 +118,7 @@ func FuzzBtcCache(f *testing.F) { } // Remove random number of blocks from the cache - prevSize := cache.Size() + prevSize := uint64(cache.Size()) deleteCount := datagen.RandomInt(r, int(prevSize)) cacheBlocksBeforeDeletion := cache.GetAllBlocks() for i := 0; i < int(deleteCount); i++ { @@ -126,7 +126,7 @@ func FuzzBtcCache(f *testing.F) { require.NoError(t, err) } cacheBlocksAfterDeletion := cache.GetAllBlocks() - require.Equal(t, prevSize-deleteCount, cache.Size()) + require.Equal(t, prevSize-deleteCount, uint64(cache.Size())) require.Equal(t, cacheBlocksBeforeDeletion[:len(cacheBlocksBeforeDeletion)-int(deleteCount)], cacheBlocksAfterDeletion) }) } diff --git a/types/genesis_info.go b/types/genesis_info.go index 13922b92..b7d3bed2 100644 --- a/types/genesis_info.go +++ b/types/genesis_info.go @@ -79,10 +79,14 @@ func GetGenesisInfoFromFile(filePath string) (*GenesisInfo, error) { msgCreateValidator := msgs[0].(*stakingtypes.MsgCreateValidator) if gk.ValidatorAddress == msgCreateValidator.ValidatorAddress { + power := sdk.TokensToConsensusPower(msgCreateValidator.Value.Amount, sdk.DefaultPowerReduction) + if power < 0 { + return nil, fmt.Errorf("consensus power calculation returned a negative value") + } keyWithPower := &checkpointingtypes.ValidatorWithBlsKey{ ValidatorAddress: msgCreateValidator.ValidatorAddress, BlsPubKey: *gk.BlsKey.Pubkey, - VotingPower: uint64(sdk.TokensToConsensusPower(msgCreateValidator.Value.Amount, sdk.DefaultPowerReduction)), + VotingPower: uint64(power), } valSet.ValSet = append(valSet.ValSet, keyWithPower) } diff --git a/types/indexed_block.go b/types/indexed_block.go index 7e5ee900..ec28bc17 100644 --- a/types/indexed_block.go +++ b/types/indexed_block.go @@ -16,16 +16,16 @@ import ( // - txHash, txHashWitness, txIndex for each Tx // These are necessary for generating Merkle proof (and thus the `MsgInsertBTCSpvProof` message in babylon) of a certain tx type IndexedBlock struct { - Height int32 + Height uint32 Header *wire.BlockHeader Txs []*btcutil.Tx } -func NewIndexedBlock(height int32, header *wire.BlockHeader, txs []*btcutil.Tx) *IndexedBlock { +func NewIndexedBlock(height uint32, header *wire.BlockHeader, txs []*btcutil.Tx) *IndexedBlock { return &IndexedBlock{height, header, txs} } -func NewIndexedBlockFromMsgBlock(height int32, block *wire.MsgBlock) *IndexedBlock { +func NewIndexedBlockFromMsgBlock(height uint32, block *wire.MsgBlock) *IndexedBlock { return &IndexedBlock{ height, &block.Header, @@ -34,7 +34,7 @@ func NewIndexedBlockFromMsgBlock(height int32, block *wire.MsgBlock) *IndexedBlo } func (ib *IndexedBlock) MsgBlock() *wire.MsgBlock { - msgTxs := []*wire.MsgTx{} + msgTxs := make([]*wire.MsgTx, 0, len(ib.Txs)) for _, tx := range ib.Txs { msgTxs = append(msgTxs, tx.MsgTx()) } diff --git a/types/indexed_block_test.go b/types/indexed_block_test.go index ae7a868e..6c7e0524 100644 --- a/types/indexed_block_test.go +++ b/types/indexed_block_test.go @@ -26,7 +26,7 @@ func FuzzIndexedBlock(f *testing.F) { blocks, _, rawCkpts := vdatagen.GenRandomBlockchainWithBabylonTx(r, 100, 0, 0.4) for i, block := range blocks { - ib := types.NewIndexedBlockFromMsgBlock(int32(i), block) + ib := types.NewIndexedBlockFromMsgBlock(uint32(i), block) if rawCkpts[i] != nil { // Babylon tx spvProof, err := ib.GenSPVProof(1)