From c299aa483a4d258e173426c4bc845e2a5d764d5a Mon Sep 17 00:00:00 2001 From: aalu1418 <50029043+aalu1418@users.noreply.github.com> Date: Thu, 26 Sep 2024 16:47:58 -0600 Subject: [PATCH] fix gosec g115 overflow errors or add explanations to ignore --- .golangci.yml | 6 +++++- pkg/monitoring/source_envelope.go | 6 +++--- pkg/solana/chain.go | 2 +- pkg/solana/client/multinode/multi_node.go | 4 ++-- pkg/solana/config/config.go | 2 +- pkg/solana/config/toml.go | 2 +- pkg/solana/config_digester.go | 8 ++++---- pkg/solana/fees/computebudget.go | 10 +++++----- pkg/solana/report.go | 2 +- pkg/solana/txm/txm.go | 18 +++++++++--------- 10 files changed, 32 insertions(+), 28 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 0e66a8650..d0542f400 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -149,4 +149,8 @@ issues: - path: test text: "^G404:" linters: - - gosec \ No newline at end of file + - gosec + - path: _test.go + text: "G115:" # ignore integer overflow in test conversions + linters: + - gosec diff --git a/pkg/monitoring/source_envelope.go b/pkg/monitoring/source_envelope.go index 8b0850ed6..7a475bed7 100644 --- a/pkg/monitoring/source_envelope.go +++ b/pkg/monitoring/source_envelope.go @@ -237,9 +237,9 @@ func getLinkAvailableForPayment(state pkgSolana.State, linkBalance *big.Int) (*b } var countUnpaidRounds, reimbursements uint64 = 0, 0 for _, oracle := range oracles { - numRounds := int(state.Config.LatestAggregatorRoundID) - int(oracle.FromRoundID) - if numRounds < 0 { - numRounds = 0 + numRounds := uint32(0) // prevent overflow if RoundID is larger than latest aggregator RoundID + if state.Config.LatestAggregatorRoundID >= oracle.FromRoundID { + numRounds = state.Config.LatestAggregatorRoundID - oracle.FromRoundID } countUnpaidRounds += uint64(numRounds) reimbursements += oracle.Payment diff --git a/pkg/solana/chain.go b/pkg/solana/chain.go index 8b4ad5787..5749e3f34 100644 --- a/pkg/solana/chain.go +++ b/pkg/solana/chain.go @@ -327,7 +327,7 @@ func (c *chain) LatestHead(_ context.Context) (types.Head, error) { return types.Head{ Height: strconv.FormatUint(*latestBlock.BlockHeight, 10), Hash: hashBytes, - Timestamp: uint64(latestBlock.BlockTime.Time().Unix()), + Timestamp: uint64(latestBlock.BlockTime.Time().Unix()), //nolint:gosec // blocktime will never be negative (pre 1970) }, nil } diff --git a/pkg/solana/client/multinode/multi_node.go b/pkg/solana/client/multinode/multi_node.go index 1a4846edf..8b7efc46b 100644 --- a/pkg/solana/client/multinode/multi_node.go +++ b/pkg/solana/client/multinode/multi_node.go @@ -90,10 +90,10 @@ func (c *MultiNode[CHAIN_ID, RPC]) ChainID() CHAIN_ID { return c.chainID } -func (c *MultiNode[CHAIN_ID, RPC]) DoAll(ctx context.Context, do func(ctx context.Context, rpc RPC, isSendOnly bool)) error { +func (c *MultiNode[CHAIN_ID, RPC]) DoAll(baseCtx context.Context, do func(ctx context.Context, rpc RPC, isSendOnly bool)) error { var err error ok := c.IfNotStopped(func() { - ctx, _ = c.chStop.Ctx(ctx) + ctx, _ := c.chStop.Ctx(baseCtx) callsCompleted := 0 for _, n := range c.primaryNodes { diff --git a/pkg/solana/config/config.go b/pkg/solana/config/config.go index 28698c7c3..cabfe3365 100644 --- a/pkg/solana/config/config.go +++ b/pkg/solana/config/config.go @@ -122,7 +122,7 @@ func (c *Chain) SetDefaults() { c.Commitment = (*string)(&defaultConfigSet.Commitment) } if c.MaxRetries == nil && defaultConfigSet.MaxRetries != nil { - i := int64(*defaultConfigSet.MaxRetries) + i := int64(*defaultConfigSet.MaxRetries) //nolint:gosec // reasonable default value does not cause overflow c.MaxRetries = &i } if c.FeeEstimatorMode == nil { diff --git a/pkg/solana/config/toml.go b/pkg/solana/config/toml.go index 90657fd2c..fcceb0603 100644 --- a/pkg/solana/config/toml.go +++ b/pkg/solana/config/toml.go @@ -249,7 +249,7 @@ func (c *TOMLConfig) MaxRetries() *uint { if *c.Chain.MaxRetries < 0 { return nil // interpret negative numbers as nil (prevents unlikely case of overflow) } - mr := uint(*c.Chain.MaxRetries) + mr := uint(*c.Chain.MaxRetries) //nolint:gosec // overflow check is handled above return &mr } diff --git a/pkg/solana/config_digester.go b/pkg/solana/config_digester.go index 17c70e04a..289a20416 100644 --- a/pkg/solana/config_digester.go +++ b/pkg/solana/config_digester.go @@ -32,11 +32,11 @@ func (d OffchainConfigDigester) ConfigDigest(cfg types.ContractConfig) (types.Co return digest, err } - if err := binary.Write(buf, binary.BigEndian, uint32(cfg.ConfigCount)); err != nil { + if err := binary.Write(buf, binary.BigEndian, uint32(cfg.ConfigCount)); err != nil { //nolint:gosec // max onchain config count is u32 return digest, err } - if err := binary.Write(buf, binary.BigEndian, uint8(len(cfg.Signers))); err != nil { + if err := binary.Write(buf, binary.BigEndian, uint8(len(cfg.Signers))); err != nil { //nolint:gosec // cannot be negative and protocol does not allow more than 255 signers return digest, err } @@ -60,7 +60,7 @@ func (d OffchainConfigDigester) ConfigDigest(cfg types.ContractConfig) (types.Co return digest, err } - if err := binary.Write(buf, binary.BigEndian, uint32(len(cfg.OnchainConfig))); err != nil { + if err := binary.Write(buf, binary.BigEndian, uint32(len(cfg.OnchainConfig))); err != nil { //nolint:gosec // cannot be negative and omax u32 exceeds max onchain config length return digest, err } @@ -72,7 +72,7 @@ func (d OffchainConfigDigester) ConfigDigest(cfg types.ContractConfig) (types.Co return digest, err } - if err := binary.Write(buf, binary.BigEndian, uint32(len(cfg.OffchainConfig))); err != nil { + if err := binary.Write(buf, binary.BigEndian, uint32(len(cfg.OffchainConfig))); err != nil { //nolint:gosec // cannot be negative and max u32 exceeds max offchain config length return digest, err } diff --git a/pkg/solana/fees/computebudget.go b/pkg/solana/fees/computebudget.go index fec4a4442..6738d4fb4 100644 --- a/pkg/solana/fees/computebudget.go +++ b/pkg/solana/fees/computebudget.go @@ -82,18 +82,18 @@ func SetComputeUnitPrice(tx *solana.Transaction, price ComputeUnitPrice) error { // find ComputeBudget program to accounts if it exists // reimplements HasAccount to retrieve index: https://github.com/gagliardetto/solana-go/blob/618f56666078f8131a384ab27afd918d248c08b7/message.go#L233 var exists bool - var programIdx uint16 + var programIdx int for i, a := range tx.Message.AccountKeys { if a.Equals(price.ProgramID()) { exists = true - programIdx = uint16(i) + programIdx = i break } } // if it doesn't exist, add to account keys if !exists { tx.Message.AccountKeys = append(tx.Message.AccountKeys, price.ProgramID()) - programIdx = uint16(len(tx.Message.AccountKeys) - 1) // last index of account keys + programIdx = len(tx.Message.AccountKeys) - 1 // last index of account keys // https://github.com/gagliardetto/solana-go/blob/618f56666078f8131a384ab27afd918d248c08b7/transaction.go#L293 tx.Message.Header.NumReadonlyUnsignedAccounts++ @@ -107,7 +107,7 @@ func SetComputeUnitPrice(tx *solana.Transaction, price ComputeUnitPrice) error { // compiled instruction instruction := solana.CompiledInstruction{ - ProgramIDIndex: programIdx, + ProgramIDIndex: uint16(programIdx), //nolint:gosec // max value would exceed tx size Data: data, } @@ -115,7 +115,7 @@ func SetComputeUnitPrice(tx *solana.Transaction, price ComputeUnitPrice) error { var found bool var instructionIdx int for i := range tx.Message.Instructions { - if tx.Message.Instructions[i].ProgramIDIndex == programIdx && + if int(tx.Message.Instructions[i].ProgramIDIndex) == programIdx && len(tx.Message.Instructions[i].Data) > 0 && tx.Message.Instructions[i].Data[0] == InstructionSetComputeUnitPrice { found = true diff --git a/pkg/solana/report.go b/pkg/solana/report.go index 572ac6023..35d384de7 100644 --- a/pkg/solana/report.go +++ b/pkg/solana/report.go @@ -56,7 +56,7 @@ func (c ReportCodec) BuildReport(oo []median.ParsedAttributedObservation) (types binary.BigEndian.PutUint32(time, timestamp) report = append(report, time[:]...) - observersCount := uint8(n) + observersCount := uint8(n) //nolint:gosec // count can never be 0, and oracle network will never be larger than 255 report = append(report, observersCount) report = append(report, observers[:]...) diff --git a/pkg/solana/txm/txm.go b/pkg/solana/txm/txm.go index a5700bd8b..dd4a31c9b 100644 --- a/pkg/solana/txm/txm.go +++ b/pkg/solana/txm/txm.go @@ -151,17 +151,17 @@ func (txm *Txm) sendWithRetry(chanCtx context.Context, baseTx solanaGo.Transacti // only calculate base price once // prevent underlying base changing when bumping (could occur with RPC based estimation) basePrice := txm.fee.BaseComputeUnitPrice() - getFee := func(count uint) fees.ComputeUnitPrice { + getFee := func(count int) fees.ComputeUnitPrice { fee := fees.CalculateFee( basePrice, txm.cfg.ComputeUnitPriceMax(), txm.cfg.ComputeUnitPriceMin(), - count, + uint(count), //nolint:gosec // reasonable number of bumps should never cause overflow ) return fees.ComputeUnitPrice(fee) } - buildTx := func(base solanaGo.Transaction, retryCount uint) (solanaGo.Transaction, error) { + buildTx := func(base solanaGo.Transaction, retryCount int) (solanaGo.Transaction, error) { newTx := base // make copy // set fee @@ -224,7 +224,7 @@ func (txm *Txm) sendWithRetry(chanCtx context.Context, baseTx solanaGo.Transacti go func(baseTx, currentTx solanaGo.Transaction) { deltaT := 1 // ms tick := time.After(0) - bumpCount := uint(0) + bumpCount := 0 bumpTime := time.Now() var wg sync.WaitGroup @@ -253,7 +253,7 @@ func (txm *Txm) sendWithRetry(chanCtx context.Context, baseTx solanaGo.Transacti return // exit func if cannot build tx for retrying } ind := sigs.Allocate() - if uint(ind) != bumpCount { + if ind != bumpCount { txm.lggr.Errorw("INVARIANT VIOLATION: index (%d) != bumpCount (%d)", ind, bumpCount) return } @@ -261,7 +261,7 @@ func (txm *Txm) sendWithRetry(chanCtx context.Context, baseTx solanaGo.Transacti // take currentTx and broadcast, if bumped fee -> save signature to list wg.Add(1) - go func(bump bool, count uint, retryTx solanaGo.Transaction) { + go func(bump bool, count int, retryTx solanaGo.Transaction) { defer wg.Done() retrySig, retrySendErr := client.SendTx(ctx, &retryTx) @@ -281,7 +281,7 @@ func (txm *Txm) sendWithRetry(chanCtx context.Context, baseTx solanaGo.Transacti txm.lggr.Warnw("error in adding retry transaction", "error", retryStoreErr, "id", id) return } - if setErr := sigs.Set(int(count), retrySig); setErr != nil { + if setErr := sigs.Set(count, retrySig); setErr != nil { // this should never happen txm.lggr.Errorw("INVARIANT VIOLATION", "error", setErr) } @@ -292,7 +292,7 @@ func (txm *Txm) sendWithRetry(chanCtx context.Context, baseTx solanaGo.Transacti wait := make(chan struct{}) go func() { defer close(wait) - sigs.Wait(int(count)) // wait until bump tx has set the tx signature to compare rebroadcast signatures + sigs.Wait(count) // wait until bump tx has set the tx signature to compare rebroadcast signatures }() select { case <-ctx.Done(): @@ -301,7 +301,7 @@ func (txm *Txm) sendWithRetry(chanCtx context.Context, baseTx solanaGo.Transacti } // this should never happen (should match the signature saved to sigs) - if fetchedSig, fetchErr := sigs.Get(int(count)); fetchErr != nil || retrySig != fetchedSig { + if fetchedSig, fetchErr := sigs.Get(count); fetchErr != nil || retrySig != fetchedSig { txm.lggr.Errorw("original signature does not match retry signature", "expectedSignatures", sigs.List(), "receivedSignature", retrySig, "error", fetchErr) } }(shouldBump, bumpCount, currentTx)