Ensures that the primary GID is also included in the additional GIDs

Apply `ensureAdditionalGids()` from containerd/containerd@3eda46a (CVE-2023-25173, GHSA-hmfx-3pcx-653p) Signed-off-by: Akihiro Suda <[email protected]> (cherry picked from commit a0ef27c) (cherry-pick was not clean) Signed-off-by: Akihiro Suda <[email protected]>
moby · Feb 24, 2023 · 0b1f977 · 0b1f977
1 parent 098fd86
commit 0b1f977
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 0 deletions.
diff --git a/executor/oci/user.go b/executor/oci/user.go
@@ -91,6 +91,7 @@ func parseUID(str string) (uint32, error) {
 // once the PR in containerd is merged we should remove this function.
 func WithUIDGID(uid, gid uint32, sgids []uint32) containerdoci.SpecOpts {
 	return func(_ context.Context, _ containerdoci.Client, _ *containers.Container, s *containerdoci.Spec) error {
+		defer ensureAdditionalGids(s)
 		setProcess(s)
 		s.Process.User.UID = uid
 		s.Process.User.GID = gid
@@ -106,3 +107,15 @@ func setProcess(s *containerdoci.Spec) {
 		s.Process = &specs.Process{}
 	}
 }
+
+// ensureAdditionalGids ensures that the primary GID is also included in the additional GID list.
+// From https://github.com/containerd/containerd/blob/v1.7.0-beta.4/oci/spec_opts.go#L124-L133
+func ensureAdditionalGids(s *containerdoci.Spec) {
+	setProcess(s)
+	for _, f := range s.Process.User.AdditionalGids {
+		if f == s.Process.User.GID {
+			return
+		}
+	}
+	s.Process.User.AdditionalGids = append([]uint32{s.Process.User.GID}, s.Process.User.AdditionalGids...)
+}
diff --git a/frontend/dockerfile/dockerfile_test.go b/frontend/dockerfile/dockerfile_test.go
@@ -67,6 +67,7 @@ var allTests = integration.TestFuncs(
 	testExportedHistory,
 	testExposeExpansion,
 	testUser,
+	testUserAdditionalGids,
 	testCacheReleased,
 	testDockerignore,
 	testDockerignoreInvalid,
@@ -3108,6 +3109,42 @@ USER nobody
 	require.Equal(t, "nobody", ociimg.Config.User)
 }
 
+// testUserAdditionalGids ensures that that the primary GID is also included in the additional GID list.
+// CVE-2023-25173: https://github.com/advisories/GHSA-hmfx-3pcx-653p
+func testUserAdditionalGids(t *testing.T, sb integration.Sandbox) {
+	f := getFrontend(t, sb)
+
+	dockerfile := []byte(`
+# Mimics the tests in https://github.com/containerd/containerd/commit/3eda46af12b1deedab3d0802adb2e81cb3521950
+FROM busybox
+SHELL ["/bin/sh", "-euxc"]
+RUN [ "$(id)" = "uid=0(root) gid=0(root) groups=0(root),10(wheel)" ]
+USER 1234
+RUN [ "$(id)" = "uid=1234 gid=0(root) groups=0(root)" ]
+USER 1234:1234
+RUN [ "$(id)" = "uid=1234 gid=1234 groups=1234" ]
+USER daemon
+RUN [ "$(id)" = "uid=1(daemon) gid=1(daemon) groups=1(daemon)" ]
+`)
+
+	dir, err := tmpdir(
+		fstest.CreateFile("Dockerfile", dockerfile, 0600),
+	)
+	require.NoError(t, err)
+
+	c, err := client.New(sb.Context(), sb.Address())
+	require.NoError(t, err)
+	defer c.Close()
+
+	_, err = f.Solve(sb.Context(), c, client.SolveOpt{
+		LocalDirs: map[string]string{
+			builder.DefaultLocalNameDockerfile: dir,
+			builder.DefaultLocalNameContext:    dir,
+		},
+	}, nil)
+	require.NoError(t, err)
+}
+
 func testCopyChown(t *testing.T, sb integration.Sandbox) {
 	f := getFrontend(t, sb)
 	isFileOp := getFileOp(t, sb)