bug: resolve type conversion alerts (#20553)

.changelog/20553.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+core: Fix multiple incorrect type conversion for potential overflows
+```

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")
 

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
 	}

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

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)
 		})

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
 }

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...)
 

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
 	}