From 358a46819bd8321f0bc0c97ee882c7d7c1ca7cd7 Mon Sep 17 00:00:00 2001
From: Tim Gross <tgross@hashicorp.com>
Date: Tue, 25 Jan 2022 11:16:48 -0500
Subject: [PATCH] fix integer bounds checks (#11815)

* driver: fix integer conversion error

The shared executor incorrectly parsed the user's group into int32 and
then cast to uint32 without bounds checking. This is harmless because
an out-of-bounds gid will throw an error later, but it triggers
security and code quality scans. Parse directly to uint32 so that we
get correct error handling.

* helper: fix integer conversion error

The autopilot flags helper incorrectly parses a uint64 to a uint which
is machine specific size. Although we don't have 32-bit builds, this
sets off security and code quality scaans. Parse to the machine sized
uint.

* driver: restrict bounds of port map

The plugin server doesn't constrain the maximum integer for port
maps. This could result in a user-visible misconfiguration, but it
also triggers security and code quality scans. Restrict the bounds
before casting to int32 and return an error.

* cpuset: restrict upper bounds of cpuset values

Our cpuset configuration expects values in the range of uint16 to
match the expectations set by the kernel, but we don't constrain the
values before downcasting. An underflow could lead to allocations
failing on the client rather than being caught earlier. This also make
security and code quality scanners happy.

* http: fix integer downcast for per_page parameter

The parser for the `per_page` query parameter downcasts to int32
without bounds checking. This could result in underflow and
nonsensical paging, but there's no server-side consequences for
this. Fixing this will silence some security and code quality scanners
though.
---
 command/agent/http.go                               | 2 +-
 drivers/shared/executor/executor_universal_linux.go | 4 ++--
 helper/flags/autopilot_flags.go                     | 4 +++-
 lib/cpuset/cpuset.go                                | 8 ++++++++
 plugins/drivers/server.go                           | 4 ++++
 5 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/command/agent/http.go b/command/agent/http.go
index d33be8e85c9..de6a314e3bc 100644
--- a/command/agent/http.go
+++ b/command/agent/http.go
@@ -766,7 +766,7 @@ func parsePagination(req *http.Request, b *structs.QueryOptions) {
 	query := req.URL.Query()
 	rawPerPage := query.Get("per_page")
 	if rawPerPage != "" {
-		perPage, err := strconv.Atoi(rawPerPage)
+		perPage, err := strconv.ParseInt(rawPerPage, 10, 32)
 		if err == nil {
 			b.PerPage = int32(perPage)
 		}
diff --git a/drivers/shared/executor/executor_universal_linux.go b/drivers/shared/executor/executor_universal_linux.go
index 87184d843b6..d5076a8d3d6 100644
--- a/drivers/shared/executor/executor_universal_linux.go
+++ b/drivers/shared/executor/executor_universal_linux.go
@@ -33,9 +33,9 @@ func setCmdUser(cmd *exec.Cmd, userid string) error {
 
 	gids := make([]uint32, len(gidStrings))
 	for _, gidString := range gidStrings {
-		u, err := strconv.Atoi(gidString)
+		u, err := strconv.ParseUint(gidString, 10, 32)
 		if err != nil {
-			return fmt.Errorf("Unable to convert user's group to int %s: %v", gidString, err)
+			return fmt.Errorf("Unable to convert user's group to uint32 %s: %v", gidString, err)
 		}
 
 		gids = append(gids, uint32(u))
diff --git a/helper/flags/autopilot_flags.go b/helper/flags/autopilot_flags.go
index 67740a8f594..d0a984a72e0 100644
--- a/helper/flags/autopilot_flags.go
+++ b/helper/flags/autopilot_flags.go
@@ -5,6 +5,7 @@ package flags
 
 import (
 	"fmt"
+	"math/bits"
 	"strconv"
 	"time"
 )
@@ -88,7 +89,8 @@ func (u *UintValue) Set(v string) error {
 	if u.v == nil {
 		u.v = new(uint)
 	}
-	parsed, err := strconv.ParseUint(v, 0, 64)
+
+	parsed, err := strconv.ParseUint(v, 0, bits.UintSize)
 	*(u.v) = (uint)(parsed)
 	return err
 }
diff --git a/lib/cpuset/cpuset.go b/lib/cpuset/cpuset.go
index e794d354c5b..0caa966710d 100644
--- a/lib/cpuset/cpuset.go
+++ b/lib/cpuset/cpuset.go
@@ -2,6 +2,7 @@ package cpuset
 
 import (
 	"fmt"
+	"math"
 	"reflect"
 	"sort"
 	"strconv"
@@ -153,6 +154,9 @@ func Parse(s string) (CPUSet, error) {
 				return New(), err
 			}
 
+			if v > math.MaxUint16 {
+				return New(), fmt.Errorf("failed to parse element %s, more than max allowed cores", set)
+			}
 			cpuset.cpus[uint16(v)] = struct{}{}
 			continue
 		}
@@ -168,7 +172,11 @@ func Parse(s string) (CPUSet, error) {
 		if err != nil {
 			return New(), err
 		}
+
 		for v := lower; v <= upper; v++ {
+			if v > math.MaxUint16 {
+				return New(), fmt.Errorf("failed to parse element %s, more than max allowed cores", set)
+			}
 			cpuset.cpus[uint16(v)] = struct{}{}
 		}
 	}
diff --git a/plugins/drivers/server.go b/plugins/drivers/server.go
index 15896801e64..0117e0c1c65 100644
--- a/plugins/drivers/server.go
+++ b/plugins/drivers/server.go
@@ -3,6 +3,7 @@ package drivers
 import (
 	"fmt"
 	"io"
+	"math"
 
 	"github.com/golang/protobuf/ptypes"
 	plugin "github.com/hashicorp/go-plugin"
@@ -125,6 +126,9 @@ func (b *driverPluginServer) StartTask(ctx context.Context, req *proto.StartTask
 			AutoAdvertise: net.AutoAdvertise,
 		}
 		for k, v := range net.PortMap {
+			if v > math.MaxInt32 {
+				return nil, fmt.Errorf("port map out of bounds")
+			}
 			pbNet.PortMap[k] = int32(v)
 		}
 	}