diff --git a/.changelog/20553.txt b/.changelog/20553.txt new file mode 100644 index 000000000..ff4ef7cf2 --- /dev/null +++ b/.changelog/20553.txt @@ -0,0 +1,3 @@ +```release-note:bug +core: Fix multiple incorrect type conversion for potential overflows +``` diff --git a/api/api.go b/api/api.go index b30740767..7b42b32de 100644 --- a/api/api.go +++ b/api/api.go @@ -1184,6 +1184,9 @@ func parseQueryMeta(resp *http.Response, q *QueryMeta) error { if err != nil { return fmt.Errorf("Failed to parse X-Nomad-LastContact: %v", err) } + if last > math.MaxInt64 { + return fmt.Errorf("Last contact duration is out of range: %d", last) + } q.LastContact = time.Duration(last) * time.Millisecond q.NextToken = header.Get("X-Nomad-NextToken") diff --git a/client/lib/numalib/detect_linux.go b/client/lib/numalib/detect_linux.go index c4c933412..51dfaaf1e 100644 --- a/client/lib/numalib/detect_linux.go +++ b/client/lib/numalib/detect_linux.go @@ -89,7 +89,7 @@ func (*Sysfs) discoverCosts(st *Topology, readerFunc pathReaderFn) { } for i, c := range strings.Fields(s) { - cost, _ := strconv.Atoi(c) + cost, _ := strconv.ParseUint(c, 10, 8) st.Distances[id][i] = Cost(cost) } return nil @@ -110,8 +110,8 @@ func (*Sysfs) discoverCores(st *Topology, readerFunc pathReaderFn) { st.NodeIDs = idset.From[hw.NodeID]([]hw.NodeID{0}) const node = 0 const socket = 0 - cpuMax, _ := getNumeric[hw.KHz](cpuMaxFile, readerFunc, core) - base, _ := getNumeric[hw.KHz](cpuBaseFile, readerFunc, core) + cpuMax, _ := getNumeric[hw.KHz](cpuMaxFile, 64, readerFunc, core) + base, _ := getNumeric[hw.KHz](cpuBaseFile, 64, readerFunc, core) st.insert(node, socket, core, Performance, cpuMax, base) return nil }) @@ -126,9 +126,9 @@ func (*Sysfs) discoverCores(st *Topology, readerFunc pathReaderFn) { cores := idset.Parse[hw.CoreID](string(s)) _ = cores.ForEach(func(core hw.CoreID) error { // best effort, zero values are defaults - socket, _ := getNumeric[hw.SocketID](cpuSocketFile, readerFunc, core) - cpuMax, _ := getNumeric[hw.KHz](cpuMaxFile, readerFunc, core) - base, _ := getNumeric[hw.KHz](cpuBaseFile, readerFunc, core) + socket, _ := getNumeric[hw.SocketID](cpuSocketFile, 8, readerFunc, core) + cpuMax, _ := getNumeric[hw.KHz](cpuMaxFile, 64, readerFunc, core) + base, _ := getNumeric[hw.KHz](cpuBaseFile, 64, readerFunc, core) siblings, _ := getIDSet[hw.CoreID](cpuSiblingFile, readerFunc, core) // if we get an incorrect core number, this means we're not getting the right @@ -154,13 +154,13 @@ func getIDSet[T idset.ID](path string, readerFunc pathReaderFn, args ...any) (*i return idset.Parse[T](string(s)), nil } -func getNumeric[T int | idset.ID](path string, readerFunc pathReaderFn, args ...any) (T, error) { +func getNumeric[T int | idset.ID](path string, maxSize int, readerFunc pathReaderFn, args ...any) (T, error) { path = fmt.Sprintf(path, args...) s, err := readerFunc(path) if err != nil { return 0, err } - i, err := strconv.Atoi(strings.TrimSpace(string(s))) + i, err := strconv.ParseUint(strings.TrimSpace(string(s)), 10, maxSize) if err != nil { return 0, err } diff --git a/command/agent/http.go b/command/agent/http.go index 77394e8d8..23d5cef15 100644 --- a/command/agent/http.go +++ b/command/agent/http.go @@ -903,21 +903,23 @@ func parseWait(resp http.ResponseWriter, req *http.Request, b *structs.QueryOpti } // parseConsistency is used to parse the ?stale query params. -func parseConsistency(resp http.ResponseWriter, req *http.Request, b *structs.QueryOptions) { +func parseConsistency(resp http.ResponseWriter, req *http.Request, b *structs.QueryOptions) error { query := req.URL.Query() if staleVal, ok := query["stale"]; ok { if len(staleVal) == 0 || staleVal[0] == "" { b.AllowStale = true - return + return nil } staleQuery, err := strconv.ParseBool(staleVal[0]) if err != nil { + errMsg := "Expect `true` or `false` for `stale` query string parameter" resp.WriteHeader(http.StatusBadRequest) - _, _ = resp.Write([]byte(fmt.Sprintf("Expect `true` or `false` for `stale` query string parameter, got %s", staleVal[0]))) - return + resp.Write([]byte(errMsg)) + return CodedError(http.StatusBadRequest, errMsg) } b.AllowStale = staleQuery } + return nil } // parsePrefix is used to parse the ?prefix query param @@ -1014,27 +1016,36 @@ func (s *HTTPServer) parseToken(req *http.Request, token *string) { func (s *HTTPServer) parse(resp http.ResponseWriter, req *http.Request, r *string, b *structs.QueryOptions) bool { s.parseRegion(req, r) s.parseToken(req, &b.AuthToken) - parseConsistency(resp, req, b) + if err := parseConsistency(resp, req, b); err != nil { + return true + } parsePrefix(req, b) parseNamespace(req, &b.Namespace) - parsePagination(req, b) + if err := parsePagination(resp, req, b); err != nil { + return true + } parseFilter(req, b) parseReverse(req, b) return parseWait(resp, req, b) } // parsePagination parses the pagination fields for QueryOptions -func parsePagination(req *http.Request, b *structs.QueryOptions) { +func parsePagination(resp http.ResponseWriter, req *http.Request, b *structs.QueryOptions) error { query := req.URL.Query() rawPerPage := query.Get("per_page") if rawPerPage != "" { perPage, err := strconv.ParseInt(rawPerPage, 10, 32) - if err == nil { - b.PerPage = int32(perPage) + if err != nil { + errMsg := "Expect a number for `per_page` query string parameter" + resp.WriteHeader(http.StatusBadRequest) + resp.Write([]byte(errMsg)) + return CodedError(http.StatusBadRequest, errMsg) } + b.PerPage = int32(perPage) } b.NextToken = query.Get("next_token") + return nil } // parseFilter parses the filter query parameter for QueryOptions diff --git a/command/agent/http_test.go b/command/agent/http_test.go index 19b70d305..a79fed7e5 100644 --- a/command/agent/http_test.go +++ b/command/agent/http_test.go @@ -503,6 +503,15 @@ func TestParseConsistency(t *testing.T) { parseConsistency(resp, req, &b) must.False(t, b.AllowStale) must.EqOp(t, 400, resp.Code) + must.EqOp(t, "Expect `true` or `false` for `stale` query string parameter", resp.Body.String()) + + req, err = http.NewRequest(http.MethodGet, "/v1/jobs?stale=random", nil) + must.NoError(t, err) + resp = httptest.NewRecorder() + parseConsistency(resp, req, &b) + must.False(t, b.AllowStale) + must.EqOp(t, 400, resp.Code) + must.EqOp(t, "Expect `true` or `false` for `stale` query string parameter", resp.Body.String()) b = structs.QueryOptions{} req, err = http.NewRequest(http.MethodGet, "/v1/catalog/nodes?consistent", nil) @@ -721,7 +730,8 @@ func TestParsePagination(t *testing.T) { require.NoError(t, err) opts := &structs.QueryOptions{} - parsePagination(req, opts) + resp := httptest.NewRecorder() + parsePagination(resp, req, opts) require.Equal(t, tc.ExpectedNextToken, opts.NextToken) require.Equal(t, tc.ExpectedPerPage, opts.PerPage) }) diff --git a/helper/flags/autopilot_flags.go b/helper/flags/autopilot_flags.go index 0f2bd1953..414661343 100644 --- a/helper/flags/autopilot_flags.go +++ b/helper/flags/autopilot_flags.go @@ -94,6 +94,10 @@ func (u *UintValue) Set(v string) error { } parsed, err := strconv.ParseUint(v, 0, bits.UintSize) + if err != nil { + return err + } + *(u.v) = (uint)(parsed) return err } diff --git a/helper/testlog/testlog.go b/helper/testlog/testlog.go index 043c275d4..a5feaa8d7 100644 --- a/helper/testlog/testlog.go +++ b/helper/testlog/testlog.go @@ -11,6 +11,7 @@ import ( "fmt" "io" "log" + "math" "os" hclog "github.com/hashicorp/go-hclog" @@ -104,7 +105,11 @@ func (w *prefixStderr) Write(p []byte) (int, error) { // decrease likely hood of partial line writes that may mess up test // indicator success detection - buf := make([]byte, 0, len(w.prefix)+len(p)) + totalLength := len(w.prefix) + len(p) + if totalLength < 0 || totalLength > math.MaxInt32 { + return 0, fmt.Errorf("Total length of the prefix message is out of range: %d", totalLength) + } + buf := make([]byte, 0, totalLength) buf = append(buf, w.prefix...) buf = append(buf, p...) diff --git a/helper/users/dynamic/users.go b/helper/users/dynamic/users.go index 9d065b5fd..5ce17560c 100644 --- a/helper/users/dynamic/users.go +++ b/helper/users/dynamic/users.go @@ -5,6 +5,7 @@ package dynamic import ( "fmt" + "math" "regexp" "strconv" @@ -38,7 +39,7 @@ func Parse(user string) (UGID, error) { } i, err := strconv.ParseUint(values[1], 10, 64) - if err != nil { + if err != nil || i > uint64(math.MaxInt32) { return none, ErrCannotParse }