Merge commit from fork

dmcgowan · web-flow · commit cf158e884cfe · 2025-03-18T01:23:21.000+08:00
[release 1.6] validate uid/gid
diff --git a/oci/spec_opts.go b/oci/spec_opts.go
@@ -22,6 +22,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"math"
 	"os"
 	"path/filepath"
 	"runtime"
@@ -576,6 +577,20 @@ func WithUser(userstr string) SpecOpts {
 		defer ensureAdditionalGids(s)
 		setProcess(s)
 		s.Process.User.AdditionalGids = nil
+		// While the Linux kernel allows the max UID to be MaxUint32 - 2,
+                // and the OCI Runtime Spec has no definition about the max UID,
+                // the runc implementation is known to require the UID to be <= MaxInt32.
+                //
+                // containerd follows runc's limitation here.
+                //
+                // In future we may relax this limitation to allow MaxUint32 - 2,
+                // or, amend the OCI Runtime Spec to codify the implementation limitation.
+		const (
+			minUserID  = 0
+			maxUserID  = math.MaxInt32
+			minGroupID = 0
+			maxGroupID = math.MaxInt32
+		)
 
 		// For LCOW it's a bit harder to confirm that the user actually exists on the host as a rootfs isn't
 		// mounted on the host and shared into the guest, but rather the rootfs is constructed entirely in the
@@ -592,8 +607,8 @@ func WithUser(userstr string) SpecOpts {
 		switch len(parts) {
 		case 1:
 			v, err := strconv.Atoi(parts[0])
-			if err != nil {
-				// if we cannot parse as a uint they try to see if it is a username
+			if err != nil || v < minUserID || v > maxUserID {
+				// if we cannot parse as an int32 then try to see if it is a username
 				return WithUsername(userstr)(ctx, client, c, s)
 			}
 			return WithUserID(uint32(v))(ctx, client, c, s)
@@ -604,12 +619,13 @@ func WithUser(userstr string) SpecOpts {
 			)
 			var uid, gid uint32
 			v, err := strconv.Atoi(parts[0])
-			if err != nil {
+			if err != nil || v < minUserID || v > maxUserID {
 				username = parts[0]
 			} else {
 				uid = uint32(v)
 			}
-			if v, err = strconv.Atoi(parts[1]); err != nil {
+			v, err = strconv.Atoi(parts[1])
+			if err != nil || v < minGroupID || v > maxGroupID {
 				groupname = parts[1]
 			} else {
 				gid = uint32(v)
diff --git a/oci/spec_opts_linux_test.go b/oci/spec_opts_linux_test.go
@@ -32,6 +32,98 @@ import (
 	"golang.org/x/sys/unix"
 )
 
+//nolint:gosec
+func TestWithUser(t *testing.T) {
+	t.Parallel()
+
+	expectedPasswd := `root:x:0:0:root:/root:/bin/ash
+guest:x:405:100:guest:/dev/null:/sbin/nologin
+`
+	expectedGroup := `root:x:0:root
+bin:x:1:root,bin,daemon
+daemon:x:2:root,bin,daemon
+sys:x:3:root,bin,adm
+guest:x:100:guest
+`
+	td := t.TempDir()
+	apply := fstest.Apply(
+		fstest.CreateDir("/etc", 0777),
+		fstest.CreateFile("/etc/passwd", []byte(expectedPasswd), 0777),
+		fstest.CreateFile("/etc/group", []byte(expectedGroup), 0777),
+	)
+	if err := apply.Apply(td); err != nil {
+		t.Fatalf("failed to apply: %v", err)
+	}
+	c := containers.Container{ID: t.Name()}
+	testCases := []struct {
+		user        string
+		expectedUID uint32
+		expectedGID uint32
+		err         string
+	}{
+		{
+			user:        "0",
+			expectedUID: 0,
+			expectedGID: 0,
+		},
+		{
+			user:        "root:root",
+			expectedUID: 0,
+			expectedGID: 0,
+		},
+		{
+			user:        "guest",
+			expectedUID: 405,
+			expectedGID: 100,
+		},
+		{
+			user:        "guest:guest",
+			expectedUID: 405,
+			expectedGID: 100,
+		},
+		{
+			user: "guest:nobody",
+			err:  "no groups found",
+		},
+		{
+			user:        "405:100",
+			expectedUID: 405,
+			expectedGID: 100,
+		},
+		{
+			user: "405:2147483648",
+			err:  "no groups found",
+		},
+		{
+			user: "-1000",
+			err:  "no users found",
+		},
+		{
+			user: "2147483648",
+			err:  "no users found",
+		},
+	}
+	for _, testCase := range testCases {
+		testCase := testCase
+		t.Run(testCase.user, func(t *testing.T) {
+			t.Parallel()
+			s := Spec{
+				Version: specs.Version,
+				Root: &specs.Root{
+					Path: td,
+				},
+				Linux: &specs.Linux{},
+			}
+			err := WithUser(testCase.user)(context.Background(), nil, &c, &s)
+			if err != nil {
+				assert.EqualError(t, err, testCase.err)
+			}
+			assert.Equal(t, testCase.expectedUID, s.Process.User.UID)
+			assert.Equal(t, testCase.expectedGID, s.Process.User.GID)
+		})
+	}
+}
+
 //nolint:gosec
 func TestWithUserID(t *testing.T) {
 	t.Parallel()
