diff --git a/.golangci.yml b/.golangci.yml index 3f7e458..af789dd 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -2,15 +2,7 @@ linters: enable: - misspell - revive - disable: - # Disable soon to deprecated[1] linters that lead to false - # positives when build tags disable certain files[2] - # 1: https://github.com/golangci/golangci-lint/issues/1841 - # 2: https://github.com/prometheus/node_exporter/issues/1545 - - deadcode - - unused - - structcheck - - varcheck + - gosec issues: exclude-rules: diff --git a/internal/osexec/osexec.go b/internal/osexec/osexec.go index 5b2764f..9e69928 100644 --- a/internal/osexec/osexec.go +++ b/internal/osexec/osexec.go @@ -3,6 +3,7 @@ package osexec import ( "context" + "math" "os" "os/exec" "strings" @@ -46,9 +47,18 @@ func ExecuteAs(cmd string, args []string, uid int, gid int, env []string, logger Log("msg", "Executing as user", "command", cmd, "args", strings.Join(args, " "), "uid", uid, "gid", gid) execCmd := exec.Command(cmd, args...) + // Check bounds on uid and gid before converting into int32 + var uidInt32, gidInt32 uint32 + if uid > 0 && uid <= math.MaxInt32 { + uidInt32 = uint32(uid) + } + if gid > 0 && gid <= math.MaxInt32 { + gidInt32 = uint32(gid) + } + // Set uid and gid for process execCmd.SysProcAttr = &syscall.SysProcAttr{} - execCmd.SysProcAttr.Credential = &syscall.Credential{Uid: uint32(uid), Gid: uint32(gid)} + execCmd.SysProcAttr.Credential = &syscall.Credential{Uid: uidInt32, Gid: gidInt32} // If env is not nil pointer, add env vars into subprocess cmd if env != nil { @@ -129,9 +139,18 @@ func ExecuteAsWithTimeout( execCmd.Env = append(os.Environ(), env...) } - // Set uid and gid for the process + // Check bounds on uid and gid before converting into int32 + var uidInt32, gidInt32 uint32 + if uid > 0 && uid <= math.MaxInt32 { + uidInt32 = uint32(uid) + } + if gid > 0 && gid <= math.MaxInt32 { + gidInt32 = uint32(gid) + } + + // Set uid and gid for process execCmd.SysProcAttr = &syscall.SysProcAttr{} - execCmd.SysProcAttr.Credential = &syscall.Credential{Uid: uint32(uid), Gid: uint32(gid)} + execCmd.SysProcAttr.Credential = &syscall.Credential{Uid: uidInt32, Gid: gidInt32} // Execute command out, err := execCmd.CombinedOutput() diff --git a/pkg/api/cli/cli_test.go b/pkg/api/cli/cli_test.go index 3676c2c..a12b7d2 100644 --- a/pkg/api/cli/cli_test.go +++ b/pkg/api/cli/cli_test.go @@ -37,7 +37,7 @@ func queryServer(address string) error { func makeConfigFile(configFile string, tmpDir string) string { configPath := filepath.Join(tmpDir, "config.yml") - os.WriteFile(configPath, []byte(configFile), 0644) + os.WriteFile(configPath, []byte(configFile), 0600) return configPath } diff --git a/pkg/api/db/db.go b/pkg/api/db/db.go index 0e794c8..a55b981 100644 --- a/pkg/api/db/db.go +++ b/pkg/api/db/db.go @@ -507,18 +507,18 @@ func (s *statsDB) getUnitStats(startTime, endTime time.Time) error { // Make prepare statement and defer closing statement level.Debug(s.logger).Log("msg", "Preparing SQL statements") - stmtMap, err := s.prepareStatements(tx) + sqlStmts, err := s.prepareStatements(tx) if err != nil { return err } - for _, stmt := range stmtMap { + for _, stmt := range sqlStmts { defer stmt.Close() } level.Debug(s.logger).Log("msg", "Finished preparing SQL statements") // Insert data into DB level.Debug(s.logger).Log("msg", "Executing SQL statements") - s.execStatements(stmtMap, units, users, projects) + s.execStatements(sqlStmts, units, users, projects) level.Debug(s.logger).Log("msg", "Finished executing SQL statements") // Commit changes @@ -536,7 +536,7 @@ func (s *statsDB) getUnitStats(startTime, endTime time.Time) error { func (s *statsDB) purgeExpiredUnits(tx *sql.Tx) error { // Purge expired units deleteUnitsQuery := fmt.Sprintf( - "DELETE FROM %s WHERE started_at <= date('now', '-%d day')", + "DELETE FROM %s WHERE started_at <= date('now', '-%d day')", // #nosec base.UnitsDBTableName, int(s.storage.retentionPeriod.Hours()/24), ) @@ -551,7 +551,7 @@ func (s *statsDB) purgeExpiredUnits(tx *sql.Tx) error { // Purge stale usage data deleteUsageQuery := fmt.Sprintf( - "DELETE FROM %s WHERE last_updated_at <= date('now', '-%d day')", + "DELETE FROM %s WHERE last_updated_at <= date('now', '-%d day')", // #nosec base.UsageDBTableName, int(s.storage.retentionPeriod.Hours()/24), ) diff --git a/pkg/api/db/helpers.go b/pkg/api/db/helpers.go index bf4b8d9..bb7ceb6 100644 --- a/pkg/api/db/helpers.go +++ b/pkg/api/db/helpers.go @@ -39,7 +39,7 @@ func makeDSN(filePath string, opts map[string]string) string { func writeTimeStampToFile(filePath string, timeStamp time.Time, logger log.Logger) { timeStampString := timeStamp.Format(base.DatetimeLayout) timeStampByte := []byte(timeStampString) - if err := os.WriteFile(filePath, timeStampByte, 0644); err != nil { + if err := os.WriteFile(filePath, timeStampByte, 0600); err != nil { level.Error(logger). Log("msg", "Failed to write timestamp to file", "time", timeStampString, "file", filePath, "err", err) } diff --git a/pkg/api/http/demo.go b/pkg/api/http/demo.go index a48eeee..8843956 100644 --- a/pkg/api/http/demo.go +++ b/pkg/api/http/demo.go @@ -58,7 +58,7 @@ func init() { // randomFloats returns random float64s in the range func randomFloats(min, max float64) models.JSONFloat { - return models.JSONFloat(min + rand.Float64()*(max-min)) + return models.JSONFloat(min + rand.Float64()*(max-min)) // #nosec } // random returns random number between min and max excluding max @@ -69,11 +69,11 @@ func random(min, max int64) int64 { // randomHelper returns max int64 if n is more than max func randomHelper(n int64) int64 { if n < maxInt64 { - return int64(rand.Int63n(int64(n + 1))) + return int64(rand.Int63n(int64(n + 1))) // #nosec } - x := int64(rand.Uint64()) + x := int64(rand.Uint64()) // #nosec for x > n { - x = int64(rand.Uint64()) + x = int64(rand.Uint64()) // #nosec } return x } diff --git a/pkg/api/http/middleware_test.go b/pkg/api/http/middleware_test.go index 3266fe5..bd24386 100644 --- a/pkg/api/http/middleware_test.go +++ b/pkg/api/http/middleware_test.go @@ -40,9 +40,11 @@ func TestMiddlewareSuccess(t *testing.T) { // call the handler using a mock response recorder (we'll not use that anyway) w := httptest.NewRecorder() handlerToTest.ServeHTTP(w, req) + resp := w.Result() + defer resp.Body.Close() // Should pass test - if w.Result().StatusCode != 200 { + if resp.StatusCode != 200 { t.Errorf("expected 200 got %d", w.Result().StatusCode) } // Check headers added to req diff --git a/pkg/api/http/server.go b/pkg/api/http/server.go index 810a769..8b6ff0d 100644 --- a/pkg/api/http/server.go +++ b/pkg/api/http/server.go @@ -7,7 +7,7 @@ import ( "encoding/json" "fmt" "net/http" - _ "net/http/pprof" + _ "net/http/pprof" // #nosec "net/url" "path/filepath" "regexp" @@ -129,10 +129,11 @@ func NewCEEMSServer(c *Config) (*CEEMSServer, func(), error) { server := &CEEMSServer{ logger: c.Logger, server: &http.Server{ - Addr: c.Web.Address, - Handler: router, - ReadTimeout: 10 * time.Second, - WriteTimeout: 10 * time.Second, + Addr: c.Web.Address, + Handler: router, + ReadTimeout: 10 * time.Second, + WriteTimeout: 10 * time.Second, + ReadHeaderTimeout: 2 * time.Second, // slowloris attack: https://app.deepsource.com/directory/analyzers/go/issues/GO-S2112 }, webConfig: &web.FlagConfig{ WebListenAddresses: &[]string{c.Web.Address}, diff --git a/pkg/api/resource/manager_test.go b/pkg/api/resource/manager_test.go index f5975dc..2bbded8 100644 --- a/pkg/api/resource/manager_test.go +++ b/pkg/api/resource/manager_test.go @@ -105,7 +105,7 @@ clusters: configFile := fmt.Sprintf(configFileTmpl, tmpDir, serverURL) configPath := filepath.Join(tmpDir, "config.yml") - os.WriteFile(configPath, []byte(configFile), 0644) + os.WriteFile(configPath, []byte(configFile), 0600) return configPath } diff --git a/pkg/api/resource/slurm/manager_test.go b/pkg/api/resource/slurm/manager_test.go index 6640f22..13a8dc2 100644 --- a/pkg/api/resource/slurm/manager_test.go +++ b/pkg/api/resource/slurm/manager_test.go @@ -145,12 +145,12 @@ func TestSLURMFetcherMultiCluster(t *testing.T) { sacctPath := filepath.Join(tmpDir, "sacct") sacctScript := fmt.Sprintf(`#!/bin/bash printf """%s"""`, sacctCmdOutput) - os.WriteFile(sacctPath, []byte(sacctScript), 0755) + os.WriteFile(sacctPath, []byte(sacctScript), 0700) // #nosec sacctMgrPath := filepath.Join(tmpDir, "sacctmgr") sacctMgrScript := fmt.Sprintf(`#!/bin/bash printf """%s"""`, sacctMgrCmdOutput) - os.WriteFile(sacctMgrPath, []byte(sacctMgrScript), 0755) + os.WriteFile(sacctMgrPath, []byte(sacctMgrScript), 0700) // #nosec sacctMgrDir := filepath.Dir(sacctMgrPath) diff --git a/pkg/api/updater/updater_test.go b/pkg/api/updater/updater_test.go index d801ef7..71819a5 100644 --- a/pkg/api/updater/updater_test.go +++ b/pkg/api/updater/updater_test.go @@ -77,7 +77,7 @@ updaters: configFile := fmt.Sprintf(configFileTmpl, serverURL, "2m") configPath := filepath.Join(tmpDir, "config.yml") - os.WriteFile(configPath, []byte(configFile), 0644) + os.WriteFile(configPath, []byte(configFile), 0600) return configPath } diff --git a/pkg/collector/cli.go b/pkg/collector/cli.go index 20a32c4..e95e046 100644 --- a/pkg/collector/cli.go +++ b/pkg/collector/cli.go @@ -3,10 +3,11 @@ package collector import ( "fmt" "net/http" - _ "net/http/pprof" + _ "net/http/pprof" // #nosec "os" "os/user" "runtime" + "time" "github.com/alecthomas/kingpin/v2" "github.com/go-kit/log" @@ -160,7 +161,9 @@ func (b *CEEMSExporter) Main() error { http.Handle("/", landingPage) } - server := &http.Server{} + server := &http.Server{ + ReadHeaderTimeout: 2 * time.Second, // slowloris attack: https://app.deepsource.com/directory/analyzers/go/issues/GO-S2112 + } if err := web.ListenAndServe(server, toolkitFlags, logger); err != nil { return fmt.Errorf("failed to start server: %s", err) } diff --git a/pkg/collector/handler.go b/pkg/collector/handler.go index 0d9b841..8d0376a 100644 --- a/pkg/collector/handler.go +++ b/pkg/collector/handler.go @@ -41,7 +41,7 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { if err != nil { level.Warn(h.logger).Log("msg", "Couldn't create filtered metrics handler:", "err", err) w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(fmt.Sprintf("Couldn't create filtered metrics handler: %s", err))) + w.Write([]byte("Couldn't create filtered metrics handler")) return } filteredHandler.ServeHTTP(w, r) diff --git a/pkg/collector/slurm.go b/pkg/collector/slurm.go index 7587514..5be8e03 100644 --- a/pkg/collector/slurm.go +++ b/pkg/collector/slurm.go @@ -581,7 +581,12 @@ func (c *slurmCollector) readJobPropsFromProlog(jobid string, jobProps *JobProps level.Error(c.logger). Log("msg", "Failed to get job properties from prolog generated files", "file", slurmJobInfo, "err", err) } else { - fmt.Sscanf(string(content), "%s %s %s", &jobProps.jobUser, &jobProps.jobAccount, &jobProps.jobNodelist) + if _, err := fmt.Sscanf( + string(content), "%s %s %s", &jobProps.jobUser, &jobProps.jobAccount, &jobProps.jobNodelist, + ); err != nil { + level.Error(c.logger). + Log("msg", "Failed to scan prolog generated file", "file", slurmJobInfo, "err", err) + } } } @@ -612,7 +617,13 @@ func (c *slurmCollector) readJobPropsFromProlog(jobid string, jobProps *JobProps ) continue } - fmt.Sscanf(string(content), "%s", &gpuJobID) + if _, err := fmt.Sscanf(string(content), "%s", &gpuJobID); err != nil { + level.Error(c.logger).Log( + "msg", "Failed to scan job ID for GPU", + "index", dev.index, "uuid", dev.uuid, "err", err, + ) + continue + } if gpuJobID == jobid { jobProps.jobGPUOrdinals = append(jobProps.jobGPUOrdinals, dev.index) } diff --git a/pkg/lb/cli/cli_test.go b/pkg/lb/cli/cli_test.go index fb76f37..47ab9a1 100644 --- a/pkg/lb/cli/cli_test.go +++ b/pkg/lb/cli/cli_test.go @@ -48,7 +48,7 @@ func queryLB(address string) error { func makeConfigFile(configFile string, tmpDir string) string { configPath := filepath.Join(tmpDir, "config.yml") - os.WriteFile(configPath, []byte(configFile), 0644) + os.WriteFile(configPath, []byte(configFile), 0600) return configPath } diff --git a/pkg/lb/frontend/frontend.go b/pkg/lb/frontend/frontend.go index a82ebcf..e96e77c 100644 --- a/pkg/lb/frontend/frontend.go +++ b/pkg/lb/frontend/frontend.go @@ -114,7 +114,8 @@ outside: return &loadBalancer{ logger: c.Logger, server: &http.Server{ - Addr: c.Address, + Addr: c.Address, + ReadHeaderTimeout: 2 * time.Second, // slowloris attack: https://app.deepsource.com/directory/analyzers/go/issues/GO-S2112 }, webConfig: &web.FlagConfig{ WebListenAddresses: &[]string{c.Address},