From f2c2a22bdcc6f2488eb326c465cadbe2d3087082 Mon Sep 17 00:00:00 2001 From: Paul Rogers <129207811+paul1r@users.noreply.github.com> Date: Thu, 28 Nov 2024 08:54:04 -0500 Subject: [PATCH] chore: Preparation for incoming static code analysis CI check (#15164) Co-authored-by: Danny Cooper --- clients/cmd/docker-driver/driver.go | 2 +- clients/cmd/docker-driver/main.go | 2 +- clients/cmd/fluent-bit/dque.go | 2 +- clients/pkg/promtail/promtail.go | 6 ++- .../promtail/targets/kafka/authentication.go | 2 +- .../promtail/targets/testutils/testutils.go | 2 +- cmd/chunks-inspect/main.go | 2 +- cmd/loki-canary/main.go | 12 ++++- cmd/migrate/main.go | 2 +- integration/client/client.go | 2 +- integration/cluster/cluster.go | 6 +-- integration/cluster/ruler.go | 6 +-- operator/internal/manifests/storage/var.go | 18 +++---- operator/internal/manifests/var.go | 4 +- pkg/canary/comparator/comparator.go | 2 +- pkg/canary/writer/writer.go | 4 +- pkg/chunkenc/memchunk.go | 2 +- .../deletion/delete_requests_store.go | 2 +- pkg/compactor/retention/retention.go | 2 +- pkg/compactor/retention/util.go | 2 +- pkg/compactor/testutil.go | 4 +- pkg/ingester/checkpoint.go | 2 +- pkg/ingester/ingester.go | 4 +- pkg/kafka/partitionring/partition_ring.go | 3 +- pkg/loghttp/query.go | 2 +- pkg/logproto/compat.go | 6 +-- pkg/logql/log/pipeline.go | 4 +- pkg/logql/sketch/topk.go | 2 +- pkg/logql/test_utils.go | 2 +- pkg/lokifrontend/frontend/v2/frontend.go | 2 +- pkg/pattern/drain/drain.go | 4 +- pkg/pattern/ingester.go | 2 +- pkg/queue/tenant_queues.go | 2 +- pkg/ruler/base/mapper.go | 11 ++-- pkg/ruler/base/ruler.go | 2 +- pkg/storage/bloom/v1/archive.go | 51 +++++++++++++++++-- pkg/storage/bloom/v1/bloom_tester.go | 4 +- pkg/storage/chunk/cache/redis_client.go | 4 +- pkg/storage/chunk/chunk.go | 4 +- .../chunk/client/aws/s3_storage_client.go | 2 +- .../chunk/client/gcp/gcs_object_client.go | 2 +- pkg/storage/hack/main.go | 2 +- .../series/index/caching_index_client.go | 2 +- .../stores/series/index/table_manager.go | 2 +- .../indexshipper/boltdb/compactor/util.go | 2 +- .../indexshipper/downloads/testutil.go | 4 +- .../stores/shipper/indexshipper/shipper.go | 2 +- .../shipper/indexshipper/tsdb/builder.go | 2 +- .../shipper/indexshipper/tsdb/compactor.go | 2 +- .../shipper/indexshipper/tsdb/index/index.go | 2 +- .../stores/shipper/indexshipper/util/util.go | 4 +- pkg/tool/commands/rules.go | 2 +- pkg/util/conv.go | 4 +- pkg/util/marshal/query.go | 2 +- pkg/util/mempool/pool.go | 7 +-- pkg/util/shard.go | 2 +- pkg/util/ticker.go | 2 +- pkg/util/time.go | 4 +- pkg/util/unmarshal/unmarshal.go | 2 +- pkg/util/yolo.go | 2 +- .../lambda-promtail/promtail_client.go | 2 +- 61 files changed, 155 insertions(+), 99 deletions(-) diff --git a/clients/cmd/docker-driver/driver.go b/clients/cmd/docker-driver/driver.go index 8783b5b92505f..c5874d63eeba1 100644 --- a/clients/cmd/docker-driver/driver.go +++ b/clients/cmd/docker-driver/driver.go @@ -88,7 +88,7 @@ func (d *driver) StartLogging(file string, logCtx logger.Info) error { var jsonl logger.Logger if !noFile { - if err := os.MkdirAll(folder, 0755); err != nil { + if err := os.MkdirAll(folder, 0750); err != nil { return errors.Wrap(err, "error setting up logger dir") } diff --git a/clients/cmd/docker-driver/main.go b/clients/cmd/docker-driver/main.go index 06d90b81bda56..f83cff7407b3b 100644 --- a/clients/cmd/docker-driver/main.go +++ b/clients/cmd/docker-driver/main.go @@ -40,7 +40,7 @@ func main() { pprofPort := os.Getenv("PPROF_PORT") if pprofPort != "" { go func() { - err := http.ListenAndServe(fmt.Sprintf(":%s", pprofPort), nil) + err := http.ListenAndServe(fmt.Sprintf(":%s", pprofPort), nil) //#nosec G114 -- This is a debug feature that must be intentionally enabled and is not used in prod, DOS is not a concern. logger.Log("msg", "http server stopped", "err", err) }() } diff --git a/clients/cmd/fluent-bit/dque.go b/clients/cmd/fluent-bit/dque.go index 6e5746033254b..d1e9bc7d33809 100644 --- a/clients/cmd/fluent-bit/dque.go +++ b/clients/cmd/fluent-bit/dque.go @@ -59,7 +59,7 @@ func newDque(cfg *config, logger log.Logger, metrics *client.Metrics) (client.Cl logger: log.With(logger, "component", "queue", "name", cfg.bufferConfig.dqueConfig.queueName), } - err = os.MkdirAll(cfg.bufferConfig.dqueConfig.queueDir, 0644) + err = os.MkdirAll(cfg.bufferConfig.dqueConfig.queueDir, 0640) if err != nil { return nil, fmt.Errorf("cannot create queue directory: %s", err) } diff --git a/clients/pkg/promtail/promtail.go b/clients/pkg/promtail/promtail.go index 73e52f21703e1..86c27d55d7727 100644 --- a/clients/pkg/promtail/promtail.go +++ b/clients/pkg/promtail/promtail.go @@ -1,7 +1,6 @@ package promtail import ( - "crypto/md5" "errors" "fmt" "os" @@ -10,6 +9,8 @@ import ( "syscall" "time" + "golang.org/x/crypto/sha3" + "github.com/go-kit/log" "github.com/go-kit/log/level" "github.com/prometheus/client_golang/prometheus" @@ -130,7 +131,8 @@ func (p *Promtail) reloadConfig(cfg *config.Config) error { return errConfigNotChange } newConf := cfg.String() - level.Info(p.logger).Log("msg", "Reloading configuration file", "md5sum", fmt.Sprintf("%x", md5.Sum([]byte(newConf)))) + hash := sha3.Sum256([]byte(newConf)) + level.Info(p.logger).Log("msg", "Reloading configuration file", "sha3sum", fmt.Sprintf("%x", hash)) if p.targetManagers != nil { p.targetManagers.Stop() } diff --git a/clients/pkg/promtail/targets/kafka/authentication.go b/clients/pkg/promtail/targets/kafka/authentication.go index a58d55589c629..6e6c70bf417b5 100644 --- a/clients/pkg/promtail/targets/kafka/authentication.go +++ b/clients/pkg/promtail/targets/kafka/authentication.go @@ -13,7 +13,7 @@ import ( func createTLSConfig(cfg promconfig.TLSConfig) (*tls.Config, error) { tc := &tls.Config{ - InsecureSkipVerify: cfg.InsecureSkipVerify, + InsecureSkipVerify: cfg.InsecureSkipVerify, //#nosec G402 -- User has explicitly requested to disable TLS ServerName: cfg.ServerName, } // load ca cert diff --git a/clients/pkg/promtail/targets/testutils/testutils.go b/clients/pkg/promtail/targets/testutils/testutils.go index b88e87b323dd1..1eaa54396176f 100644 --- a/clients/pkg/promtail/targets/testutils/testutils.go +++ b/clients/pkg/promtail/targets/testutils/testutils.go @@ -16,7 +16,7 @@ var letters = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ") func RandName() string { b := make([]rune, 10) for i := range b { - b[i] = letters[randomGenerator.Intn(len(letters))] + b[i] = letters[randomGenerator.Intn(len(letters))] //#nosec G404 -- Generating random test data, fine. } return string(b) } diff --git a/cmd/chunks-inspect/main.go b/cmd/chunks-inspect/main.go index e5d25a20b713b..47fb04123783f 100644 --- a/cmd/chunks-inspect/main.go +++ b/cmd/chunks-inspect/main.go @@ -112,7 +112,7 @@ func printFile(filename string, blockDetails, printLines, storeBlocks bool) { } func writeBlockToFile(data []byte, blockIndex int, filename string) { - err := os.WriteFile(filename, data, 0644) + err := os.WriteFile(filename, data, 0640) // #nosec G306 -- this is fencing off the "other" permissions if err != nil { log.Println("Failed to store block", blockIndex, "to file", filename, "due to error:", err) } else { diff --git a/cmd/loki-canary/main.go b/cmd/loki-canary/main.go index 304bb1b1c3e91..b89a68ef77ac7 100644 --- a/cmd/loki-canary/main.go +++ b/cmd/loki-canary/main.go @@ -208,8 +208,16 @@ func main() { }) http.Handle("/metrics", promhttp.Handler()) go func() { - err := http.ListenAndServe(":"+strconv.Itoa(*port), nil) - if err != nil { + srv := &http.Server{ + Addr: ":" + strconv.Itoa(*port), + Handler: nil, // uses default mux from http.Handle calls above + ReadTimeout: 120 * time.Second, + WriteTimeout: 120 * time.Second, + IdleTimeout: 120 * time.Second, + ReadHeaderTimeout: 120 * time.Second, + } + err := srv.ListenAndServe() + if err != nil && err != http.ErrServerClosed { panic(err) } }() diff --git a/cmd/migrate/main.go b/cmd/migrate/main.go index 9b09a462538ed..6ff24256afac1 100644 --- a/cmd/migrate/main.go +++ b/cmd/migrate/main.go @@ -53,7 +53,7 @@ func main() { flag.Parse() go func() { - log.Println(http.ListenAndServe("localhost:8080", nil)) + log.Println(http.ListenAndServe("localhost:8080", nil)) //#nosec G114 -- This is only bound to localhost, not a plausible DOS vector. }() // Create a set of defaults diff --git a/integration/client/client.go b/integration/client/client.go index fb2a795f910fe..94c5f8e0b508e 100644 --- a/integration/client/client.go +++ b/integration/client/client.go @@ -237,7 +237,7 @@ func (c *Client) Get(path string) (*http.Response, error) { // Get all the metrics func (c *Client) Metrics() (string, error) { url := fmt.Sprintf("%s/metrics", c.baseURL) - res, err := http.Get(url) + res, err := http.Get(url) //#nosec G107 -- Intentionally taking user input from config if err != nil { return "", err } diff --git a/integration/cluster/cluster.go b/integration/cluster/cluster.go index 57bc182d0c8cb..2d1c92037d05a 100644 --- a/integration/cluster/cluster.go +++ b/integration/cluster/cluster.go @@ -176,7 +176,7 @@ func New(logLevel level.Value, opts ...func(*Cluster)) *Cluster { overridesFile := filepath.Join(sharedPath, "loki-overrides.yaml") - err = os.WriteFile(overridesFile, []byte(`overrides:`), 0o777) + err = os.WriteFile(overridesFile, []byte(`overrides:`), 0640) // #nosec G306 -- this is fencing off the "other" permissions if err != nil { panic(fmt.Errorf("error creating overrides file: %w", err)) } @@ -348,7 +348,7 @@ func (c *Component) writeConfig() error { return fmt.Errorf("error getting merged config: %w", err) } - if err := os.WriteFile(configFile.Name(), mergedConfig, 0o644); err != nil { + if err := os.WriteFile(configFile.Name(), mergedConfig, 0640); err != nil { // #nosec G306 -- this is fencing off the "other" permissions return fmt.Errorf("error writing config file: %w", err) } @@ -525,7 +525,7 @@ func (c *Component) SetTenantLimits(tenant string, limits validation.Limits) err return err } - return os.WriteFile(c.overridesFile, config, 0o777) + return os.WriteFile(c.overridesFile, config, 0640) // #nosec G306 -- this is fencing off the "other" permissions } func (c *Component) GetTenantLimits(tenant string) validation.Limits { diff --git a/integration/cluster/ruler.go b/integration/cluster/ruler.go index fd7b1462dd98a..59845fd50324a 100644 --- a/integration/cluster/ruler.go +++ b/integration/cluster/ruler.go @@ -33,17 +33,17 @@ func (c *Component) WithTenantRules(tenantFilesMap map[string]map[string]string) sharedPath := c.ClusterSharedPath() rulesPath := filepath.Join(sharedPath, "rules") - if err := os.Mkdir(rulesPath, 0755); err != nil { + if err := os.Mkdir(rulesPath, 0750); err != nil { return fmt.Errorf("error creating rules path: %w", err) } for tenant, files := range tenantFilesMap { for filename, file := range files { path := filepath.Join(rulesPath, tenant) - if err := os.Mkdir(path, 0755); err != nil { + if err := os.Mkdir(path, 0750); err != nil { return fmt.Errorf("error creating tenant %s rules path: %w", tenant, err) } - if err := os.WriteFile(filepath.Join(path, filename), []byte(strings.TrimSpace(file)), 0644); err != nil { + if err := os.WriteFile(filepath.Join(path, filename), []byte(strings.TrimSpace(file)), 0640); err != nil { // #nosec G306 -- this is fencing off the "other" permissions return fmt.Errorf("error creating rule file at path %s: %w", path, err) } } diff --git a/operator/internal/manifests/storage/var.go b/operator/internal/manifests/storage/var.go index 108d811412c3d..90def59358e5d 100644 --- a/operator/internal/manifests/storage/var.go +++ b/operator/internal/manifests/storage/var.go @@ -10,15 +10,15 @@ const ( // EnvAWSAccessKeyID is the environment variable to specify the AWS client id to access S3. EnvAWSAccessKeyID = "AWS_ACCESS_KEY_ID" // EnvAWSAccessKeySecret is the environment variable to specify the AWS client secret to access S3. - EnvAWSAccessKeySecret = "AWS_ACCESS_KEY_SECRET" + EnvAWSAccessKeySecret = "AWS_ACCESS_KEY_SECRET" //#nosec G101 -- False positive // EnvAWSSseKmsEncryptionContext is the environment variable to specify the AWS KMS encryption context when using type SSE-KMS. EnvAWSSseKmsEncryptionContext = "AWS_SSE_KMS_ENCRYPTION_CONTEXT" // EnvAWSRoleArn is the environment variable to specify the AWS role ARN secret for the federated identity workflow. EnvAWSRoleArn = "AWS_ROLE_ARN" // EnvAWSWebIdentityTokenFile is the environment variable to specify the path to the web identity token file used in the federated identity workflow. - EnvAWSWebIdentityTokenFile = "AWS_WEB_IDENTITY_TOKEN_FILE" + EnvAWSWebIdentityTokenFile = "AWS_WEB_IDENTITY_TOKEN_FILE" //#nosec G101 -- False positive // EnvAWSCredentialsFile is the environment variable to specify the path to the shared credentials file - EnvAWSCredentialsFile = "AWS_SHARED_CREDENTIALS_FILE" + EnvAWSCredentialsFile = "AWS_SHARED_CREDENTIALS_FILE" //#nosec G101 -- False positive // EnvAWSSdkLoadConfig is the environment that enabled the AWS SDK to enable the shared credentials file to be loaded EnvAWSSdkLoadConfig = "AWS_SDK_LOAD_CONFIG" // EnvAzureStorageAccountName is the environment variable to specify the Azure storage account name to access the container. @@ -34,7 +34,7 @@ const ( // EnvAzureFederatedTokenFile is the environment variable used to store the path to the Managed Identity token. EnvAzureFederatedTokenFile = "AZURE_FEDERATED_TOKEN_FILE" // EnvGoogleApplicationCredentials is the environment variable to specify path to key.json - EnvGoogleApplicationCredentials = "GOOGLE_APPLICATION_CREDENTIALS" + EnvGoogleApplicationCredentials = "GOOGLE_APPLICATION_CREDENTIALS" //#nosec G101 -- False positive // EnvSwiftPassword is the environment variable to specify the OpenStack Swift password. EnvSwiftPassword = "SWIFT_PASSWORD" // EnvSwiftUsername is the environment variable to specify the OpenStack Swift username. @@ -52,7 +52,7 @@ const ( // KeyAWSAccessKeyID is the secret data key for the AWS client id to access S3. KeyAWSAccessKeyID = "access_key_id" // KeyAWSAccessKeySecret is the secret data key for the AWS client secret to access S3. - KeyAWSAccessKeySecret = "access_key_secret" + KeyAWSAccessKeySecret = "access_key_secret" //#nosec G101 -- False positive // KeyAWSBucketNames is the secret data key for the AWS S3 bucket names. KeyAWSBucketNames = "bucketnames" // KeyAWSEndpoint is the secret data key for the AWS endpoint URL. @@ -131,16 +131,16 @@ const ( saTokenVolumeName = "bound-sa-token" saTokenExpiration int64 = 3600 - saTokenVolumeMountPath = "/var/run/secrets/storage/serviceaccount" + saTokenVolumeMountPath = "/var/run/secrets/storage/serviceaccount" //#nosec G101 -- False positive ServiceAccountTokenFilePath = saTokenVolumeMountPath + "/token" - secretDirectory = "/etc/storage/secrets" + secretDirectory = "/etc/storage/secrets" //#nosec G101 -- False positive storageTLSVolume = "storage-tls" caDirectory = "/etc/storage/ca" - tokenAuthConfigVolumeName = "token-auth-config" - tokenAuthConfigDirectory = "/etc/storage/token-auth" + tokenAuthConfigVolumeName = "token-auth-config" //#nosec G101 -- False positive + tokenAuthConfigDirectory = "/etc/storage/token-auth" //#nosec G101 -- False positive awsDefaultAudience = "sts.amazonaws.com" azureDefaultAudience = "api://AzureADTokenExchange" diff --git a/operator/internal/manifests/var.go b/operator/internal/manifests/var.go index 9e501ee72ae99..8813c07454621 100644 --- a/operator/internal/manifests/var.go +++ b/operator/internal/manifests/var.go @@ -68,7 +68,7 @@ const ( // PrometheusCAFile declares the path for prometheus CA file for service monitors. PrometheusCAFile string = "/etc/prometheus/configmaps/serving-certs-ca-bundle/service-ca.crt" // BearerTokenFile declares the path for bearer token file for service monitors. - BearerTokenFile string = "/var/run/secrets/kubernetes.io/serviceaccount/token" + BearerTokenFile string = "/var/run/secrets/kubernetes.io/serviceaccount/token" //#nosec G101 -- False positive // labelJobComponent is a ServiceMonitor.Spec.JobLabel. labelJobComponent string = "loki.grafana.com/component" @@ -80,7 +80,7 @@ const ( // AnnotationLokiObjectStoreHash stores the last SHA1 hash of the loki object storage credetials. AnnotationLokiObjectStoreHash string = "loki.grafana.com/object-store-hash" // AnnotationLokiTokenCCOAuthHash stores the SHA1 hash of the secret generated by the Cloud Credential Operator. - AnnotationLokiTokenCCOAuthHash string = "loki.grafana.com/token-cco-auth-hash" + AnnotationLokiTokenCCOAuthHash string = "loki.grafana.com/token-cco-auth-hash" //#nosec G101 -- False positive // LabelCompactorComponent is the label value for the compactor component LabelCompactorComponent string = "compactor" diff --git a/pkg/canary/comparator/comparator.go b/pkg/canary/comparator/comparator.go index 8d72fac6260f9..a141e2c0864f7 100644 --- a/pkg/canary/comparator/comparator.go +++ b/pkg/canary/comparator/comparator.go @@ -275,7 +275,7 @@ func (c *Comparator) run() { t := time.NewTicker(c.pruneInterval) // Use a random tick up to the interval for the first tick firstMt := true - randomGenerator := rand.New(rand.NewSource(time.Now().UnixNano())) + randomGenerator := rand.New(rand.NewSource(time.Now().UnixNano())) //#nosec G404 -- Random sampling for health testing purposes, does not require secure random. mt := time.NewTicker(time.Duration(randomGenerator.Int63n(c.metricTestInterval.Nanoseconds()))) sc := time.NewTicker(c.spotCheckQueryRate) ct := time.NewTicker(c.cacheTestInterval) diff --git a/pkg/canary/writer/writer.go b/pkg/canary/writer/writer.go index 032e7736ea782..32b23507dd718 100644 --- a/pkg/canary/writer/writer.go +++ b/pkg/canary/writer/writer.go @@ -82,8 +82,8 @@ func (w *Writer) run() { select { case <-t.C: t := time.Now() - if i := rand.Intn(100); i < w.outOfOrderPercentage { - n := rand.Intn(int(w.outOfOrderMax.Seconds()-w.outOfOrderMin.Seconds())) + int(w.outOfOrderMin.Seconds()) + if i := rand.Intn(100); i < w.outOfOrderPercentage { //#nosec G404 -- Random sampling for testing purposes, does not require secure random. + n := rand.Intn(int(w.outOfOrderMax.Seconds()-w.outOfOrderMin.Seconds())) + int(w.outOfOrderMin.Seconds()) //#nosec G404 -- Random sampling for testing purposes, does not require secure random. t = t.Add(-time.Duration(n) * time.Second) } ts := strconv.FormatInt(t.UnixNano(), 10) diff --git a/pkg/chunkenc/memchunk.go b/pkg/chunkenc/memchunk.go index 790210d3af8b2..ef816aca52153 100644 --- a/pkg/chunkenc/memchunk.go +++ b/pkg/chunkenc/memchunk.go @@ -1332,7 +1332,7 @@ func (hb *headBlock) SampleIterator(ctx context.Context, mint, maxt int64, extra } func unsafeGetBytes(s string) []byte { - return unsafe.Slice(unsafe.StringData(s), len(s)) + return unsafe.Slice(unsafe.StringData(s), len(s)) // #nosec G103 -- we know the string is not mutated } type bufferedIterator struct { diff --git a/pkg/compactor/deletion/delete_requests_store.go b/pkg/compactor/deletion/delete_requests_store.go index b7ddfe13a6182..e351a694b0f19 100644 --- a/pkg/compactor/deletion/delete_requests_store.go +++ b/pkg/compactor/deletion/delete_requests_store.go @@ -413,7 +413,7 @@ func splitUserIDAndRequestID(rangeValue string) (userID, requestID, seqID string // unsafeGetString is like yolostring but with a meaningful name func unsafeGetString(buf []byte) string { - return *((*string)(unsafe.Pointer(&buf))) + return *((*string)(unsafe.Pointer(&buf))) // #nosec G103 -- we know the string is not mutated } func generateCacheGenNumber() []byte { diff --git a/pkg/compactor/retention/retention.go b/pkg/compactor/retention/retention.go index 96eafcc2a7d58..1e7be7b81d27c 100644 --- a/pkg/compactor/retention/retention.go +++ b/pkg/compactor/retention/retention.go @@ -484,7 +484,7 @@ func CopyMarkers(src string, dst string) error { return fmt.Errorf("read marker file: %w", err) } - if err := os.WriteFile(filepath.Join(targetDir, marker.Name()), data, 0o666); err != nil { + if err := os.WriteFile(filepath.Join(targetDir, marker.Name()), data, 0640); err != nil { // #nosec G306 -- this is fencing off the "other" permissions return fmt.Errorf("write marker file: %w", err) } } diff --git a/pkg/compactor/retention/util.go b/pkg/compactor/retention/util.go index 285fdc1fd553a..535aa8370dae6 100644 --- a/pkg/compactor/retention/util.go +++ b/pkg/compactor/retention/util.go @@ -13,7 +13,7 @@ import ( // unsafeGetString is like yolostring but with a meaningful name func unsafeGetString(buf []byte) string { - return *((*string)(unsafe.Pointer(&buf))) + return *((*string)(unsafe.Pointer(&buf))) // #nosec G103 -- we know the string is not mutated } func copyFile(src, dst string) (int64, error) { diff --git a/pkg/compactor/testutil.go b/pkg/compactor/testutil.go index 4ebba27f64bfa..feba141ba514d 100644 --- a/pkg/compactor/testutil.go +++ b/pkg/compactor/testutil.go @@ -81,7 +81,7 @@ func SetupTable(t *testing.T, path string, commonDBsConfig IndexesConfig, perUse idx := 0 for filename, content := range commonIndexes { filePath := filepath.Join(path, strings.TrimSuffix(filename, ".gz")) - require.NoError(t, os.WriteFile(filePath, []byte(content), 0777)) + require.NoError(t, os.WriteFile(filePath, []byte(content), 0640)) // #nosec G306 -- this is fencing off the "other" permissions if strings.HasSuffix(filename, ".gz") { compressFile(t, filePath) } @@ -92,7 +92,7 @@ func SetupTable(t *testing.T, path string, commonDBsConfig IndexesConfig, perUse require.NoError(t, util.EnsureDirectory(filepath.Join(path, userID))) for filename, content := range files { filePath := filepath.Join(path, userID, strings.TrimSuffix(filename, ".gz")) - require.NoError(t, os.WriteFile(filePath, []byte(content), 0777)) + require.NoError(t, os.WriteFile(filePath, []byte(content), 0640)) // #nosec G306 -- this is fencing off the "other" permissions if strings.HasSuffix(filename, ".gz") { compressFile(t, filePath) } diff --git a/pkg/ingester/checkpoint.go b/pkg/ingester/checkpoint.go index 1f6dcf8eefeee..8fb2e055a7469 100644 --- a/pkg/ingester/checkpoint.go +++ b/pkg/ingester/checkpoint.go @@ -344,7 +344,7 @@ func (w *WALCheckpointWriter) Advance() (bool, error) { } } - if err := os.MkdirAll(checkpointDirTemp, 0777); err != nil { + if err := os.MkdirAll(checkpointDirTemp, 0750); err != nil { return false, fmt.Errorf("create checkpoint dir: %w", err) } diff --git a/pkg/ingester/ingester.go b/pkg/ingester/ingester.go index 958e01b8d9907..5659c3b056a39 100644 --- a/pkg/ingester/ingester.go +++ b/pkg/ingester/ingester.go @@ -335,7 +335,7 @@ func New(cfg Config, clientConfig client.Config, store Store, limits Limits, con i.replayController = newReplayController(metrics, cfg.WAL, &replayFlusher{i}) if cfg.WAL.Enabled { - if err := os.MkdirAll(cfg.WAL.Dir, os.ModePerm); err != nil { + if err := os.MkdirAll(cfg.WAL.Dir, 0750); err != nil { // Best effort try to make path absolute for easier debugging. path, _ := filepath.Abs(cfg.WAL.Dir) if path == "" { @@ -759,7 +759,7 @@ func (i *Ingester) loop() { // flush at the same time. Flushing at the same time can cause concurrently // writing the same chunk to object storage, which in AWS S3 leads to being // rate limited. - jitter := time.Duration(rand.Int63n(int64(float64(i.cfg.FlushCheckPeriod.Nanoseconds()) * 0.8))) + jitter := time.Duration(rand.Int63n(int64(float64(i.cfg.FlushCheckPeriod.Nanoseconds()) * 0.8))) //#nosec G404 -- Jitter does not require a CSPRNG. initialDelay := time.NewTimer(jitter) defer initialDelay.Stop() diff --git a/pkg/kafka/partitionring/partition_ring.go b/pkg/kafka/partitionring/partition_ring.go index 15dad003dd931..542a4aee80a41 100644 --- a/pkg/kafka/partitionring/partition_ring.go +++ b/pkg/kafka/partitionring/partition_ring.go @@ -62,8 +62,9 @@ func ExtractIngesterPartitionID(ingesterID string) (int32, error) { if len(match) == 0 { return 0, fmt.Errorf("ingester ID %s doesn't match regular expression %q", ingesterID, ingesterIDRegexp.String()) } + // Parse the ingester sequence number. - ingesterSeq, err := strconv.Atoi(match[1]) + ingesterSeq, err := strconv.ParseInt(match[1], 10, 32) if err != nil { return 0, fmt.Errorf("no ingester sequence number in ingester ID %s", ingesterID) } diff --git a/pkg/loghttp/query.go b/pkg/loghttp/query.go index a2bce462aab80..86644407a8c15 100644 --- a/pkg/loghttp/query.go +++ b/pkg/loghttp/query.go @@ -266,7 +266,7 @@ func (s Streams) ToProto() []logproto.Stream { } result := make([]logproto.Stream, 0, len(s)) for _, s := range s { - entries := *(*[]logproto.Entry)(unsafe.Pointer(&s.Entries)) + entries := *(*[]logproto.Entry)(unsafe.Pointer(&s.Entries)) // #nosec G103 -- we know the string is not mutated result = append(result, logproto.Stream{ Labels: s.Labels.String(), Entries: entries, diff --git a/pkg/logproto/compat.go b/pkg/logproto/compat.go index 69ffe4dece3ca..b0110c2250b5e 100644 --- a/pkg/logproto/compat.go +++ b/pkg/logproto/compat.go @@ -51,14 +51,14 @@ func ToWriteRequest(lbls []labels.Labels, samples []LegacySample, metadata []*Me // Note: while resulting labels.Labels is supposedly sorted, this function // doesn't enforce that. If input is not sorted, output will be wrong. func FromLabelAdaptersToLabels(ls []LabelAdapter) labels.Labels { - return *(*labels.Labels)(unsafe.Pointer(&ls)) + return *(*labels.Labels)(unsafe.Pointer(&ls)) // #nosec G103 -- we know the string is not mutated } // FromLabelsToLabelAdapters casts labels.Labels to []LabelAdapter. // It uses unsafe, but as LabelAdapter == labels.Label this should be safe. // This allows us to use labels.Labels directly in protos. func FromLabelsToLabelAdapters(ls labels.Labels) []LabelAdapter { - return *(*[]LabelAdapter)(unsafe.Pointer(&ls)) + return *(*[]LabelAdapter)(unsafe.Pointer(&ls)) // #nosec G103 -- we know the string is not mutated } // FromLabelAdaptersToMetric converts []LabelAdapter to a model.Metric. @@ -155,7 +155,7 @@ func SampleJsoniterDecode(ptr unsafe.Pointer, iter *jsoniter.Iterator) { } bs := iter.ReadStringAsSlice() - ss := *(*string)(unsafe.Pointer(&bs)) + ss := *(*string)(unsafe.Pointer(&bs)) // #nosec G103 -- we know the string is not mutated v, err := strconv.ParseFloat(ss, 64) if err != nil { iter.ReportError("logproto.LegacySample", err.Error()) diff --git a/pkg/logql/log/pipeline.go b/pkg/logql/log/pipeline.go index 181947fc07435..fe4828f682a37 100644 --- a/pkg/logql/log/pipeline.go +++ b/pkg/logql/log/pipeline.go @@ -380,9 +380,9 @@ func ReduceStages(stages []Stage) Stage { } func unsafeGetBytes(s string) []byte { - return unsafe.Slice(unsafe.StringData(s), len(s)) + return unsafe.Slice(unsafe.StringData(s), len(s)) // #nosec G103 -- we know the string is not mutated } func unsafeGetString(buf []byte) string { - return *((*string)(unsafe.Pointer(&buf))) + return *((*string)(unsafe.Pointer(&buf))) // #nosec G103 -- we know the string is not mutated } diff --git a/pkg/logql/sketch/topk.go b/pkg/logql/sketch/topk.go index 23280154daed8..86b01e4c56638 100644 --- a/pkg/logql/sketch/topk.go +++ b/pkg/logql/sketch/topk.go @@ -210,7 +210,7 @@ func (t *Topk) updateBF(removed, added string) { } func unsafeGetBytes(s string) []byte { - return unsafe.Slice(unsafe.StringData(s), len(s)) + return unsafe.Slice(unsafe.StringData(s), len(s)) // #nosec G103 -- we know the string is not mutated } // Observe is our sketch event observation function, which is a bit more complex than the original count min sketch + heap TopK diff --git a/pkg/logql/test_utils.go b/pkg/logql/test_utils.go index e7f003327ea01..703842b23f2eb 100644 --- a/pkg/logql/test_utils.go +++ b/pkg/logql/test_utils.go @@ -257,7 +257,7 @@ func (m MockDownstreamer) Downstream(ctx context.Context, queries []DownstreamQu // create nStreams of nEntries with labelNames each where each label value // with the exception of the "index" label is modulo'd into a shard func randomStreams(nStreams, nEntries, nShards int, labelNames []string, valueField bool) (streams []logproto.Stream) { - r := rand.New(rand.NewSource(42)) + r := rand.New(rand.NewSource(42)) //#nosec G404 -- Generation of test data only, no need for a cryptographic PRNG for i := 0; i < nStreams; i++ { // labels stream := logproto.Stream{} diff --git a/pkg/lokifrontend/frontend/v2/frontend.go b/pkg/lokifrontend/frontend/v2/frontend.go index dae27ec94682c..7789b7d8f3710 100644 --- a/pkg/lokifrontend/frontend/v2/frontend.go +++ b/pkg/lokifrontend/frontend/v2/frontend.go @@ -154,7 +154,7 @@ func NewFrontend(cfg Config, ring ring.ReadRing, log log.Logger, reg prometheus. // Randomize to avoid getting responses from queries sent before restart, which could lead to mixing results // between different queries. Note that frontend verifies the user, so it cannot leak results between tenants. // This isn't perfect, but better than nothing. - f.lastQueryID.Store(rand.Uint64()) + f.lastQueryID.Store(rand.Uint64()) //#nosec G404 -- See above comment, this can't leak data or otherwise result in a vuln, simply very rarely cause confusing behavior. A CSPRNG would not help. promauto.With(reg).NewGaugeFunc(prometheus.GaugeOpts{ Namespace: metricsNamespace, diff --git a/pkg/pattern/drain/drain.go b/pkg/pattern/drain/drain.go index 9308ab99a511d..2082610a7a401 100644 --- a/pkg/pattern/drain/drain.go +++ b/pkg/pattern/drain/drain.go @@ -548,9 +548,9 @@ func (d *Drain) createTemplate(tokens, matchClusterTokens []string) []string { } func unsafeString(s []byte) string { - return unsafe.String(unsafe.SliceData(s), len(s)) + return unsafe.String(unsafe.SliceData(s), len(s)) // #nosec G103 -- we know the string is not mutated } func unsafeBytes(s string) []byte { - return unsafe.Slice(unsafe.StringData(s), len(s)) + return unsafe.Slice(unsafe.StringData(s), len(s)) // #nosec G103 -- we know the string is not mutated } diff --git a/pkg/pattern/ingester.go b/pkg/pattern/ingester.go index c535ab9ebc295..0e2e1ad1432a0 100644 --- a/pkg/pattern/ingester.go +++ b/pkg/pattern/ingester.go @@ -275,7 +275,7 @@ func (i *Ingester) loop() { // flush at the same time. Flushing at the same time can cause concurrently // writing the same chunk to object storage, which in AWS S3 leads to being // rate limited. - jitter := time.Duration(rand.Int63n(int64(float64(i.cfg.FlushCheckPeriod.Nanoseconds()) * 0.8))) + jitter := time.Duration(rand.Int63n(int64(float64(i.cfg.FlushCheckPeriod.Nanoseconds()) * 0.8))) //#nosec G404 -- Jitter does not require a CSPRNG initialDelay := time.NewTimer(jitter) defer initialDelay.Stop() diff --git a/pkg/queue/tenant_queues.go b/pkg/queue/tenant_queues.go index 46f9f7adccd43..c4c38904bb8b1 100644 --- a/pkg/queue/tenant_queues.go +++ b/pkg/queue/tenant_queues.go @@ -336,7 +336,7 @@ func shuffleConsumersForTenants(userSeed int64, consumersToSelect int, allSorted } result := make(map[string]struct{}, consumersToSelect) - rnd := rand.New(rand.NewSource(userSeed)) + rnd := rand.New(rand.NewSource(userSeed)) //#nosec G404 -- Load spreading does not require CSPRNG scratchpad = scratchpad[:0] scratchpad = append(scratchpad, allSortedConsumers...) diff --git a/pkg/ruler/base/mapper.go b/pkg/ruler/base/mapper.go index 9da3e43aebad5..38a3e229a6a86 100644 --- a/pkg/ruler/base/mapper.go +++ b/pkg/ruler/base/mapper.go @@ -1,7 +1,6 @@ package base import ( - "crypto/md5" "net/url" "os" "path/filepath" @@ -11,6 +10,7 @@ import ( "github.com/go-kit/log/level" "github.com/prometheus/prometheus/model/rulefmt" "github.com/spf13/afero" + "golang.org/x/crypto/sha3" "gopkg.in/yaml.v3" ) @@ -148,11 +148,14 @@ func (m *mapper) writeRuleGroupsIfNewer(groups []rulefmt.RuleGroup, filename str if err != nil { return false, err } - newHash := md5.New() - currentHash := md5.New() + newHash := sha3.New256() + currentHash := sha3.New256() + + newHash.Write(d) + currentHash.Write(current) // bailout if there is no update - if string(currentHash.Sum(current)) == string(newHash.Sum(d)) { + if string(currentHash.Sum(nil)) == string(newHash.Sum(nil)) { return false, nil } } diff --git a/pkg/ruler/base/ruler.go b/pkg/ruler/base/ruler.go index 2e6c74c759dfb..adb1a7c136def 100644 --- a/pkg/ruler/base/ruler.go +++ b/pkg/ruler/base/ruler.go @@ -783,7 +783,7 @@ func cloneGroupWithRule(g *rulespb.RuleGroupDesc, r *rulespb.RuleDesc) *rulespb. } // the delimiter is prefixed with ";" since that is what Prometheus uses for its group key -const ruleTokenDelimiter = ";rule-shard-token" +const ruleTokenDelimiter = ";rule-shard-token" //#nosec G101 -- False positive // AddRuleTokenToGroupName adds a rule shard token to a given group's name to make it unique. // Only relevant when using "by-rule" sharding strategy. diff --git a/pkg/storage/bloom/v1/archive.go b/pkg/storage/bloom/v1/archive.go index a7b7232f230d1..0e854efcef5b8 100644 --- a/pkg/storage/bloom/v1/archive.go +++ b/pkg/storage/bloom/v1/archive.go @@ -5,6 +5,7 @@ import ( "io" "os" "path/filepath" + "strings" "github.com/pkg/errors" @@ -73,8 +74,17 @@ func UnTarCompress(enc compression.Codec, dst string, r io.Reader) error { } func UnTar(dst string, r io.Reader) error { + // Add safety checks for destination + dst = filepath.Clean(dst) + if !filepath.IsAbs(dst) { + return errors.New("destination path must be absolute") + } tarballer := tar.NewReader(r) + // Track total size to prevent decompression bombs + var totalSize int64 + const maxSize = 20 << 30 // 20GB limit + for { header, err := tarballer.Next() if err == io.EOF { @@ -84,7 +94,17 @@ func UnTar(dst string, r io.Reader) error { return errors.Wrap(err, "error reading tarball header") } + // Check for path traversal target := filepath.Join(dst, header.Name) + if !isWithinDir(target, dst) { + return errors.Errorf("invalid path %q: path traversal attempt", header.Name) + } + + // Update and check total size + totalSize += header.Size + if totalSize > maxSize { + return errors.New("decompression bomb: extracted content too large") + } // check the file type switch header.Typeflag { @@ -92,7 +112,7 @@ func UnTar(dst string, r io.Reader) error { // if its a dir and it doesn't exist create it case tar.TypeDir: if _, err := os.Stat(target); err != nil { - if err := os.MkdirAll(target, 0755); err != nil { + if err := os.MkdirAll(target, 0750); err != nil { return err } } @@ -104,13 +124,21 @@ func UnTar(dst string, r io.Reader) error { return errors.Wrapf(err, "error creating file %s", target) } - // copy over contents - if _, err := io.Copy(f, tarballer); err != nil { + // Use LimitReader to prevent reading more than declared size + limited := io.LimitReader(tarballer, header.Size) + written, err := io.Copy(f, limited) + if err != nil { + f.Close() return errors.Wrapf(err, "error copying contents of file %s", target) } - // manually close here after each file operation; defering would cause each file close - // to wait until all operations have completed. + // Verify the actual bytes written match the header size + if written != header.Size { + f.Close() + return errors.Errorf("size mismatch for %s: header claimed %d bytes but got %d bytes", + header.Name, header.Size, written) + } + if err := f.Close(); err != nil { return errors.Wrapf(err, "error closing file %s", target) } @@ -120,3 +148,16 @@ func UnTar(dst string, r io.Reader) error { return nil } + +// Helper function to check for path traversal +func isWithinDir(target, dir string) bool { + targetPath := filepath.Clean(target) + dirPath := filepath.Clean(dir) + + relative, err := filepath.Rel(dirPath, targetPath) + if err != nil { + return false + } + + return !strings.HasPrefix(relative, ".."+string(filepath.Separator)) +} diff --git a/pkg/storage/bloom/v1/bloom_tester.go b/pkg/storage/bloom/v1/bloom_tester.go index 1682556bf7d60..ed3939e38a76f 100644 --- a/pkg/storage/bloom/v1/bloom_tester.go +++ b/pkg/storage/bloom/v1/bloom_tester.go @@ -163,7 +163,7 @@ func (kvm keyValueMatcherTest) Matches(series labels.Labels, bloom filter.Checke var ( combined = fmt.Sprintf("%s=%s", kvm.matcher.Key, kvm.matcher.Value) - rawCombined = unsafe.Slice(unsafe.StringData(combined), len(combined)) + rawCombined = unsafe.Slice(unsafe.StringData(combined), len(combined)) // #nosec G103 -- we know the string is not mutated ) return kvm.match(series, bloom, rawCombined) @@ -199,7 +199,7 @@ func (kvm keyValueMatcherTest) match(series labels.Labels, bloom filter.Checker, // appendToBuf is the equivalent of append(buf[:prefixLen], str). len(buf) must // be greater than or equal to prefixLen+len(str) to avoid allocations. func appendToBuf(buf []byte, prefixLen int, str string) []byte { - rawString := unsafe.Slice(unsafe.StringData(str), len(str)) + rawString := unsafe.Slice(unsafe.StringData(str), len(str)) // #nosec G103 -- we know the string is not mutated return append(buf[:prefixLen], rawString...) } diff --git a/pkg/storage/chunk/cache/redis_client.go b/pkg/storage/chunk/cache/redis_client.go index 55731c7b11a4a..a43441d4a12bb 100644 --- a/pkg/storage/chunk/cache/redis_client.go +++ b/pkg/storage/chunk/cache/redis_client.go @@ -74,7 +74,7 @@ func NewRedisClient(cfg *RedisConfig) (*RedisClient, error) { RouteRandomly: cfg.RouteRandomly, } if cfg.EnableTLS { - opt.TLSConfig = &tls.Config{InsecureSkipVerify: cfg.InsecureSkipVerify} + opt.TLSConfig = &tls.Config{InsecureSkipVerify: cfg.InsecureSkipVerify} //#nosec G402 -- User has explicitly requested to disable TLS } return &RedisClient{ expiration: cfg.Expiration, @@ -208,7 +208,7 @@ func (c *RedisClient) Close() error { // StringToBytes converts string to byte slice. (copied from vendor/github.com/go-redis/redis/v8/internal/util/unsafe.go) func StringToBytes(s string) []byte { - return *(*[]byte)(unsafe.Pointer( + return *(*[]byte)(unsafe.Pointer( // #nosec G103 -- we know the string is not mutated &struct { string Cap int diff --git a/pkg/storage/chunk/chunk.go b/pkg/storage/chunk/chunk.go index aadfe6ea937b2..a4cb63a442cb5 100644 --- a/pkg/storage/chunk/chunk.go +++ b/pkg/storage/chunk/chunk.go @@ -214,11 +214,11 @@ func readOneHexPart(hex []byte) (part []byte, i int) { } func unsafeGetBytes(s string) []byte { - return unsafe.Slice(unsafe.StringData(s), len(s)) + return unsafe.Slice(unsafe.StringData(s), len(s)) // #nosec G103 -- we know the string is not mutated } func unsafeGetString(buf []byte) string { - return *((*string)(unsafe.Pointer(&buf))) + return *((*string)(unsafe.Pointer(&buf))) // #nosec G103 -- we know the string is not mutated } var writerPool = sync.Pool{ diff --git a/pkg/storage/chunk/client/aws/s3_storage_client.go b/pkg/storage/chunk/client/aws/s3_storage_client.go index 65817f38c9d9f..e7891e4963fc0 100644 --- a/pkg/storage/chunk/client/aws/s3_storage_client.go +++ b/pkg/storage/chunk/client/aws/s3_storage_client.go @@ -230,7 +230,7 @@ func buildS3Client(cfg S3Config, hedgingCfg hedging.Config, hedging bool) (*s3.S } tlsConfig := &tls.Config{ - InsecureSkipVerify: cfg.HTTPConfig.InsecureSkipVerify, + InsecureSkipVerify: cfg.HTTPConfig.InsecureSkipVerify, //#nosec G402 -- User has explicitly requested to disable TLS } if cfg.HTTPConfig.CAFile != "" { diff --git a/pkg/storage/chunk/client/gcp/gcs_object_client.go b/pkg/storage/chunk/client/gcp/gcs_object_client.go index 1d44659b3f3cc..95b55c3319929 100644 --- a/pkg/storage/chunk/client/gcp/gcs_object_client.go +++ b/pkg/storage/chunk/client/gcp/gcs_object_client.go @@ -357,7 +357,7 @@ func gcsTransport(ctx context.Context, scope string, insecure bool, http2 bool, } transportOptions := []option.ClientOption{option.WithScopes(scope)} if insecure { - customTransport.TLSClientConfig = &tls.Config{InsecureSkipVerify: true} + customTransport.TLSClientConfig = &tls.Config{InsecureSkipVerify: true} //#nosec G402 -- User has explicitly requested to disable TLS transportOptions = append(transportOptions, option.WithoutAuthentication()) } if serviceAccount.String() != "" { diff --git a/pkg/storage/hack/main.go b/pkg/storage/hack/main.go index 4e6c348ceb3e0..a9e57affa9940 100644 --- a/pkg/storage/hack/main.go +++ b/pkg/storage/hack/main.go @@ -143,7 +143,7 @@ const charset = "abcdefghijklmnopqrstuvwxyz" + func randStringWithCharset(length int, charset string) string { b := make([]byte, length) for i := range b { - b[i] = charset[rand.Intn(len(charset)-1)] + b[i] = charset[rand.Intn(len(charset)-1)] //#nosec G404 -- Generation of test data does not require CSPRNG, this is not meant to be secret. } return string(b) } diff --git a/pkg/storage/stores/series/index/caching_index_client.go b/pkg/storage/stores/series/index/caching_index_client.go index 40181ba794c71..d0945003f4c92 100644 --- a/pkg/storage/stores/series/index/caching_index_client.go +++ b/pkg/storage/stores/series/index/caching_index_client.go @@ -259,7 +259,7 @@ func (s *cachingIndexClient) doQueries(ctx context.Context, queries []Query, cal } func yoloString(buf []byte) string { - return *((*string)(unsafe.Pointer(&buf))) + return *((*string)(unsafe.Pointer(&buf))) // #nosec G103 -- we know the string is not mutated } // Iterator implements chunk.ReadBatch. diff --git a/pkg/storage/stores/series/index/table_manager.go b/pkg/storage/stores/series/index/table_manager.go index 414e08f494c89..820c880c1e00a 100644 --- a/pkg/storage/stores/series/index/table_manager.go +++ b/pkg/storage/stores/series/index/table_manager.go @@ -228,7 +228,7 @@ func (m *TableManager) loop(ctx context.Context) error { // Sleep for a bit to spread the sync load across different times if the tablemanagers are all started at once. select { - case <-time.After(time.Duration(rand.Int63n(int64(m.cfg.PollInterval)))): + case <-time.After(time.Duration(rand.Int63n(int64(m.cfg.PollInterval)))): //#nosec G404 -- This is also just essentially jitter, no need for CSPRNG. case <-ctx.Done(): return nil } diff --git a/pkg/storage/stores/shipper/indexshipper/boltdb/compactor/util.go b/pkg/storage/stores/shipper/indexshipper/boltdb/compactor/util.go index 04948c38a17c1..4fedf6f0fffe2 100644 --- a/pkg/storage/stores/shipper/indexshipper/boltdb/compactor/util.go +++ b/pkg/storage/stores/shipper/indexshipper/boltdb/compactor/util.go @@ -19,7 +19,7 @@ import ( // unsafeGetString is like yolostring but with a meaningful name func unsafeGetString(buf []byte) string { - return *((*string)(unsafe.Pointer(&buf))) + return *((*string)(unsafe.Pointer(&buf))) // #nosec G103 -- we know the string is not mutated } func createChunk(t testing.TB, chunkFormat byte, headBlockFmt chunkenc.HeadBlockFmt, userID string, lbs labels.Labels, from model.Time, through model.Time) chunk.Chunk { diff --git a/pkg/storage/stores/shipper/indexshipper/downloads/testutil.go b/pkg/storage/stores/shipper/indexshipper/downloads/testutil.go index 4f30762bdd244..e942782e546cd 100644 --- a/pkg/storage/stores/shipper/indexshipper/downloads/testutil.go +++ b/pkg/storage/stores/shipper/indexshipper/downloads/testutil.go @@ -33,13 +33,13 @@ func (m *mockIndex) Reader() (io.ReadSeeker, error) { } func setupIndexesAtPath(t *testing.T, userID, path string, start, end int) []string { - require.NoError(t, os.MkdirAll(path, 0755)) + require.NoError(t, os.MkdirAll(path, 0750)) var testIndexes []string for ; start < end; start++ { fileName := buildIndexFilename(userID, start) indexPath := filepath.Join(path, fileName) - require.NoError(t, os.WriteFile(indexPath, []byte(fileName), 0755)) + require.NoError(t, os.WriteFile(indexPath, []byte(fileName), 0640)) // #nosec G306 -- this is fencing off the "other" permissions testIndexes = append(testIndexes, indexPath) } diff --git a/pkg/storage/stores/shipper/indexshipper/shipper.go b/pkg/storage/stores/shipper/indexshipper/shipper.go index 2917b1fc7974f..5eaeaaa08d598 100644 --- a/pkg/storage/stores/shipper/indexshipper/shipper.go +++ b/pkg/storage/stores/shipper/indexshipper/shipper.go @@ -112,7 +112,7 @@ func (cfg *Config) GetUniqueUploaderName() (string, error) { if !os.IsNotExist(err) { return "", err } - if err := os.WriteFile(uploaderFilePath, []byte(uploader), 0o666); err != nil { + if err := os.WriteFile(uploaderFilePath, []byte(uploader), 0640); err != nil { // #nosec G306 -- this is fencing off the "other" permissions return "", err } } else { diff --git a/pkg/storage/stores/shipper/indexshipper/tsdb/builder.go b/pkg/storage/stores/shipper/indexshipper/tsdb/builder.go index a86ac392d259e..815888c14586f 100644 --- a/pkg/storage/stores/shipper/indexshipper/tsdb/builder.go +++ b/pkg/storage/stores/shipper/indexshipper/tsdb/builder.go @@ -108,7 +108,7 @@ func (b *Builder) Build( } // First write tenant/index-bounds-random.staging - rng := rand.Int63() + rng := rand.Int63() //#nosec G404 -- just generating a random filename in a slightly unidiomatic way. Collision resistance is not a concern. name := fmt.Sprintf("%s-%x.staging", index.IndexFilename, rng) tmpPath := filepath.Join(scratchDir, name) diff --git a/pkg/storage/stores/shipper/indexshipper/tsdb/compactor.go b/pkg/storage/stores/shipper/indexshipper/tsdb/compactor.go index df8ea85465142..06028ce1d63d2 100644 --- a/pkg/storage/stores/shipper/indexshipper/tsdb/compactor.go +++ b/pkg/storage/stores/shipper/indexshipper/tsdb/compactor.go @@ -409,5 +409,5 @@ func (c *compactedIndex) ToIndexFile() (shipperindex.Index, error) { } func getUnsafeBytes(s string) []byte { - return *((*[]byte)(unsafe.Pointer(&s))) + return *((*[]byte)(unsafe.Pointer(&s))) // #nosec G103 -- we know the string is not mutated } diff --git a/pkg/storage/stores/shipper/indexshipper/tsdb/index/index.go b/pkg/storage/stores/shipper/indexshipper/tsdb/index/index.go index 0766bd058fdf4..0e10f8648a375 100644 --- a/pkg/storage/stores/shipper/indexshipper/tsdb/index/index.go +++ b/pkg/storage/stores/shipper/indexshipper/tsdb/index/index.go @@ -2520,7 +2520,7 @@ func readChunkMetaWithForcedMintime(d *encoding.Decbuf, mint int64, chunkMeta *C } func yoloString(b []byte) string { - return *((*string)(unsafe.Pointer(&b))) + return *((*string)(unsafe.Pointer(&b))) // #nosec G103 -- we know the string is not mutated } func overlap(from, through, chkFrom, chkThrough int64) bool { diff --git a/pkg/storage/stores/shipper/indexshipper/util/util.go b/pkg/storage/stores/shipper/indexshipper/util/util.go index f47cea40d6d7d..8c74342c7e9c0 100644 --- a/pkg/storage/stores/shipper/indexshipper/util/util.go +++ b/pkg/storage/stores/shipper/indexshipper/util/util.go @@ -82,11 +82,11 @@ func safeOpenBoltDbFile(path string, ret chan *result) { // } func GetUnsafeBytes(s string) []byte { - return *((*[]byte)(unsafe.Pointer(&s))) + return *((*[]byte)(unsafe.Pointer(&s))) // #nosec G103 -- we know the string is not mutated } func GetUnsafeString(buf []byte) string { - return *((*string)(unsafe.Pointer(&buf))) + return *((*string)(unsafe.Pointer(&buf))) // #nosec G103 -- we know the string is not mutated } func logPanic(p interface{}) { diff --git a/pkg/tool/commands/rules.go b/pkg/tool/commands/rules.go index c030e9c24dfc5..b91da0f324abe 100644 --- a/pkg/tool/commands/rules.go +++ b/pkg/tool/commands/rules.go @@ -772,7 +772,7 @@ func save(nss map[string]rules.RuleNamespace, i bool) error { filepath = filepath + ".result" } - if err := os.WriteFile(filepath, payload, 0644); err != nil { + if err := os.WriteFile(filepath, payload, 0640); err != nil { // #nosec G306 -- this is fencing off the "other" permissions return err } } diff --git a/pkg/util/conv.go b/pkg/util/conv.go index e3ff145676e6e..dec752291fbca 100644 --- a/pkg/util/conv.go +++ b/pkg/util/conv.go @@ -13,7 +13,7 @@ func ModelLabelSetToMap(m model.LabelSet) map[string]string { if len(m) == 0 { return map[string]string{} } - return *(*map[string]string)(unsafe.Pointer(&m)) + return *(*map[string]string)(unsafe.Pointer(&m)) // #nosec G103 -- we know the string is not mutated } // MapToModelLabelSet converts a map into a model.LabelSet @@ -21,7 +21,7 @@ func MapToModelLabelSet(m map[string]string) model.LabelSet { if len(m) == 0 { return model.LabelSet{} } - return *(*map[model.LabelName]model.LabelValue)(unsafe.Pointer(&m)) + return *(*map[model.LabelName]model.LabelValue)(unsafe.Pointer(&m)) // #nosec G103 -- we know the string is not mutated } // RoundToMilliseconds returns milliseconds precision time from nanoseconds. diff --git a/pkg/util/marshal/query.go b/pkg/util/marshal/query.go index cbf3b40d94856..901a57daeca51 100644 --- a/pkg/util/marshal/query.go +++ b/pkg/util/marshal/query.go @@ -113,7 +113,7 @@ func NewStream(s logproto.Stream) (loghttp.Stream, error) { // Avoid a nil entries slice to be consistent with the decoding entries := []loghttp.Entry{} if len(s.Entries) > 0 { - entries = *(*[]loghttp.Entry)(unsafe.Pointer(&s.Entries)) + entries = *(*[]loghttp.Entry)(unsafe.Pointer(&s.Entries)) //#nosec G103 -- Just preventing an allocation, safe. Entry types are the same. } ret := loghttp.Stream{ diff --git a/pkg/util/mempool/pool.go b/pkg/util/mempool/pool.go index e02bc628881ff..1bc5b9a369a15 100644 --- a/pkg/util/mempool/pool.go +++ b/pkg/util/mempool/pool.go @@ -42,7 +42,7 @@ func (s *slab) init() { s.buffer = make(chan unsafe.Pointer, s.count) for i := 0; i < s.count; i++ { buf := make([]byte, 0, s.size) - ptr := unsafe.Pointer(unsafe.SliceData(buf)) + ptr := unsafe.Pointer(unsafe.SliceData(buf)) //#nosec G103 -- Simple arena allocator implementation, does not appear to allow for any unsafe operations. s.buffer <- ptr } s.metrics.availableBuffersPerSlab.WithLabelValues(s.name).Set(float64(s.count)) @@ -55,7 +55,7 @@ func (s *slab) get(size int) ([]byte, error) { waitStart := time.Now() // wait for available buffer on channel ptr := <-s.buffer - buf := unsafe.Slice((*byte)(ptr), s.size) + buf := unsafe.Slice((*byte)(ptr), s.size) //#nosec G103 -- Simple arena allocator implementation, does not appear to allow for any unsafe operations. s.metrics.waitDuration.WithLabelValues(s.name).Observe(time.Since(waitStart).Seconds()) return buf[:size], nil @@ -67,7 +67,8 @@ func (s *slab) put(buf []byte) { panic("slab is not initialized") } - ptr := unsafe.Pointer(unsafe.SliceData(buf)) + ptr := unsafe.Pointer(unsafe.SliceData(buf)) //#nosec G103 -- Simple arena allocator implementation, does not appear to allow for any unsafe operations. + // Note that memory is NOT zero'd on return, but since all allocations are of defined widths and we only ever then read a record of exactly that width into the allocation, it will always be overwritten before use and can't leak. s.buffer <- ptr } diff --git a/pkg/util/shard.go b/pkg/util/shard.go index 8e7f8e8c6c2e6..404d38b992f18 100644 --- a/pkg/util/shard.go +++ b/pkg/util/shard.go @@ -29,7 +29,7 @@ var ( // ShuffleShardSeed returns seed for random number generator, computed from provided identifier. func ShuffleShardSeed(identifier, zone string) int64 { // Use the identifier to compute an hash we'll use to seed the random. - hasher := md5.New() + hasher := md5.New() //#nosec G401 -- This does not require collision resistance, this is an intentionally predictable value hasher.Write(YoloBuf(identifier)) // nolint:errcheck if zone != "" { hasher.Write(seedSeparator) // nolint:errcheck diff --git a/pkg/util/ticker.go b/pkg/util/ticker.go index e3a8ee244225f..4edde3d7c9d2e 100644 --- a/pkg/util/ticker.go +++ b/pkg/util/ticker.go @@ -19,7 +19,7 @@ func NewJitter(b time.Duration, d time.Duration) Jitter { // Duration returns a random duration from the base duration and +/- jitter func (j Jitter) Duration() time.Duration { base := j.base - j.deviation - jitter := time.Duration(rand.Int63n(int64(float64(2 * j.deviation.Nanoseconds())))) + jitter := time.Duration(rand.Int63n(int64(float64(2 * j.deviation.Nanoseconds())))) //#nosec G404 -- Jitter does not require CSPRNG return base + jitter } diff --git a/pkg/util/time.go b/pkg/util/time.go index 9de06f381c88c..5b620a73d0a86 100644 --- a/pkg/util/time.go +++ b/pkg/util/time.go @@ -58,7 +58,7 @@ func DurationWithJitter(input time.Duration, variancePerc float64) time.Duration } variance := int64(float64(input) * variancePerc) - jitter := rand.Int63n(variance*2) - variance + jitter := rand.Int63n(variance*2) - variance //#nosec G404 -- Jitter does not require CSPRNG return input + time.Duration(jitter) } @@ -71,7 +71,7 @@ func DurationWithPositiveJitter(input time.Duration, variancePerc float64) time. } variance := int64(float64(input) * variancePerc) - jitter := rand.Int63n(variance) + jitter := rand.Int63n(variance) //#nosec G404 -- Jitter does not require CSPRNG return input + time.Duration(jitter) } diff --git a/pkg/util/unmarshal/unmarshal.go b/pkg/util/unmarshal/unmarshal.go index 4b048d7089c65..ce20a97d373e4 100644 --- a/pkg/util/unmarshal/unmarshal.go +++ b/pkg/util/unmarshal/unmarshal.go @@ -19,7 +19,7 @@ func DecodePushRequest(b io.Reader, r *logproto.PushRequest) error { } *r = logproto.PushRequest{ - Streams: *(*[]logproto.Stream)(unsafe.Pointer(&request.Streams)), + Streams: *(*[]logproto.Stream)(unsafe.Pointer(&request.Streams)), //#nosec G103 -- Just preventing an allocation, safe, there's no chance of an incorrect type cast here. } return nil diff --git a/pkg/util/yolo.go b/pkg/util/yolo.go index 9870d296ff903..14fd8a5807766 100644 --- a/pkg/util/yolo.go +++ b/pkg/util/yolo.go @@ -3,5 +3,5 @@ package util import "unsafe" func YoloBuf(s string) []byte { - return *((*[]byte)(unsafe.Pointer(&s))) + return *((*[]byte)(unsafe.Pointer(&s))) //#nosec G103 -- This is used correctly; all uses of this function do not allow the mutable reference to escape } diff --git a/tools/lambda-promtail/lambda-promtail/promtail_client.go b/tools/lambda-promtail/lambda-promtail/promtail_client.go index a322e82452b7a..6aa28ab8063ca 100644 --- a/tools/lambda-promtail/lambda-promtail/promtail_client.go +++ b/tools/lambda-promtail/lambda-promtail/promtail_client.go @@ -42,7 +42,7 @@ func NewPromtailClient(cfg *promtailClientConfig, log *log.Logger) *promtailClie func NewHTTPClient(cfg *httpClientConfig) *http.Client { transport := http.DefaultTransport if cfg.skipTlsVerify { - transport = &http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: true}} + transport = &http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: true}} //#nosec G402 -- User has explicitly requested to disable TLS } return &http.Client{ Timeout: cfg.timeout,