diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 627a2caaf6e..ee4d306aa2b 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -307,7 +307,7 @@ jobs: - name: Formatting checks run: ./scripts/lint.sh -l -g format - name: Install linters - run: curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v2.8.0 + run: curl -sSfL https://golangci-lint.run/install.sh | sh -s -- -b $(go env GOPATH)/bin v2.12.2 - name: Run linters run: make generate && golangci-lint --version && ./scripts/lint.sh -g lint - name: Ensure generated proto matches diff --git a/node/cmd/ccq/permissions.go b/node/cmd/ccq/permissions.go index 93f08c4d0ca..caaaae9f86a 100644 --- a/node/cmd/ccq/permissions.go +++ b/node/cmd/ccq/permissions.go @@ -31,7 +31,7 @@ type ( User struct { UserName string `json:"userName"` - APIKey string `json:"apiKey"` + APIKey string `json:"apiKey"` // #nosec G117 -- This is a config field name, not a hardcoded secret AllowUnsigned bool `json:"allowUnsigned"` AllowAnything bool `json:"allowAnything"` RateLimit *float64 `json:"RateLimit"` diff --git a/node/cmd/guardiand/adminclient.go b/node/cmd/guardiand/adminclient.go index 190616f30b4..676260f11fc 100644 --- a/node/cmd/guardiand/adminclient.go +++ b/node/cmd/guardiand/adminclient.go @@ -423,7 +423,13 @@ func runInjectGovernanceVAA(cmd *cobra.Command, args []string) { } func runFindMissingMessages(cmd *cobra.Command, args []string) { - chainID, err := strconv.Atoi(args[0]) + // Effectively allow any uint16 value for ChainID here. A Guardian may want to find missing messages + // for deprecated chains, so we don't use [`vaa.StringToKnownChainID`] here. + // + // This value is later converted to uint32 for use in protobuf, but before that + // we should make sure it fits into uint16 to disallow numbers > max.Uint16 + // from being serialized. + chainId, err := strconv.ParseUint(args[0], 10, 16) if err != nil { log.Fatalf("invalid chain ID: %v", err) } @@ -439,12 +445,8 @@ func runFindMissingMessages(cmd *cobra.Command, args []string) { } defer conn.Close() - if chainID > math.MaxUint16 { - log.Fatalf("chain ID is not a valid 16 bit unsigned integer: %v", err) - } - msg := nodev1.FindMissingMessagesRequest{ - EmitterChain: uint32(chainID), // #nosec G115 -- This conversion is checked above + EmitterChain: uint32(chainId), EmitterAddress: emitterAddress, RpcBackfill: *shouldBackfill, BackfillNodes: sdk.PublicRPCEndpoints, @@ -470,13 +472,13 @@ func runDumpVAAByMessageID(cmd *cobra.Command, args []string) { if len(parts) != 3 { log.Fatalf("invalid message ID: %s", args[0]) } - chainID, err := strconv.ParseUint(parts[0], 10, 32) + // Parse string ChainID as uint16 to ensure it fits the vaa.ChainID type. + // There is no "known" check here as Guardians may want to dump messages + // even for deprecated chains. + chainId, err := strconv.ParseUint(parts[0], 10, 16) if err != nil { log.Fatalf("invalid chain ID: %v", err) } - if chainID > math.MaxUint16 { - log.Fatalf("chain id must not exceed the max uint16: %v", chainID) - } emitterAddress := parts[1] seq, err := strconv.ParseUint(parts[2], 10, 64) if err != nil { @@ -494,7 +496,7 @@ func runDumpVAAByMessageID(cmd *cobra.Command, args []string) { msg := publicrpcv1.GetSignedVAARequest{ MessageId: &publicrpcv1.MessageID{ - EmitterChain: publicrpcv1.ChainID(chainID), + EmitterChain: publicrpcv1.ChainID(chainId), EmitterAddress: emitterAddress, Sequence: seq, }, @@ -1083,15 +1085,11 @@ func runChainGovernorResetReleaseTimer(cmd *cobra.Command, args []string) { } func runPurgePythNetVaas(cmd *cobra.Command, args []string) { - daysOld, err := strconv.Atoi(args[0]) + daysOld, err := strconv.ParseUint(args[0], 10, 64) if err != nil { log.Fatalf("invalid DAYS_OLD: %v", err) } - if daysOld < 0 { - log.Fatalf("DAYS_OLD may not be negative") - } - logOnly := false if len(args) > 1 { if args[1] != "logonly" { @@ -1237,6 +1235,7 @@ func runSignExistingVaasFromCSV(cmd *cobra.Command, args []string) { } resp, err := c.SignExistingVAA(ctx, &msg) if err != nil { + // #nosec G706 -- row[0] is from admin CSV input, err is safe to log log.Printf("signing VAA (%s)[%d] failed - skipping: %v", row[0], i, err) continue } diff --git a/node/cmd/guardiand/adminnodes.go b/node/cmd/guardiand/adminnodes.go index b14d0c26c67..aaaeaeb6dab 100644 --- a/node/cmd/guardiand/adminnodes.go +++ b/node/cmd/guardiand/adminnodes.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "log" - "math" "os" "sort" "strings" @@ -179,15 +178,19 @@ func runListNodes(cmd *cobra.Command, args []string) { truncAddrs := make(map[vaa.ChainID]string) errors := map[vaa.ChainID]uint64{} for _, n := range h.RawHeartbeat.Networks { - if n.Id > math.MaxUint16 { - log.Fatalf("heartbeat chain id is greater than MaxUint16: %v", n.Id) + // Convert uint32 to ChainID. We deliberately don't use [vaa.KnownChainIDFromNumber] here + // in case the Guardians are running with different versions and so have a different set of + // chain IDs that are considered "known". + chainId, err := vaa.ChainIDFromNumber[uint32](n.Id) + if err != nil { + log.Fatalf("invalid chain id in heartbeat: %v", err) } - heights[vaa.ChainID(n.Id)] = n.Height - errors[vaa.ChainID(n.Id)] = n.ErrorCount + heights[chainId] = n.Height + errors[chainId] = n.ErrorCount if len(n.ContractAddress) >= 16 { - truncAddrs[vaa.ChainID(n.Id)] = n.ContractAddress[:16] + truncAddrs[chainId] = n.ContractAddress[:16] } else { - truncAddrs[vaa.ChainID(n.Id)] = "INVALID" + truncAddrs[chainId] = "INVALID" } } diff --git a/node/cmd/guardiand/node.go b/node/cmd/guardiand/node.go index 54161d2a548..6ded740aef1 100644 --- a/node/cmd/guardiand/node.go +++ b/node/cmd/guardiand/node.go @@ -794,8 +794,12 @@ func runNode(cmd *cobra.Command, args []string) { } // Node's main lifecycle context. - rootCtx, rootCtxCancel = context.WithCancel(context.Background()) - defer rootCtxCancel() + // Keep the cancel func in a local var so linters can see + // that the WithCancel return value is actually invoked. + var cancel context.CancelFunc + rootCtx, cancel = context.WithCancel(context.Background()) + rootCtxCancel = cancel + defer cancel() // Create the Guardian Signer guardianSigner, err := guardiansigner.NewGuardianSignerFromUri(rootCtx, *guardianSignerUri, env == common.UnsafeDevNet) diff --git a/node/cmd/spy/spy.go b/node/cmd/spy/spy.go index af496db16e4..9a68e0d1232 100644 --- a/node/cmd/spy/spy.go +++ b/node/cmd/spy/spy.go @@ -126,7 +126,7 @@ func (s *spyServer) PublishSignedVAA(vaaBytes []byte) error { return err } } - sub.ch <- message{vaaBytes: vaaBytes} //nolint:channelcheck // Don't want to drop incoming VAAs + sub.ch <- message{vaaBytes: vaaBytes} // Note on channel capacity: Don't want to drop incoming VAAs continue } @@ -146,7 +146,7 @@ func (s *spyServer) PublishSignedVAA(vaaBytes []byte) error { return err } } - sub.ch <- message{vaaBytes: vaaBytes} //nolint:channelcheck // Don't want to drop incoming VAAs + sub.ch <- message{vaaBytes: vaaBytes} // Note on channel capacity: Don't want to drop incoming VAAs } } @@ -252,7 +252,7 @@ func newSpyServer(logger *zap.Logger) *spyServer { func DoWithTimeout(f func() error, d time.Duration) error { errChan := make(chan error, 1) go func() { - errChan <- f() //nolint:channelcheck // Has timeout below + errChan <- f() // Note on channel capacity: Has timeout below close(errChan) }() t := time.NewTimer(d) @@ -341,8 +341,12 @@ func runSpy(cmd *cobra.Command, args []string) { } // Node's main lifecycle context. - rootCtx, rootCtxCancel = context.WithCancel(context.Background()) - defer rootCtxCancel() + // Keep the cancel func in a local var so linters can see + // that the WithCancel return value is actually invoked. + var cancel context.CancelFunc + rootCtx, cancel = context.WithCancel(context.Background()) + rootCtxCancel = cancel + defer cancel() // Inbound signed VAAs signedInC := make(chan *gossipv1.SignedVAAWithQuorum, 1024) diff --git a/node/cmd/txverifier/sui.go b/node/cmd/txverifier/sui.go index 7ee76940feb..7effbefb502 100644 --- a/node/cmd/txverifier/sui.go +++ b/node/cmd/txverifier/sui.go @@ -238,6 +238,7 @@ func pullDigestsFromWormholeScan(ctx context.Context, logger *zap.Logger) ([]str } req.Header.Set("Accept", "application/json") + // #nosec G704 -- Hardcoded WormholeScan API URL resp, err := http.DefaultClient.Do(req) if err != nil { diff --git a/node/hack/evm_test/wstest.go b/node/hack/evm_test/wstest.go index c6cb53360cc..2d94760d9e9 100644 --- a/node/hack/evm_test/wstest.go +++ b/node/hack/evm_test/wstest.go @@ -77,7 +77,7 @@ func main() { case <-ctx.Done(): return case err := <-headerSubscription.Err(): - errC <- fmt.Errorf("block subscription failed: %w", err) //nolint:channelcheck // The watcher will exit anyway + errC <- fmt.Errorf("block subscription failed: %w", err) // Note on channel capacity: The watcher will exit anyway return case block := <-headSink: // These two pointers should have been checked before the event was placed on the channel, but just being safe. @@ -114,7 +114,7 @@ func main() { case <-ctx.Done(): return case err := <-messageSub.Err(): - errC <- fmt.Errorf("message subscription failed: %w", err) //nolint:channelcheck // The watcher will exit anyway + errC <- fmt.Errorf("message subscription failed: %w", err) // Note on channel capacity: The watcher will exit anyway return case ev := <-messageC: logger.Info("Received a log event from the contract", zap.Any("ev", ev)) diff --git a/node/hack/release_verification/guardian_vaa_stats.go b/node/hack/release_verification/guardian_vaa_stats.go index c35c2efd88c..6a177bedd8d 100644 --- a/node/hack/release_verification/guardian_vaa_stats.go +++ b/node/hack/release_verification/guardian_vaa_stats.go @@ -39,6 +39,7 @@ func getValidatorIndexForChain(chainId vaa.ChainID, onlyafter time.Time) (map[ui } req.Header.Add("Accept", "*/*") + // #nosec G704 -- Maintenance script calling hardcoded WormholeScan API res, err := client.Do(req) if err != nil { return nil, err diff --git a/node/hack/repair_eth/repair_eth.go b/node/hack/repair_eth/repair_eth.go index b5c24bf6fc1..220b21b1bbf 100644 --- a/node/hack/repair_eth/repair_eth.go +++ b/node/hack/repair_eth/repair_eth.go @@ -140,6 +140,7 @@ func getCurrentHeight(chainId vaa.ChainID, ctx context.Context, c *http.Client, } req = addUserAgent(req) + // #nosec G704 -- Maintenance script calling hardcoded or config-provided explorer API resp, err := c.Do(req.WithContext(ctx)) if err != nil { return 0, fmt.Errorf("failed to get current height: %w", err) @@ -184,6 +185,7 @@ func getLogs(chainId vaa.ChainID, ctx context.Context, c *http.Client, api, key, } req = addUserAgent(req) + // #nosec G704 -- Maintenance script calling hardcoded or config-provided explorer API resp, err := c.Do(req) if err != nil { return nil, fmt.Errorf("failed to get logs: %w", err) @@ -487,6 +489,7 @@ func main() { panic(err) } req = addUserAgent(req) + // #nosec G704 -- Maintenance script calling hardcoded or config-provided explorer API resp, err := c.Do(req) if err != nil { log.Fatalf("verify: %v", err) @@ -494,6 +497,7 @@ func main() { defer resp.Body.Close() if resp.StatusCode != http.StatusOK { + // #nosec G706 -- Logging response status code from trusted API log.Printf("status %d, retrying", resp.StatusCode) time.Sleep(5 * time.Second) continue diff --git a/node/hack/repair_solana/repair.go b/node/hack/repair_solana/repair.go index 1ea0d9090e5..141b248b279 100644 --- a/node/hack/repair_solana/repair.go +++ b/node/hack/repair_solana/repair.go @@ -220,6 +220,7 @@ func main() { if err != nil { panic(err) } + // #nosec G704 -- Maintenance script calling public Wormhole RPC endpoint resp, err := hc.Do(req) if err != nil { log.Fatalf("verify: %v", err) @@ -228,6 +229,7 @@ func main() { defer resp.Body.Close() if resp.StatusCode != http.StatusOK { + // #nosec G706 -- Logging response status code from trusted API log.Printf("status %d, retrying", resp.StatusCode) time.Sleep(5 * time.Second) continue diff --git a/node/pkg/adminrpc/adminserver.go b/node/pkg/adminrpc/adminserver.go index e6b875081b6..99cca6f0662 100644 --- a/node/pkg/adminrpc/adminserver.go +++ b/node/pkg/adminrpc/adminserver.go @@ -1058,7 +1058,7 @@ func (s *nodePrivilegedService) InjectGovernanceVAA(ctx context.Context, req *no vaaInjectionsTotal.Inc() - s.injectC <- &common.MessagePublication{ //nolint:channelcheck // Only blocks this command + s.injectC <- &common.MessagePublication{ // Note on channel capacity: Only blocks this command TxID: ethcommon.Hash{}.Bytes(), Timestamp: v.Timestamp, Nonce: v.Nonce, @@ -1102,6 +1102,7 @@ func (s *nodePrivilegedService) fetchMissing( return false, fmt.Errorf("failed to create request: %w", err) } + // #nosec G704 -- Admin RPC: BackfillNodes from authorized admin request resp, err := c.Do(req) if err != nil { s.logger.Warn("failed to fetch missing VAA", @@ -1159,7 +1160,7 @@ func (s *nodePrivilegedService) fetchMissing( // Inject into the gossip signed VAA receive path. // This has the same effect as if the VAA was received from the network // (verifying signature, storing in local DB...). - s.signedInC <- &gossipv1.SignedVAAWithQuorum{ //nolint:channelcheck // Only blocks this command + s.signedInC <- &gossipv1.SignedVAAWithQuorum{ // Note on channel capacity: Only blocks this command Vaa: vaaBytes, } @@ -1678,7 +1679,7 @@ func (s *nodePrivilegedService) GetAndObserveMissingVAAs(ctx context.Context, re Timeout: 30 * time.Second, } - // Call the cloud function to get the missing VAAs + // #nosec G704 -- Admin RPC: URL from authorized admin request results, err := client.Do(httpRequest) if err != nil { fmt.Printf("GetAndObserveMissingVAAs: error making http request: %s\n", err) diff --git a/node/pkg/altpub/alternate_pub.go b/node/pkg/altpub/alternate_pub.go index c1ec15e09d2..4cd31c81543 100644 --- a/node/pkg/altpub/alternate_pub.go +++ b/node/pkg/altpub/alternate_pub.go @@ -469,6 +469,7 @@ func (ap *AlternatePublisher) httpPost(ctx context.Context, client *http.Client, r.Header.Add("Content-Type", "application/octet-stream") start := time.Now() + // #nosec G704 -- URL configured at startup, not user input resp, err := client.Do(r) if err != nil { if strings.Contains(err.Error(), "connection refused") { diff --git a/node/pkg/common/nodekey.go b/node/pkg/common/nodekey.go index 58e4ab2b09c..c81b797242d 100644 --- a/node/pkg/common/nodekey.go +++ b/node/pkg/common/nodekey.go @@ -10,6 +10,7 @@ import ( ) func GetOrCreateNodeKey(logger *zap.Logger, path string) (crypto.PrivKey, error) { + // #nosec G703 -- Path comes from operator configuration, not user input b, err := os.ReadFile(path) if err != nil { if os.IsNotExist(err) { @@ -25,6 +26,7 @@ func GetOrCreateNodeKey(logger *zap.Logger, path string) (crypto.PrivKey, error) panic(marshalErr) } + // #nosec G703 -- Path comes from operator configuration, not user input err = os.WriteFile(path, s, 0600) if err != nil { return nil, fmt.Errorf("failed to write node key: %w", err) diff --git a/node/pkg/common/symmetric.go b/node/pkg/common/symmetric.go index 378c82bc54c..15bbf89d9a7 100644 --- a/node/pkg/common/symmetric.go +++ b/node/pkg/common/symmetric.go @@ -37,10 +37,12 @@ func EncryptAESGCM(plaintext, key []byte) ([]byte, error) { if err != nil { return nil, fmt.Errorf("failed to create gcm: %v", err) } - nonce := make([]byte, gcm.NonceSize()) + // Preallocate the output to avoid an extra allocation in append(nonce, out...). + out := make([]byte, gcm.NonceSize(), gcm.NonceSize()+len(plaintext)+gcm.Overhead()) + nonce := out[:gcm.NonceSize()] if _, err = rand.Read(nonce); err != nil { return nil, fmt.Errorf("failed to read random data: %v", err) } - out := gcm.Seal(nil, nonce, plaintext, nil) - return append(nonce, out...), nil + out = gcm.Seal(out, nonce, plaintext, nil) + return out, nil } diff --git a/node/pkg/governor/governor_monitoring.go b/node/pkg/governor/governor_monitoring.go index a54ec821ce7..c84a6dedc4a 100644 --- a/node/pkg/governor/governor_monitoring.go +++ b/node/pkg/governor/governor_monitoring.go @@ -345,7 +345,7 @@ func (gov *ChainGovernor) GetAvailableNotionalByChain() (resp []*publicrpcv1.Gov zap.Uint64("dailyLimit", ce.dailyLimit), zap.Int64("netUsage", netUsage), zap.Error(err)) - } else if uint64(netUsage) > ce.dailyLimit { + } else if uint64(netUsage) > ce.dailyLimit { // #nosec G115 -- netUsage is checked to be non-negative above gov.logger.Warn("GetAvailableNotionalByChain: net value for chain exceeds daily limit even though flow cancel is disabled", zap.String("chainID", chainId.String()), zap.Uint64("dailyLimit", ce.dailyLimit), @@ -515,13 +515,17 @@ func (gov *ChainGovernor) CollectMetrics(ctx context.Context, hb *gossipv1.Heart continue } - if n.Id > math.MaxUint16 { - gov.logger.Error("CollectMetrics: chain id is not a valid uint16", zap.Uint32("chain_id", n.Id)) + // The Governor is only concerned with registered chains. However, it may be possible that Guardians + // are operating with different versions, or may have pending messages from the last 24h for a chain + // that was just deprecated. + // In any case, any uint16 value is accepted here as the worst-case scenario is that the Guardian's + // heartbeat will contain data about a chain ID being unset here. + chain, err := vaa.ChainIDFromNumber[uint32](n.Id) + if err != nil { + gov.logger.Error("CollectMetrics: chain id is not valid", zap.Uint32("chain_id", n.Id), zap.Error(err)) continue } - chain := vaa.ChainID(n.Id) - chainId := fmt.Sprint(n.Id) enabled := "0" totalNotional := "0" diff --git a/node/pkg/governor/governor_prices.go b/node/pkg/governor/governor_prices.go index 152a1b8da54..be6d9a359a9 100644 --- a/node/pkg/governor/governor_prices.go +++ b/node/pkg/governor/governor_prices.go @@ -157,7 +157,7 @@ func (gov *ChainGovernor) queryCoinGecko(ctx context.Context) error { for { select { case <-ticker.C: - throttle <- 1 //nolint:channelcheck // We want this to block for throttling + throttle <- 1 // Note on channel capacity: We want this to block for throttling case <-ctx.Done(): return } diff --git a/node/pkg/manager/der/constants.go b/node/pkg/manager/der/constants.go new file mode 100644 index 00000000000..740cf82af77 --- /dev/null +++ b/node/pkg/manager/der/constants.go @@ -0,0 +1,26 @@ +// Package der contains shared DER signature encoding constants for manager chains. +package der + +const ( + // SequenceTag is the ASN.1 SEQUENCE tag. + SequenceTag = 0x30 + // IntegerTag is the ASN.1 INTEGER tag. + IntegerTag = 0x02 + // FixedOverhead is the DER body overhead for two INTEGER values: integer + // tag + length byte for r, plus integer tag + length byte for s. + FixedOverhead = 4 + // SequenceHeaderLen is the SEQUENCE tag byte plus the total-length byte. + SequenceHeaderLen = 2 + + // Secp256k1OrderLen is the byte length of the secp256k1 curve order (256 bits). + // Source: SEC 2 section 2.4.1, https://www.secg.org/sec2-v2.pdf + Secp256k1OrderLen = 32 + // Secp256k1MaxIntEncodedLen is the maximum DER-encoded integer component length + // for a secp256k1 signature: the curve order (32 bytes) plus one leading zero + // byte that canonicalization may prepend when the high bit is set. + Secp256k1MaxIntEncodedLen = Secp256k1OrderLen + 1 + // MaxTotalLen is the maximum DER SEQUENCE body length for a secp256k1 + // signature: fixed overhead (two tag+length pairs; 4 bytes) plus both + // integer components at their max encoded length. + MaxTotalLen = FixedOverhead + 2*Secp256k1MaxIntEncodedLen +) diff --git a/node/pkg/manager/dogecoin/transaction.go b/node/pkg/manager/dogecoin/transaction.go index 42ccad9dca1..46dd88fec54 100644 --- a/node/pkg/manager/dogecoin/transaction.go +++ b/node/pkg/manager/dogecoin/transaction.go @@ -12,6 +12,7 @@ import ( "github.com/btcsuite/btcd/txscript" "github.com/btcsuite/btcd/wire" "github.com/btcsuite/btcutil" + "github.com/certusone/wormhole/node/pkg/manager/der" "github.com/wormhole-foundation/wormhole/sdk/vaa" ) @@ -206,19 +207,30 @@ func EncodeDERSignature(r, s []byte, hashType txscript.SigHashType) []byte { // Calculate lengths rLen := len(r) sLen := len(s) - totalLen := 4 + rLen + sLen // 0x02 + rLen + r + 0x02 + sLen + s + totalLen := der.FixedOverhead + rLen + sLen // 0x02 + rLen + r + 0x02 + sLen + s + + // DER X.690 section 8.1.3, https://www.itu.int/rec/T-REC-X.690 + // Short-form length encoding uses a single byte for lengths 0-127. + // The constants below reflect the secp256k1 physical maximums; this + // ensures that every length can be encoded in a single byte. + if totalLen > der.MaxTotalLen || rLen > der.Secp256k1MaxIntEncodedLen || sLen > der.Secp256k1MaxIntEncodedLen { + return nil + } + if hashType > 255 { + return nil + } // Build DER signature - sig := make([]byte, 0, totalLen+3) // +3 for 0x30, totalLen, and hashType - sig = append(sig, 0x30) // DER sequence tag - sig = append(sig, byte(totalLen)) // Total length - sig = append(sig, 0x02) // Integer tag for r - sig = append(sig, byte(rLen)) // r length - sig = append(sig, r...) // r value - sig = append(sig, 0x02) // Integer tag for s - sig = append(sig, byte(sLen)) // s length - sig = append(sig, s...) // s value - sig = append(sig, byte(hashType)) // Sighash type + sig := make([]byte, 0, totalLen+der.SequenceHeaderLen+1) // +1 for hashType + sig = append(sig, der.SequenceTag) // DER sequence tag + sig = append(sig, byte(totalLen)) // Total length + sig = append(sig, der.IntegerTag) // Integer tag for r + sig = append(sig, byte(rLen)) // r length + sig = append(sig, r...) // r value + sig = append(sig, der.IntegerTag) // Integer tag for s + sig = append(sig, byte(sLen)) // s length + sig = append(sig, s...) // s value + sig = append(sig, byte(hashType)) // Sighash type return sig } diff --git a/node/pkg/manager/dogecoin/transaction_test.go b/node/pkg/manager/dogecoin/transaction_test.go index 23fbdea3fc1..4c61d303e10 100644 --- a/node/pkg/manager/dogecoin/transaction_test.go +++ b/node/pkg/manager/dogecoin/transaction_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/btcsuite/btcd/txscript" + "github.com/certusone/wormhole/node/pkg/manager/der" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/wormhole-foundation/wormhole/sdk/vaa" @@ -390,6 +391,22 @@ func TestEncodeDERSignatureHighBit(t *testing.T) { assert.Equal(t, byte(0x00), rValue[0], "high-bit r should have 0x00 prefix") } +func TestEncodeDERSignatureLengthOverflow(t *testing.T) { + valid := make([]byte, 32) + tooBig := make([]byte, der.Secp256k1MaxIntEncodedLen+1) + tooBig[0] = 0x01 // non-zero first byte prevents canonicalizeInt from stripping + + t.Run("r too long", func(t *testing.T) { + assert.Nil(t, EncodeDERSignature(tooBig, valid, txscript.SigHashAll)) + }) + t.Run("s too long", func(t *testing.T) { + assert.Nil(t, EncodeDERSignature(valid, tooBig, txscript.SigHashAll)) + }) + t.Run("hash type too large", func(t *testing.T) { + assert.Nil(t, EncodeDERSignature(valid, valid, txscript.SigHashType(256))) + }) +} + func TestCanonicalizeInt(t *testing.T) { tests := []struct { name string diff --git a/node/pkg/manager/helpers_test.go b/node/pkg/manager/helpers_test.go index 08ee32f3281..989086a0d4b 100644 --- a/node/pkg/manager/helpers_test.go +++ b/node/pkg/manager/helpers_test.go @@ -271,6 +271,13 @@ func TestConvertEthSigToDER_Error(t *testing.T) { require.Error(t, err) } +func TestConvertEthSigToDER_EncodeError(t *testing.T) { + sig := makeEthSig(be32([]byte{0x05}), be32([]byte{0x06})) + _, err := convertEthSigToDER(sig, txscript.SigHashType(256)) + require.Error(t, err) + assert.Contains(t, err.Error(), "failed to DER-encode Dogecoin signature") +} + func TestConvertEthSigToXRPLDER(t *testing.T) { sig := makeEthSig(be32([]byte{0x05}), be32([]byte{0x06})) der, err := convertEthSigToXRPLDER(sig) diff --git a/node/pkg/manager/manager.go b/node/pkg/manager/manager.go index 8571b54564e..35ea300fa52 100644 --- a/node/pkg/manager/manager.go +++ b/node/pkg/manager/manager.go @@ -982,7 +982,11 @@ func convertEthSigToDER(ethSig []byte, hashType txscript.SigHashType) ([]byte, e if err != nil { return nil, err } - return dogecoin.EncodeDERSignature(r, s, hashType), nil + derSig := dogecoin.EncodeDERSignature(r, s, hashType) + if derSig == nil { + return nil, fmt.Errorf("failed to DER-encode Dogecoin signature") + } + return derSig, nil } // signXRPLTransaction signs an XRPL Payment transaction for the given XRPL release payload. @@ -1051,7 +1055,11 @@ func convertEthSigToXRPLDER(ethSig []byte) ([]byte, error) { if err != nil { return nil, err } - return xrpl.EncodeDERSignature(r, s), nil + derSig := xrpl.EncodeDERSignature(r, s) + if derSig == nil { + return nil, fmt.Errorf("failed to DER-encode XRPL signature") + } + return derSig, nil } // getCurrentManagerSet retrieves the current manager set for a chain by first looking up diff --git a/node/pkg/manager/xrpl/transaction.go b/node/pkg/manager/xrpl/transaction.go index 9318f7ee1ad..f77f27a7f5a 100644 --- a/node/pkg/manager/xrpl/transaction.go +++ b/node/pkg/manager/xrpl/transaction.go @@ -12,6 +12,7 @@ import ( binarycodec "github.com/Peersyst/xrpl-go/binary-codec" "github.com/Peersyst/xrpl-go/xrpl/transaction" "github.com/Peersyst/xrpl-go/xrpl/transaction/types" + "github.com/certusone/wormhole/node/pkg/manager/der" "github.com/certusone/wormhole/node/pkg/watchers/xrpl/currencycodec" "github.com/wormhole-foundation/wormhole/sdk/vaa" ) @@ -27,15 +28,6 @@ const ( xrplBurnFeeDrops = 15 // secp256k1CompressedPubKeyLen is the length of a compressed secp256k1 public key. secp256k1CompressedPubKeyLen = 33 - - // DER signature encoding constants. - derSequenceTag = 0x30 // ASN.1 SEQUENCE tag - derIntegerTag = 0x02 // ASN.1 INTEGER tag - // derFixedOverhead is the fixed byte overhead inside the DER SEQUENCE body: - // integer tag + length byte for r, plus integer tag + length byte for s. - derFixedOverhead = 4 - // derSequenceHeaderLen is the SEQUENCE tag byte plus the total-length byte. - derSequenceHeaderLen = 2 ) // BuildPaymentTransaction builds and flattens an XRPL Payment transaction for multisigning. @@ -229,17 +221,25 @@ func EncodeDERSignature(r, s []byte) []byte { rLen := len(r) sLen := len(s) - totalLen := derFixedOverhead + rLen + sLen // 0x02 + rLen + r + 0x02 + sLen + s - - sig := make([]byte, 0, totalLen+derSequenceHeaderLen) // +2 for 0x30 and totalLen - sig = append(sig, derSequenceTag) // DER sequence tag - sig = append(sig, byte(totalLen)) // Total length - sig = append(sig, derIntegerTag) // Integer tag for r - sig = append(sig, byte(rLen)) // r length - sig = append(sig, r...) // r value - sig = append(sig, derIntegerTag) // Integer tag for s - sig = append(sig, byte(sLen)) // s length - sig = append(sig, s...) // s value + totalLen := der.FixedOverhead + rLen + sLen // 0x02 + rLen + r + 0x02 + sLen + s + + // DER X.690 §8.1.3 — https://www.itu.int/rec/T-REC-X.690 + // Short-form length encoding uses a single byte for lengths 0–127. + // The constants below reflect the secp256k1 physical maximums; this + // ensures that every length can be encoded in a single byte. + if totalLen > der.MaxTotalLen || rLen > der.Secp256k1MaxIntEncodedLen || sLen > der.Secp256k1MaxIntEncodedLen { + return nil + } + + sig := make([]byte, 0, totalLen+der.SequenceHeaderLen) // +2 for 0x30 and totalLen + sig = append(sig, der.SequenceTag) // DER sequence tag + sig = append(sig, byte(totalLen)) // Total length + sig = append(sig, der.IntegerTag) // Integer tag for r + sig = append(sig, byte(rLen)) // r length + sig = append(sig, r...) // r value + sig = append(sig, der.IntegerTag) // Integer tag for s + sig = append(sig, byte(sLen)) // s length + sig = append(sig, s...) // s value return sig } diff --git a/node/pkg/manager/xrpl/transaction_test.go b/node/pkg/manager/xrpl/transaction_test.go index dc5c74ecba9..2f82ecbef85 100644 --- a/node/pkg/manager/xrpl/transaction_test.go +++ b/node/pkg/manager/xrpl/transaction_test.go @@ -4,6 +4,7 @@ import ( "encoding/hex" "testing" + "github.com/certusone/wormhole/node/pkg/manager/der" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/wormhole-foundation/wormhole/sdk/vaa" @@ -304,6 +305,22 @@ func TestEncodeDERSignatureStripLeadingZeros(t *testing.T) { assert.Equal(t, byte(0x02), derSig[5]) } +func TestEncodeDERSignatureLengthOverflow(t *testing.T) { + valid := make([]byte, 32) + tooBig := make([]byte, der.Secp256k1MaxIntEncodedLen+1) + tooBig[0] = 0x01 // non-zero first byte prevents canonicalizeInt from stripping + + t.Run("r too long", func(t *testing.T) { + assert.Nil(t, EncodeDERSignature(tooBig, valid)) + }) + t.Run("s too long", func(t *testing.T) { + assert.Nil(t, EncodeDERSignature(valid, tooBig)) + }) + t.Run("both too long", func(t *testing.T) { + assert.Nil(t, EncodeDERSignature(tooBig, tooBig)) + }) +} + func TestAccountIDToAddress(t *testing.T) { // Test with a known account ID addr, err := AccountIDToAddress(testCustodyAccountID[:]) diff --git a/node/pkg/node/node_test.go b/node/pkg/node/node_test.go index 9a68c46ea52..92f0b573057 100644 --- a/node/pkg/node/node_test.go +++ b/node/pkg/node/node_test.go @@ -293,6 +293,7 @@ func waitForPromMetricGte(t testing.TB, ctx context.Context, gs []*mockGuardian, } ready := func() bool { // use anonymous function to have proper scope for the defer + // #nosec G704 -- Test making requests to local test nodes resp, err := httpClient.Do(requests[i]) if err != nil { return false @@ -383,6 +384,7 @@ var someMsgEmitterChain vaa.ChainID = vaa.ChainIDSolana func someMessage() *common.MessagePublication { someMsgSequenceCounter++ + // #nosec G115 -- Test helper creating synthetic data with intentional truncation txID := [32]byte{byte(someMsgSequenceCounter % 8), byte(someMsgSequenceCounter / 8), 3} return &common.MessagePublication{ TxID: txID[:], @@ -449,6 +451,7 @@ func governedMsg(shouldBeDelayed bool) *common.MessagePublication { ) tokenBridgeSequenceCounter++ + // #nosec G115 -- Test helper creating synthetic data with intentional truncation txID := [32]byte{byte(tokenBridgeSequenceCounter % 8), byte(tokenBridgeSequenceCounter / 8), 3, 1, 10, 76} return &common.MessagePublication{ TxID: txID[:], @@ -488,6 +491,7 @@ func waitForStatusServer(ctx context.Context, logger *zap.Logger, statusAddr str if err != nil { return err } + // #nosec G704 -- Test making requests to local test status server resp, err := httpClient.Do(req) if err != nil { logger.Info("StatusServer error, waiting 100ms...", zap.String("url", url)) @@ -507,6 +511,7 @@ func waitForStatusServer(ctx context.Context, logger *zap.Logger, statusAddr str if err != nil { return err } + // #nosec G704 -- Test making requests to local test metrics endpoint resp, err := httpClient.Do(req) if err != nil { logger.Info("StatusServer error, waiting 100ms...", zap.String("url", url)) diff --git a/node/pkg/node/options.go b/node/pkg/node/options.go index 36762542a1b..d611eac550f 100644 --- a/node/pkg/node/options.go +++ b/node/pkg/node/options.go @@ -454,7 +454,7 @@ func GuardianOptionWatchers(watcherConfigs []watchers.WatcherConfig, ibcWatcherC zap.String("txID", msg.TxIDString()), zap.Time("timestamp", msg.Timestamp)) } else { - g.msgC.writeC <- msg //nolint:channelcheck // The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations + g.msgC.writeC <- msg // Note on channel capacity: The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations } } } @@ -480,7 +480,7 @@ func GuardianOptionWatchers(watcherConfigs []watchers.WatcherConfig, ibcWatcherC zap.Stringer("watcherChainId", chainId), ) } - g.queryResponseC.writeC <- response //nolint:channelcheck // This channel is buffered, if it backs up we'll stop processing queries until it clears + g.queryResponseC.writeC <- response // Note on channel capacity: This channel is buffered, if it backs up we'll stop processing queries until it clears } } }(chainQueryResponseC[chainId], chainId) diff --git a/node/pkg/node/publicwebRunnable.go b/node/pkg/node/publicwebRunnable.go index 53982694ca4..3c2a68440c2 100644 --- a/node/pkg/node/publicwebRunnable.go +++ b/node/pkg/node/publicwebRunnable.go @@ -168,9 +168,9 @@ func publicwebServiceRunnable( go func() { logger.Info("publicweb server listening", zap.String("addr", srv.Addr)) if tlsHostname != "" { - errC <- srv.ServeTLS(listener, "", "") //nolint:channelcheck // Only does one write + errC <- srv.ServeTLS(listener, "", "") // Note on channel capacity: Only does one write } else { - errC <- srv.Serve(listener) //nolint:channelcheck // Only does one write + errC <- srv.Serve(listener) // Note on channel capacity: Only does one write } }() select { diff --git a/node/pkg/node/reobserve.go b/node/pkg/node/reobserve.go index 5fb691e434b..a775a814cc9 100644 --- a/node/pkg/node/reobserve.go +++ b/node/pkg/node/reobserve.go @@ -3,7 +3,6 @@ package node import ( "context" "encoding/hex" - "math" "time" gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1" @@ -63,15 +62,17 @@ func handleReobservationRequests( } } case req := <-obsvReqC: - if req.ChainId > math.MaxUint16 { - logger.Error("chain id is larger than MaxUint16", + chainId, err := vaa.KnownChainIDFromNumber[uint32](req.ChainId) + if err != nil { + logger.Error("invalid chain id in reobservation request", zap.Uint32("chain_id", req.ChainId), + zap.Error(err), ) continue } r := cachedRequest{ - chainId: vaa.ChainID(req.ChainId), + chainId: chainId, txHash: hex.EncodeToString(req.TxHash), } diff --git a/node/pkg/node/reobserve_test.go b/node/pkg/node/reobserve_test.go index 3ab10ab3df5..600e49a54f9 100644 --- a/node/pkg/node/reobserve_test.go +++ b/node/pkg/node/reobserve_test.go @@ -60,7 +60,7 @@ type reobservationTestContext struct { } func setUpReobservationTest() (reobservationTestContext, func()) { - ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) // #nosec G118 -- Cancel is returned to the caller for cleanup. clock := NewMockClock(time.Now()) @@ -83,7 +83,7 @@ func setUpReobservationTest() (reobservationTestContext, func()) { } func readFromChannel(parent context.Context, c <-chan *gossipv1.ObservationRequest) (*gossipv1.ObservationRequest, bool) { - ctx, cancel := context.WithTimeout(parent, 50*time.Millisecond) + ctx, cancel := context.WithTimeout(parent, 200*time.Millisecond) defer cancel() select { @@ -161,10 +161,10 @@ func TestMultipleReobservations(t *testing.T) { assert.Equal(t, req, actual) // Send a request for the same tx hash but different chain id. - req.ChainId = 3 + req.ChainId = uint32(vaa.ChainIDPolygon) ctx.obsvReqC <- req - actual, ok = readFromChannel(ctx, ctx.chainObsvReqC[vaa.ChainID(req.ChainId)]) // #nosec G115 -- Chain id set to 1 above + actual, ok = readFromChannel(ctx, ctx.chainObsvReqC[vaa.ChainID(req.ChainId)]) // #nosec G115 -- Chain id set to ChainIDPolygon above require.True(t, ok) assert.Equal(t, req, actual) } diff --git a/node/pkg/notary/admincommands.go b/node/pkg/notary/admincommands.go index e14da5de151..559e19c3eb1 100644 --- a/node/pkg/notary/admincommands.go +++ b/node/pkg/notary/admincommands.go @@ -10,6 +10,7 @@ package notary // acceptable. import ( + "encoding/binary" "errors" "fmt" "math/big" @@ -285,10 +286,10 @@ func encodePayloadBytes(payload *vaa.TransferPayloadHdr) []byte { copy(bz[1+offset:33], amtBytes) copy(bz[33:], payload.OriginAddress[:]) - bz[65] = byte(payload.OriginChain) + binary.BigEndian.PutUint16(bz[65:67], uint16(payload.OriginChain)) - copy(bz[66:], payload.TargetAddress[:]) - bz[98] = byte(payload.TargetChain) + copy(bz[67:], payload.TargetAddress[:]) + binary.BigEndian.PutUint16(bz[99:101], uint16(payload.TargetChain)) return bz } diff --git a/node/pkg/p2p/p2p.go b/node/pkg/p2p/p2p.go index f1b7d91b5f7..7f8ba41071b 100644 --- a/node/pkg/p2p/p2p.go +++ b/node/pkg/p2p/p2p.go @@ -923,7 +923,7 @@ func Run(params *RunParams) func(ctx context.Context) error { for { envelope, err := controlSubscription.Next(ctx) // Note: sub.Next(ctx) will return an error once ctx is canceled if err != nil { - errC <- fmt.Errorf("failed to receive pubsub message on control topic: %w", err) //nolint:channelcheck // The runnable will exit anyway + errC <- fmt.Errorf("failed to receive pubsub message on control topic: %w", err) // Note on channel capacity: The runnable will exit anyway return } @@ -1081,7 +1081,7 @@ func Run(params *RunParams) func(ctx context.Context) error { for { envelope, err := attestationSubscription.Next(ctx) // Note: sub.Next(ctx) will return an error once ctx is canceled if err != nil { - errC <- fmt.Errorf("failed to receive pubsub message on attestation topic: %w", err) //nolint:channelcheck // The runnable will exit anyway + errC <- fmt.Errorf("failed to receive pubsub message on attestation topic: %w", err) // Note on channel capacity: The runnable will exit anyway return } @@ -1163,7 +1163,7 @@ func Run(params *RunParams) func(ctx context.Context) error { for { envelope, err := delegatedAttestationSubscription.Next(ctx) // Note: sub.Next(ctx) will return an error once ctx is canceled if err != nil { - errC <- fmt.Errorf("failed to receive pubsub message on delegated attestation topic: %w", err) //nolint:channelcheck // The runnable will exit anyway + errC <- fmt.Errorf("failed to receive pubsub message on delegated attestation topic: %w", err) // Note on channel capacity: The runnable will exit anyway return } @@ -1307,7 +1307,7 @@ func Run(params *RunParams) func(ctx context.Context) error { for { envelope, err := vaaSubscription.Next(ctx) // Note: sub.Next(ctx) will return an error once ctx is canceled if err != nil { - errC <- fmt.Errorf("failed to receive pubsub message on vaa topic: %w", err) //nolint:channelcheck // The runnable will exit anyway + errC <- fmt.Errorf("failed to receive pubsub message on vaa topic: %w", err) // Note on channel capacity: The runnable will exit anyway return } diff --git a/node/pkg/p2p/watermark_test.go b/node/pkg/p2p/watermark_test.go index 3c800754582..4f44808fcc3 100644 --- a/node/pkg/p2p/watermark_test.go +++ b/node/pkg/p2p/watermark_test.go @@ -64,7 +64,7 @@ func NewG(t *testing.T, nodeName string) *G { panic(err) } - _, rootCtxCancel := context.WithCancel(context.Background()) + _, rootCtxCancel := context.WithCancel(context.Background()) // #nosec G118 -- Cancel is invoked by p2p.Run when the runnable exits. g := &G{ batchObsvC: make(chan *node_common.MsgWithTimeStamp[gossipv1.SignedObservationBatch], cs), @@ -119,21 +119,20 @@ func TestWatermark(t *testing.T) { // Create 4 nodes var guardianset = &node_common.GuardianSet{} - var gs [4]*G + var gs [4]*G // #nosec G602 -- Array size is known at compile time, loop uses range for i := range gs { - gs[i] = NewG(t, fmt.Sprintf("n%d", i)) - gs[i].components.Port = uint(LOCAL_P2P_PORTRANGE_START + i) // #nosec G115 -- This is safe as the inputs are constants - gs[i].networkID = "/wormhole/localdev" - - guardianset.Keys = append(guardianset.Keys, crypto.PubkeyToAddress(gs[i].guardianSigner.PublicKey(ctx))) + gs[i] = NewG(t, fmt.Sprintf("n%d", i)) // #nosec G602 -- Loop uses range over fixed-size array + gs[i].components.Port = uint(LOCAL_P2P_PORTRANGE_START + i) // #nosec G115 G602 -- This is safe as the inputs are constants + gs[i].networkID = "/wormhole/localdev" // #nosec G602 -- Loop uses range over fixed-size array + guardianset.Keys = append(guardianset.Keys, crypto.PubkeyToAddress(gs[i].guardianSigner.PublicKey(ctx))) // #nosec G602 -- Loop uses range over fixed-size array id, err := p2ppeer.IDFromPublicKey(gs[0].priv.GetPublic()) require.NoError(t, err) - gs[i].bootstrapPeers = fmt.Sprintf("/ip4/127.0.0.1/udp/%d/quic/p2p/%s", LOCAL_P2P_PORTRANGE_START, id.String()) - gs[i].gst.Set(guardianset) + gs[i].bootstrapPeers = fmt.Sprintf("/ip4/127.0.0.1/udp/%d/quic/p2p/%s", LOCAL_P2P_PORTRANGE_START, id.String()) // #nosec G602 -- Loop uses range over fixed-size array + gs[i].gst.Set(guardianset) // #nosec G602 -- Loop uses range over fixed-size array - gs[i].components.ConnMgr, _ = connmgr.NewConnManager(2, 3, connmgr.WithGracePeriod(2*time.Second)) + gs[i].components.ConnMgr, _ = connmgr.NewConnManager(2, 3, connmgr.WithGracePeriod(2*time.Second)) // #nosec G602 -- Loop uses range over fixed-size array } // The 4th guardian does not put its libp2p key in the heartbeat @@ -189,15 +188,14 @@ func TestManualProtectedPeers(t *testing.T) { // Create 4 protected nodes var guardianset = &node_common.GuardianSet{} - var gs [4]*G + var gs [4]*G // #nosec G602 -- Array size is known at compile time, loop uses range for i := range gs { - gs[i] = NewG(t, fmt.Sprintf("n%d", i)) - gs[i].components.Port = uint(LOCAL_P2P_PORTRANGE_START + i) // #nosec G115 -- This is safe as the inputs are constants - gs[i].networkID = "/wormhole/localdev" - - guardianset.Keys = append(guardianset.Keys, crypto.PubkeyToAddress(gs[i].guardianSigner.PublicKey(ctx))) + gs[i] = NewG(t, fmt.Sprintf("n%d", i)) // #nosec G602 -- Loop uses range over fixed-size array + gs[i].components.Port = uint(LOCAL_P2P_PORTRANGE_START + i) // #nosec G115 G602 -- This is safe as the inputs are constants + gs[i].networkID = "/wormhole/localdev" // #nosec G602 -- Loop uses range over fixed-size array + guardianset.Keys = append(guardianset.Keys, crypto.PubkeyToAddress(gs[i].guardianSigner.PublicKey(ctx))) // #nosec G602 -- Loop uses range over fixed-size array - id, err := p2ppeer.IDFromPublicKey(gs[i].priv.GetPublic()) + id, err := p2ppeer.IDFromPublicKey(gs[i].priv.GetPublic()) // #nosec G602 -- Loop uses range over fixed-size array require.NoError(t, err) protectedPeers = append(protectedPeers, id.String()) // Protect all nodes @@ -205,31 +203,31 @@ func TestManualProtectedPeers(t *testing.T) { bootstrapId, err := p2ppeer.IDFromPublicKey(gs[0].priv.GetPublic()) require.NoError(t, err) - gs[i].bootstrapPeers = fmt.Sprintf("/ip4/127.0.0.1/udp/%d/quic/p2p/%s", LOCAL_P2P_PORTRANGE_START, bootstrapId.String()) + gs[i].bootstrapPeers = fmt.Sprintf("/ip4/127.0.0.1/udp/%d/quic/p2p/%s", LOCAL_P2P_PORTRANGE_START, bootstrapId.String()) // #nosec G602 -- Loop uses range over fixed-size array - gs[i].components.ConnMgr, _ = connmgr.NewConnManager(2, 3, connmgr.WithGracePeriod(2*time.Second)) + gs[i].components.ConnMgr, _ = connmgr.NewConnManager(2, 3, connmgr.WithGracePeriod(2*time.Second)) // #nosec G602 -- Loop uses range over fixed-size array } // Create 4 random nodes that will not be protected - var randomGs [4]*G + var randomGs [4]*G // #nosec G602 -- Array size is known at compile time, loop uses range for i := range randomGs { - randomGs[i] = NewG(t, fmt.Sprintf("r%d", i)) - randomGs[i].components.Port = uint(LOCAL_P2P_PORTRANGE_START + 10 + i) // #nosec G115 -- This is safe as the inputs are constants - randomGs[i].networkID = "/wormhole/localdev" + randomGs[i] = NewG(t, fmt.Sprintf("r%d", i)) // #nosec G602 -- Loop uses range over fixed-size array + randomGs[i].components.Port = uint(LOCAL_P2P_PORTRANGE_START + 10 + i) // #nosec G115 G602 -- This is safe as the inputs are constants + randomGs[i].networkID = "/wormhole/localdev" // #nosec G602 -- Loop uses range over fixed-size array // Set bootstrap to the first protected node bootstrapId, err := p2ppeer.IDFromPublicKey(gs[0].priv.GetPublic()) require.NoError(t, err) // Use the real booststrap peer of the protected nodes - randomGs[i].bootstrapPeers = fmt.Sprintf("/ip4/127.0.0.1/udp/%d/quic/p2p/%s", LOCAL_P2P_PORTRANGE_START, bootstrapId.String()) + randomGs[i].bootstrapPeers = fmt.Sprintf("/ip4/127.0.0.1/udp/%d/quic/p2p/%s", LOCAL_P2P_PORTRANGE_START, bootstrapId.String()) // #nosec G602 -- Loop uses range over fixed-size array - gs[i].components.ConnMgr, _ = connmgr.NewConnManager(2, 3, connmgr.WithGracePeriod(2*time.Second)) + gs[i].components.ConnMgr, _ = connmgr.NewConnManager(2, 3, connmgr.WithGracePeriod(2*time.Second)) // #nosec G602 -- Loop uses range over fixed-size array } // Start the nodes with the manual list of protected peers for i, g := range gs { - gs[i].gst.Set(guardianset) // Set guardian set + gs[i].gst.Set(guardianset) // #nosec G602 -- Loop uses range over fixed-size array startGuardian(t, ctx, g, protectedPeers) } diff --git a/node/pkg/supervisor/supervisor.go b/node/pkg/supervisor/supervisor.go index ac70ba022c0..94a1ac453f4 100644 --- a/node/pkg/supervisor/supervisor.go +++ b/node/pkg/supervisor/supervisor.go @@ -110,7 +110,7 @@ func New(ctx context.Context, logger *zap.Logger, rootRunnable Runnable, opts .. go sup.processor(ctx) - sup.pReq <- &processorRequest{ //nolint:channelcheck // Only does one write + sup.pReq <- &processorRequest{ // Note on channel capacity: Only does one write schedule: &processorRequestSchedule{dn: "root"}, } diff --git a/node/pkg/supervisor/supervisor_node.go b/node/pkg/supervisor/supervisor_node.go index 22e0320cf73..5079969c26a 100644 --- a/node/pkg/supervisor/supervisor_node.go +++ b/node/pkg/supervisor/supervisor_node.go @@ -164,7 +164,7 @@ func (n *node) reset() { // Mark DN and supervisor in context. ctx := context.WithValue(pCtx, dnKey, n.dn()) ctx = context.WithValue(ctx, supervisorKey, n.sup) - ctx, ctxC := context.WithCancel(ctx) + ctx, ctxC := context.WithCancel(ctx) // #nosec G118 -- Cancel stored on node and invoked by processor. // Set context n.ctx = ctx n.ctxC = ctxC @@ -238,7 +238,7 @@ func (n *node) runGroup(runnables map[string]Runnable) error { // Schedule execution of group members. go func() { for name := range runnables { - n.sup.pReq <- &processorRequest{ //nolint:channelcheck // Will only block this go routine + n.sup.pReq <- &processorRequest{ // Note on channel capacity: Will only block this go routine schedule: &processorRequestSchedule{ dn: dns[name], }, diff --git a/node/pkg/supervisor/supervisor_processor.go b/node/pkg/supervisor/supervisor_processor.go index f0f7c60b9be..8fe0b1379f7 100644 --- a/node/pkg/supervisor/supervisor_processor.go +++ b/node/pkg/supervisor/supervisor_processor.go @@ -134,7 +134,7 @@ func (s *supervisor) processSchedule(r *processorRequestSchedule) { if !s.propagatePanic { defer func() { if rec := recover(); rec != nil { - s.pReq <- &processorRequest{ //nolint:channelcheck // Will only block this go routine + s.pReq <- &processorRequest{ // Note on channel capacity: Will only block this go routine died: &processorRequestDied{ dn: r.dn, err: fmt.Errorf("panic: %v, stacktrace: %s", rec, string(debug.Stack())), @@ -146,7 +146,7 @@ func (s *supervisor) processSchedule(r *processorRequestSchedule) { res := n.runnable(n.ctx) - s.pReq <- &processorRequest{ //nolint:channelcheck // Will only block this go routine + s.pReq <- &processorRequest{ // Note on channel capacity: Will only block this go routine died: &processorRequestDied{ dn: r.dn, err: res, @@ -387,7 +387,7 @@ func (s *supervisor) processGC() { // Reschedule node runnable to run after backoff. go func(n *node, bo time.Duration) { time.Sleep(bo) //nolint:forbidigo // TODO: This code should be refactored to not use time.Sleep - s.pReq <- &processorRequest{ //nolint:channelcheck // Will only block this go routine + s.pReq <- &processorRequest{ // Note on channel capacity: Will only block this go routine schedule: &processorRequestSchedule{dn: n.dn()}, } }(n, bo) diff --git a/node/pkg/supervisor/supervisor_support.go b/node/pkg/supervisor/supervisor_support.go index a93e4d2bf10..99570eb55f3 100644 --- a/node/pkg/supervisor/supervisor_support.go +++ b/node/pkg/supervisor/supervisor_support.go @@ -18,7 +18,7 @@ func GRPCServer(srv *grpc.Server, lis net.Listener, graceful bool) Runnable { Signal(ctx, SignalHealthy) errC := make(chan error) go func() { - errC <- srv.Serve(lis) //nolint:channelcheck // Will only block this go routine + errC <- srv.Serve(lis) // Note on channel capacity: Will only block this go routine }() select { case <-ctx.Done(): diff --git a/node/pkg/telemetry/loki.go b/node/pkg/telemetry/loki.go index a5d350a290b..bf0059b6961 100644 --- a/node/pkg/telemetry/loki.go +++ b/node/pkg/telemetry/loki.go @@ -209,7 +209,7 @@ func logWriter(ctx context.Context, logger *zap.Logger, localC chan api.Entry, w // Write to Loki in a blocking manner unless we are signaled to shutdown. select { - case c.Chan() <- entry: //nolint:channelcheck // We want to block on the Loki client. + case c.Chan() <- entry: // Note on channel capacity: We want to block on the Loki client. pendingEntry = nil case <-ctx.Done(): // Time to shutdown. We probably failed to write this message, save it so we can try to flush it. @@ -240,7 +240,7 @@ func flushLogsWithTimeout(localC chan api.Entry, c client.Client, pendingEntry * if pendingEntry != nil { select { - case c.Chan() <- *pendingEntry: //nolint:channelcheck // We want to block on the Loki client. The timeout will interrupt us. + case c.Chan() <- *pendingEntry: // Note on channel capacity: We want to block on the Loki client. The timeout will interrupt us. case <-timeout.Done(): // If we timeout, we didn't write the pending one, so count that as remaining. return (1 + len(localC)), errors.New("timeout writing pending entry") @@ -250,7 +250,7 @@ func flushLogsWithTimeout(localC chan api.Entry, c client.Client, pendingEntry * for len(localC) > 0 { select { case entry := <-localC: - c.Chan() <- entry //nolint:channelcheck // We want to block on the Loki client. The timeout will interrupt us. + c.Chan() <- entry // Note on channel capacity: We want to block on the Loki client. The timeout will interrupt us. case <-timeout.Done(): // If we timeout, we didn't write the current one, so count that as remaining. return (1 + len(localC)), errors.New("timeout flushing buffered entry") @@ -282,7 +282,7 @@ func stopClientWithTimeout(c client.Client) error { stopExitedC := make(chan struct{}, 1) go func(c client.Client) { c.StopNow() - stopExitedC <- struct{}{} //nolint:channelcheck // We only do a single write. + stopExitedC <- struct{}{} // Note on channel capacity: We only do a single write. }(c) // Wait for the go routine to exit or the timer to expire. Using `time.After` since this is a one shot and we don't have the context. diff --git a/node/pkg/telemetry/prom_remote_write/scrape.go b/node/pkg/telemetry/prom_remote_write/scrape.go index 0e24fa8a334..ad2e3fd309b 100644 --- a/node/pkg/telemetry/prom_remote_write/scrape.go +++ b/node/pkg/telemetry/prom_remote_write/scrape.go @@ -67,6 +67,7 @@ func ScrapeAndSendLocalMetrics(ctx context.Context, info PromTelemetryInfo, logg req.Header.Set("User-Agent", "Guardian") req.Header.Set("X-Prometheus-Remote-Write-Version", "0.1.0") + // #nosec G704 -- Prometheus remote write URL from configuration res, err := http.DefaultClient.Do(req) if err != nil { logger.Error("Error creating http request", zap.Error(err)) diff --git a/node/pkg/txverifier/evm_test.go b/node/pkg/txverifier/evm_test.go index 365533df39b..ccf81dcfe6a 100644 --- a/node/pkg/txverifier/evm_test.go +++ b/node/pkg/txverifier/evm_test.go @@ -110,7 +110,7 @@ func setup() *mockConnections { client: &mockClient{}, logger: *logger, } - ctx, ctxCancel := context.WithCancel(context.Background()) + ctx, ctxCancel := context.WithCancel(context.Background()) // #nosec G118 -- Cancel is owned by the test via mocks.ctxCancel(). return &mockConnections{ transferVerifier, diff --git a/node/pkg/txverifier/evmtypes.go b/node/pkg/txverifier/evmtypes.go index 96621ca6cfe..1e83f789f77 100644 --- a/node/pkg/txverifier/evmtypes.go +++ b/node/pkg/txverifier/evmtypes.go @@ -275,9 +275,8 @@ func (s *Subscription) Subscribe(ctx context.Context) { ) if err != nil { - s.errC <- fmt.Errorf("failed to subscribe to logs: %w", err) //nolint:channelcheck // Will only block this subscriber routine + s.errC <- fmt.Errorf("failed to subscribe to logs: %w", err) // Note on channel capacity: Will only block this subscriber routine time.Sleep(RECONNECT_DELAY) //nolint:forbidigo // TODO: This code should be refactored to not use time.Sleep; // Wait before retrying - continue } @@ -287,9 +286,8 @@ func (s *Subscription) Subscribe(ctx context.Context) { err = s.handleSubscription(ctx, subscription) if err != nil { - s.errC <- err //nolint:channelcheck // Will only block this subscriber routine + s.errC <- err // Note on channel capacity: Will only block this subscriber routine time.Sleep(RECONNECT_DELAY) //nolint:forbidigo // TODO: This code should be refactored to not use time.Sleep; // Wait before retrying - } } } diff --git a/node/pkg/watchers/algorand/watcher.go b/node/pkg/watchers/algorand/watcher.go index ab88db9623c..87bebf5ccd2 100644 --- a/node/pkg/watchers/algorand/watcher.go +++ b/node/pkg/watchers/algorand/watcher.go @@ -207,7 +207,7 @@ func lookAtTxn(e *Watcher, t types.SignedTxnInBlock, b types.Block, logger *zap. zap.Uint8("consistency_level", observation.ConsistencyLevel), ) - e.msgC <- observation //nolint:channelcheck // The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations + e.msgC <- observation // Note on channel capacity: The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations } } diff --git a/node/pkg/watchers/aptos/watcher.go b/node/pkg/watchers/aptos/watcher.go index d7f96fa4593..eb678535d9b 100644 --- a/node/pkg/watchers/aptos/watcher.go +++ b/node/pkg/watchers/aptos/watcher.go @@ -399,7 +399,7 @@ func (e *Watcher) observeData(logger *zap.Logger, data gjson.Result, nativeSeq u zap.Uint8("consistencyLevel", observation.ConsistencyLevel), ) - e.msgC <- observation //nolint:channelcheck // The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations + e.msgC <- observation // Note on channel capacity: The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations } // logVersion retrieves the Aptos node version and logs it diff --git a/node/pkg/watchers/cosmwasm/watcher.go b/node/pkg/watchers/cosmwasm/watcher.go index 1250c559322..cf7c96d7b9e 100644 --- a/node/pkg/watchers/cosmwasm/watcher.go +++ b/node/pkg/watchers/cosmwasm/watcher.go @@ -221,7 +221,7 @@ func (e *Watcher) Run(ctx context.Context) error { blocksBody, err := common.SafeRead(resp.Body) if err != nil { logger.Error("query latest block response read error", zap.String("network", e.networkName), zap.Error(err)) - errC <- err //nolint:channelcheck // The watcher will exit anyway + errC <- err // Note on channel capacity: The watcher will exit anyway resp.Body.Close() continue } @@ -302,7 +302,7 @@ func (e *Watcher) Run(ctx context.Context) error { msgs := EventsToMessagePublications(e.contract, txHash, events.Array(), logger, e.chainID, contractAddressLogKey, e.b64Encoded) for _, msg := range msgs { msg.IsReobservation = true - e.msgC <- msg //nolint:channelcheck // The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations + e.msgC <- msg // Note on channel capacity: The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations messagesConfirmed.WithLabelValues(e.networkName).Inc() watchers.ReobservationsByChain.WithLabelValues(e.networkName, "std").Inc() } @@ -321,7 +321,7 @@ func (e *Watcher) Run(ctx context.Context) error { p2p.DefaultRegistry.AddErrorCount(e.chainID, 1) connectionErrors.WithLabelValues(e.networkName, "channel_read_error").Inc() logger.Error("error reading channel", zap.String("network", e.networkName), zap.Error(err)) - errC <- err //nolint:channelcheck // The watcher will exit anyway + errC <- err // Note on channel capacity: The watcher will exit anyway return nil } @@ -343,7 +343,7 @@ func (e *Watcher) Run(ctx context.Context) error { msgs := EventsToMessagePublications(e.contract, txHash, events.Array(), logger, e.chainID, e.contractAddressLogKey, e.b64Encoded) for _, msg := range msgs { - e.msgC <- msg //nolint:channelcheck // The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations + e.msgC <- msg // Note on channel capacity: The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations messagesConfirmed.WithLabelValues(e.networkName).Inc() } diff --git a/node/pkg/watchers/evm/connectors/batch_poller.go b/node/pkg/watchers/evm/connectors/batch_poller.go index 1447bd1cae4..6978bc053b0 100644 --- a/node/pkg/watchers/evm/connectors/batch_poller.go +++ b/node/pkg/watchers/evm/connectors/batch_poller.go @@ -85,11 +85,11 @@ func (b *BatchPollConnector) SubscribeForBlocks(ctx context.Context, errC chan e // Publish the initial finalized and safe blocks so we have a starting point for reobservation requests. for idx, block := range lastBlocks { b.logger.Info(fmt.Sprintf("publishing initial %s block", b.batchData[idx].finality), zap.Uint64("initial_block", block.Number.Uint64())) - sink <- block //nolint:channelcheck // This channel is buffered, if it backs up, we will just stop polling until it clears + sink <- block // Note on channel capacity: This channel is buffered, if it backs up, we will just stop polling until it clears if b.generateSafe && b.batchData[idx].finality == Finalized { safe := block.Copy(Safe) b.logger.Info("publishing generated initial safe block", zap.Uint64("initial_block", safe.Number.Uint64())) - sink <- safe //nolint:channelcheck // This channel is buffered, if it backs up, we will just stop polling until it clears + sink <- safe // Note on channel capacity: This channel is buffered, if it backs up, we will just stop polling until it clears } } @@ -106,7 +106,7 @@ func (b *BatchPollConnector) SubscribeForBlocks(ctx context.Context, errC chan e errCount++ b.logger.Error("batch polling encountered an error", zap.Int("errCount", errCount), zap.Error(err)) if errCount > 3 { - errC <- fmt.Errorf("polling encountered too many errors: %w", err) //nolint:channelcheck // The watcher will exit anyway + errC <- fmt.Errorf("polling encountered too many errors: %w", err) // Note on channel capacity: The watcher will exit anyway return nil } } else if errCount != 0 { @@ -122,7 +122,7 @@ func (b *BatchPollConnector) SubscribeForBlocks(ctx context.Context, errC chan e b.logger.Error("new latest header block number is nil") continue } - sink <- &NewBlock{ //nolint:channelcheck // This channel is buffered, if it backs up, we will just stop polling until it clears + sink <- &NewBlock{ // Note on channel capacity: This channel is buffered, if it backs up, we will just stop polling until it clears Number: ev.Number, Time: ev.Time, Hash: ev.Hash(), @@ -201,9 +201,9 @@ func (b *BatchPollConnector) pollBlocks(ctx context.Context, sink chan<- *NewBlo errorFound = true break } - sink <- block //nolint:channelcheck // This channel is buffered, if it backs up, we will just stop polling until it clears + sink <- block // Note on channel capacity: This channel is buffered, if it backs up, we will just stop polling until it clears if b.generateSafe && b.batchData[idx].finality == Finalized { - sink <- block.Copy(Safe) //nolint:channelcheck // This channel is buffered, if it backs up, we will just stop polling until it clears + sink <- block.Copy(Safe) // Note on channel capacity: This channel is buffered, if it backs up, we will just stop polling until it clears } lastPublishedBlock = block } @@ -214,9 +214,9 @@ func (b *BatchPollConnector) pollBlocks(ctx context.Context, sink chan<- *NewBlo if !errorFound { // The original value of newBlocks is still good. - sink <- newBlock //nolint:channelcheck // This channel is buffered, if it backs up, we will just stop polling until it clears + sink <- newBlock // Note on channel capacity: This channel is buffered, if it backs up, we will just stop polling until it clears if b.generateSafe && b.batchData[idx].finality == Finalized { - sink <- newBlock.Copy(Safe) //nolint:channelcheck // This channel is buffered, if it backs up, we will just stop polling until it clears + sink <- newBlock.Copy(Safe) // Note on channel capacity: This channel is buffered, if it backs up, we will just stop polling until it clears } } else { newBlocks[idx] = lastPublishedBlock diff --git a/node/pkg/watchers/evm/connectors/common.go b/node/pkg/watchers/evm/connectors/common.go index e0cf07874cc..c5607800428 100644 --- a/node/pkg/watchers/evm/connectors/common.go +++ b/node/pkg/watchers/evm/connectors/common.go @@ -91,7 +91,7 @@ func (sub *PollSubscription) Err() <-chan error { func (sub *PollSubscription) Unsubscribe() { sub.errOnce.Do(func() { select { - case sub.quit <- ErrUnsubscribed: //nolint:channelcheck // We only do a single write. + case sub.quit <- ErrUnsubscribed: // Note on channel capacity: We only do a single write. <-sub.unsubDone case <-sub.unsubDone: } diff --git a/node/pkg/watchers/evm/connectors/instant_finality.go b/node/pkg/watchers/evm/connectors/instant_finality.go index 9e2779e5ef4..a92a7fe0416 100644 --- a/node/pkg/watchers/evm/connectors/instant_finality.go +++ b/node/pkg/watchers/evm/connectors/instant_finality.go @@ -54,9 +54,9 @@ func (c *InstantFinalityConnector) SubscribeForBlocks(ctx context.Context, errC Hash: ev.Hash(), Finality: Finalized, } - sink <- block //nolint:channelcheck // This channel is buffered, if it backs up, we will just stop polling until it clears - sink <- block.Copy(Safe) //nolint:channelcheck // This channel is buffered, if it backs up, we will just stop polling until it clears - sink <- block.Copy(Latest) //nolint:channelcheck // This channel is buffered, if it backs up, we will just stop polling until it clears + sink <- block // Note on channel capacity: This channel is buffered, if it backs up, we will just stop polling until it clears + sink <- block.Copy(Safe) // Note on channel capacity: This channel is buffered, if it backs up, we will just stop polling until it clears + sink <- block.Copy(Latest) // Note on channel capacity: This channel is buffered, if it backs up, we will just stop polling until it clears } } }) diff --git a/node/pkg/watchers/evm/watcher.go b/node/pkg/watchers/evm/watcher.go index 399e5603724..9783284de90 100644 --- a/node/pkg/watchers/evm/watcher.go +++ b/node/pkg/watchers/evm/watcher.go @@ -387,7 +387,7 @@ func (w *Watcher) Run(parentCtx context.Context) error { return nil case <-t.C: if pollErr := w.fetchAndUpdateGuardianSet(ctx, w.ethConn); pollErr != nil { - errC <- fmt.Errorf("failed to request guardian set: %v", pollErr) //nolint:channelcheck // The watcher will exit anyway + errC <- fmt.Errorf("failed to request guardian set: %v", pollErr) // Note on channel capacity: The watcher will exit anyway return nil } } @@ -413,7 +413,7 @@ func (w *Watcher) Run(parentCtx context.Context) error { return nil case <-t.C: if pollErr := w.fetchAndUpdateDelegatedGuardianConfig(ctx); pollErr != nil { - errC <- fmt.Errorf("failed to request delegated guardian config: %v", pollErr) //nolint:channelcheck // The watcher will exit anyway + errC <- fmt.Errorf("failed to request delegated guardian config: %v", pollErr) // Note on channel capacity: The watcher will exit anyway return nil } } @@ -427,16 +427,18 @@ func (w *Watcher) Run(parentCtx context.Context) error { case <-ctx.Done(): return nil case r := <-w.obsvReqC: - if r.ChainId > math.MaxUint16 { - logger.Error("chain id for observation request is not a valid uint16", + chainId, chainErr := vaa.KnownChainIDFromNumber[uint32](r.ChainId) + if chainErr != nil { + logger.Error("invalid chain id for observation request", zap.Uint32("chainID", r.ChainId), zap.String("txID", hex.EncodeToString(r.TxHash)), + zap.Error(chainErr), ) continue } numObservations, handleErr := w.handleReobservationRequest( ctx, - vaa.ChainID(r.ChainId), + chainId, r.TxHash, w.ethConn, atomic.LoadUint64(&w.latestFinalizedBlockNumber), @@ -474,7 +476,7 @@ func (w *Watcher) Run(parentCtx context.Context) error { return nil case subErr := <-messageSub.Err(): ethConnectionErrors.WithLabelValues(w.networkName, "subscription_error").Inc() - errC <- fmt.Errorf("error while processing message publication subscription: %w", subErr) //nolint:channelcheck // The watcher will exit anyway + errC <- fmt.Errorf("error while processing message publication subscription: %w", subErr) // The watcher will exit anyway p2p.DefaultRegistry.AddErrorCount(w.chainID, 1) return nil case ev := <-messageC: @@ -486,7 +488,7 @@ func (w *Watcher) Run(parentCtx context.Context) error { continue } p2p.DefaultRegistry.AddErrorCount(w.chainID, 1) - errC <- fmt.Errorf("failed to request timestamp for block %d, hash %s: %w", ev.Raw.BlockNumber, ev.Raw.BlockHash.String(), blockErr) //nolint:channelcheck // The watcher will exit anyway + errC <- fmt.Errorf("failed to request timestamp for block %d, hash %s: %w", ev.Raw.BlockNumber, ev.Raw.BlockHash.String(), blockErr) // The watcher will exit anyway return nil } @@ -514,7 +516,7 @@ func (w *Watcher) Run(parentCtx context.Context) error { case err := <-headerSubscription.Err(): logger.Error("error while processing header subscription", zap.Error(err)) ethConnectionErrors.WithLabelValues(w.networkName, "header_subscription_error").Inc() - errC <- fmt.Errorf("error while processing header subscription: %w", err) //nolint:channelcheck // The watcher will exit anyway + errC <- fmt.Errorf("error while processing header subscription: %w", err) // Note on channel capacity: The watcher will exit anyway p2p.DefaultRegistry.AddErrorCount(w.chainID, 1) return nil case ev := <-headSink: @@ -759,7 +761,7 @@ func (w *Watcher) fetchAndUpdateGuardianSet( w.currentGuardianSet = &idx if w.setC != nil { - w.setC <- common.NewGuardianSet(gs.Keys, idx) //nolint:channelcheck // Will only block the guardian set update routine + w.setC <- common.NewGuardianSet(gs.Keys, idx) // Note on channel capacity: Will only block the guardian set update routine } return nil @@ -849,7 +851,7 @@ func (w *Watcher) fetchAndUpdateDelegatedGuardianConfig( zap.Uint32("timestamp", cfg.Timestamp)) } - w.dgConfigC <- dgConfig //nolint:channelcheck // Will only block the delegated guardian config update routine + w.dgConfigC <- dgConfig // Note on channel capacity: Will only block the delegated guardian config update routine w.logger.Info("sent delegated guardian config update to processor") } @@ -1101,7 +1103,7 @@ func (w *Watcher) verifyAndPublish( "publishing new message publication", msg.ZapFields()..., ) - w.msgC <- msg //nolint:channelcheck // The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations + w.msgC <- msg // Note on channel capacity: The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations ethMessagesConfirmed.WithLabelValues(w.networkName).Inc() if msg.IsReobservation { watchers.ReobservationsByChain.WithLabelValues(w.chainID.String(), "std").Inc() @@ -1155,7 +1157,7 @@ func (w *Watcher) waitForBlockTime(ctx context.Context, errC chan error, ev *eth ethConnectionErrors.WithLabelValues(w.networkName, "block_by_number_error").Inc() if !canRetryGetBlockTime(err) { p2p.DefaultRegistry.AddErrorCount(w.chainID, 1) - errC <- fmt.Errorf("failed to request timestamp for block %d, hash %s: %w", ev.Raw.BlockNumber, ev.Raw.BlockHash.String(), err) //nolint:channelcheck // The watcher will exit anyway + errC <- fmt.Errorf("failed to request timestamp for block %d, hash %s: %w", ev.Raw.BlockNumber, ev.Raw.BlockHash.String(), err) // Note on channel capacity: The watcher will exit anyway return } if retries >= MaxRetries { diff --git a/node/pkg/watchers/ibc/watcher.go b/node/pkg/watchers/ibc/watcher.go index 68f1a149c99..639f68febdb 100644 --- a/node/pkg/watchers/ibc/watcher.go +++ b/node/pkg/watchers/ibc/watcher.go @@ -689,7 +689,7 @@ func (w *Watcher) processIbcReceivePublishEvent(evt *ibcReceivePublishEvent, obs zap.Uint8("ConsistencyLevel", evt.Msg.ConsistencyLevel), ) - ce.msgC <- evt.Msg //nolint:channelcheck // The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations + ce.msgC <- evt.Msg // Note on channel capacity: The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations messagesConfirmed.WithLabelValues(ce.chainName).Inc() if evt.Msg.IsReobservation { watchers.ReobservationsByChain.WithLabelValues(evt.Msg.EmitterChain.String(), "std").Inc() diff --git a/node/pkg/watchers/mock/watcher.go b/node/pkg/watchers/mock/watcher.go index 2db881c454a..afd6773959b 100644 --- a/node/pkg/watchers/mock/watcher.go +++ b/node/pkg/watchers/mock/watcher.go @@ -29,9 +29,9 @@ func NewWatcherRunnable( return nil case observation := <-c.MockObservationC: logger.Info("message observed", observation.ZapFields(zap.String("digest", observation.CreateDigest()))...) - msgC <- observation //nolint:channelcheck // The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations + msgC <- observation // Note on channel capacity: The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations case gs := <-c.MockSetC: - setC <- gs //nolint:channelcheck // Will only block this mock watcher + setC <- gs // Note on channel capacity: Will only block this mock watcher case o := <-obsvReqC: hash := eth_common.BytesToHash(o.TxHash) logger.Info("Received obsv request", zap.String("log_msg_type", "obsv_req_received"), zap.String("tx_hash", hash.Hex())) @@ -39,7 +39,7 @@ func NewWatcherRunnable( if ok { msg2 := *msg msg2.IsReobservation = true - msgC <- &msg2 //nolint:channelcheck // The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations + msgC <- &msg2 // Note on channel capacity: The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations } } } diff --git a/node/pkg/watchers/near/finalizer.go b/node/pkg/watchers/near/finalizer.go index 3e26f02d2de..612146651a1 100644 --- a/node/pkg/watchers/near/finalizer.go +++ b/node/pkg/watchers/near/finalizer.go @@ -64,7 +64,7 @@ func (f Finalizer) isFinalized(logger *zap.Logger, ctx context.Context, queriedB } logger.Debug("block finalization cache miss", zap.String("method", "isFinalized"), zap.String("parameters", queriedBlockHash)) - f.eventChan <- EVENT_FINALIZED_CACHE_MISS //nolint:channelcheck // Only pauses this watcher + f.eventChan <- EVENT_FINALIZED_CACHE_MISS // Note on channel capacity: Only pauses this watcher queriedBlock, err := f.nearAPI.GetBlock(ctx, queriedBlockHash) if err != nil { diff --git a/node/pkg/watchers/near/nearapi/mock/mock_server.go b/node/pkg/watchers/near/nearapi/mock/mock_server.go index ab316bb13e5..90a5ef21c3c 100644 --- a/node/pkg/watchers/near/nearapi/mock/mock_server.go +++ b/node/pkg/watchers/near/nearapi/mock/mock_server.go @@ -82,7 +82,10 @@ func (s *ForwardingCachingServer) ProxyReq(_ *zap.Logger, req *http.Request) (*h req.Body = io.NopCloser(bytes.NewReader(reqBody)) url := fmt.Sprintf("%s%s", s.upstreamHost, req.RequestURI) - proxyReq, _ := http.NewRequestWithContext(req.Context(), req.Method, url, bytes.NewReader(reqBody)) + proxyReq, err := http.NewRequestWithContext(req.Context(), req.Method, url, bytes.NewReader(reqBody)) // #nosec G704 -- Test mock server forwards only to configured upstream. + if err != nil { + return nil, err + } s.logger.Debug("proxy_req", zap.String("url", url), @@ -157,6 +160,7 @@ func (s *ForwardingCachingServer) ServeHTTP(w http.ResponseWriter, req *http.Req } httpClient := http.Client{} + // #nosec G704 -- Test mock server making requests to configured upstream resp, reqErr := httpClient.Do(proxyReq) if reqErr != nil { http.Error(w, reqErr.Error(), http.StatusBadGateway) @@ -170,6 +174,7 @@ func (s *ForwardingCachingServer) ServeHTTP(w http.ResponseWriter, req *http.Req } // cache the result + // #nosec G703 -- Test mock server with controlled cache directory, not user input err = os.WriteFile(filename, respBody, 0600) panicIfError(err) diff --git a/node/pkg/watchers/near/nearapi/nearapi.go b/node/pkg/watchers/near/nearapi/nearapi.go index ce653ad7050..882f8a033cd 100644 --- a/node/pkg/watchers/near/nearapi/nearapi.go +++ b/node/pkg/watchers/near/nearapi/nearapi.go @@ -83,6 +83,7 @@ func (n HttpNearRpc) Query(ctx context.Context, s string) ([]byte, error) { // perform HTTP request req, _ := http.NewRequestWithContext(timeout, http.MethodPost, n.nearRpc, bytes.NewBuffer([]byte(s))) req.Header.Add("Content-Type", "application/json") + // #nosec G704 -- NEAR RPC URL from operator configuration resp, err := n.nearHttpClient.Do(req) if err == nil { diff --git a/node/pkg/watchers/near/poll.go b/node/pkg/watchers/near/poll.go index e5b9c827e49..0c084f065b5 100644 --- a/node/pkg/watchers/near/poll.go +++ b/node/pkg/watchers/near/poll.go @@ -50,7 +50,7 @@ func (e *Watcher) recursivelyReadFinalizedBlocks(logger *zap.Logger, ctx context // we want to avoid going too far back because that would increase the likelihood of error somewhere in the recursion stack. // If we go back too far, we just report the error and terminate early. if recursionDepth > maxFallBehindBlocks { - e.eventChan <- EVENT_NEAR_WATCHER_TOO_FAR_BEHIND //nolint:channelcheck // Only pauses this watcher + e.eventChan <- EVENT_NEAR_WATCHER_TOO_FAR_BEHIND // Note on channel capacity: Only pauses this watcher return errors.New("recursivelyReadFinalizedBlocks: maxFallBehindBlocks") } @@ -71,7 +71,7 @@ func (e *Watcher) recursivelyReadFinalizedBlocks(logger *zap.Logger, ctx context chunks := startBlock.ChunkHashes() // process chunks after recursion such that youngest chunks get processed first for i := 0; i < len(chunks); i++ { - chunkSink <- chunks[i] //nolint:channelcheck // Only pauses this watcher + chunkSink <- chunks[i] // Note on channel capacity: Only pauses this watcher } return nil } diff --git a/node/pkg/watchers/near/tx_processing.go b/node/pkg/watchers/near/tx_processing.go index 04f57a297c7..53cb1e6a837 100644 --- a/node/pkg/watchers/near/tx_processing.go +++ b/node/pkg/watchers/near/tx_processing.go @@ -256,7 +256,7 @@ func (e *Watcher) processWormholeLog(logger *zap.Logger, _ context.Context, job // tell everyone about it job.hasWormholeMsg = true - e.eventChan <- EVENT_NEAR_MESSAGE_CONFIRMED //nolint:channelcheck // Only pauses this watcher + e.eventChan <- EVENT_NEAR_MESSAGE_CONFIRMED // Note on channel capacity: Only pauses this watcher logger.Info("message observed", zap.String("log_msg_type", "wormhole_event_success"), @@ -270,7 +270,7 @@ func (e *Watcher) processWormholeLog(logger *zap.Logger, _ context.Context, job zap.Uint8("consistency_level", observation.ConsistencyLevel), ) - e.msgC <- observation //nolint:channelcheck // The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations + e.msgC <- observation // Note on channel capacity: The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations return nil } diff --git a/node/pkg/watchers/near/watcher.go b/node/pkg/watchers/near/watcher.go index ac50568923b..344485c093d 100644 --- a/node/pkg/watchers/near/watcher.go +++ b/node/pkg/watchers/near/watcher.go @@ -266,7 +266,7 @@ func (e *Watcher) runTxProcessor(ctx context.Context) error { if job.hasWormholeMsg { // report how long it took to process this transaction - e.eventChanTxProcessedDuration <- time.Since(job.creationTime) //nolint:channelcheck // Only pauses this watcher + e.eventChanTxProcessedDuration <- time.Since(job.creationTime) // Note on channel capacity: Only pauses this watcher } } @@ -346,7 +346,7 @@ func (e *Watcher) schedule(ctx context.Context, job *transactionProcessingJob, d select { case <-ctx.Done(): return nil - case e.transactionProcessingQueue <- job: //nolint:channelcheck // Only blocking this go routine. + case e.transactionProcessingQueue <- job: // Note on channel capacity: Only blocking this go routine. } } return nil diff --git a/node/pkg/watchers/solana/client.go b/node/pkg/watchers/solana/client.go index 63bd9fbbf3c..707f500391a 100644 --- a/node/pkg/watchers/solana/client.go +++ b/node/pkg/watchers/solana/client.go @@ -5,7 +5,6 @@ import ( "context" "errors" "fmt" - "math" "strings" "sync" "sync/atomic" @@ -414,7 +413,7 @@ func (s *SolanaWatcher) setupWebSocket(ctx context.Context) error { logger.Error("failed to read from account web socket", zap.Error(err)) return err } else { - s.pumpData <- msg //nolint:channelcheck // Only pauses this watcher + s.pumpData <- msg // Note on channel capacity: Only pauses this watcher } } } @@ -499,20 +498,22 @@ func (s *SolanaWatcher) Run(ctx context.Context) error { if err != nil { p2p.DefaultRegistry.AddErrorCount(s.chainID, 1) solanaConnectionErrors.WithLabelValues(s.networkName, string(s.commitment), "account_subscription_data").Inc() - s.errC <- err //nolint:channelcheck // The watcher will exit anyway + s.errC <- err // Note on channel capacity: The watcher will exit anyway return err } case m := <-s.obsvReqC: - if m.ChainId > math.MaxUint16 { - logger.Error("chain id for observation request is not a valid uint16", + chainId, err := vaa.KnownChainIDFromNumber[uint32](m.ChainId) + if err != nil { + logger.Error("invalid chain id for observation request", zap.Uint32("chainID", m.ChainId), zap.String("txID", hex.EncodeToString(m.TxHash)), + zap.Error(err), ) continue } //nolint:contextcheck // Passed via the 's' object instead of as a parameter. - numObservations, err := s.handleReobservationRequest(vaa.ChainID(m.ChainId), m.TxHash, s.rpcClient) + numObservations, err := s.handleReobservationRequest(chainId, m.TxHash, s.rpcClient) if err != nil { logger.Error("failed to process observation request", zap.Uint32("chainID", m.ChainId), @@ -536,7 +537,7 @@ func (s *SolanaWatcher) Run(ctx context.Context) error { if err != nil { p2p.DefaultRegistry.AddErrorCount(s.chainID, 1) solanaConnectionErrors.WithLabelValues(s.networkName, string(s.commitment), "get_slot_error").Inc() - s.errC <- err //nolint:channelcheck // The watcher will exit anyway + s.errC <- err // Note on channel capacity: The watcher will exit anyway return err } @@ -1269,7 +1270,7 @@ func (s *SolanaWatcher) processMessageAccount(logger *zap.Logger, messageAccount ) } - s.msgC <- observation //nolint:channelcheck // The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations + s.msgC <- observation // Note on channel capacity: The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations return 1 } diff --git a/node/pkg/watchers/solana/shim.go b/node/pkg/watchers/solana/shim.go index d4a861026d2..680bd198be4 100644 --- a/node/pkg/watchers/solana/shim.go +++ b/node/pkg/watchers/solana/shim.go @@ -392,7 +392,7 @@ func (s *SolanaWatcher) shimProcessRest( ) } - s.msgC <- observation //nolint:channelcheck // The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations + s.msgC <- observation // Note on channel capacity: The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations return nil } diff --git a/node/pkg/watchers/sui/watcher.go b/node/pkg/watchers/sui/watcher.go index 8dd54fa74ea..f59353bbaaa 100644 --- a/node/pkg/watchers/sui/watcher.go +++ b/node/pkg/watchers/sui/watcher.go @@ -234,7 +234,7 @@ func (e *Watcher) verifyAndPublish( msg = &verifiedMsg } - e.msgChan <- msg //nolint:channelcheck // The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations + e.msgChan <- msg // Note on channel capacity: The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations suiMessagesConfirmed.Inc() if msg.IsReobservation {