From 0b1f977a181e350ce7fdec9431fc4c9aef324df8 Mon Sep 17 00:00:00 2001 From: Akihiro Suda Date: Sat, 18 Feb 2023 00:20:46 +0900 Subject: [PATCH] Ensures that the primary GID is also included in the additional GIDs Apply `ensureAdditionalGids()` from https://github.com/containerd/containerd/commit/3eda46af12b1deedab3d0802adb2e81cb3521950 (CVE-2023-25173, https://github.com/advisories/GHSA-hmfx-3pcx-653p) Signed-off-by: Akihiro Suda (cherry picked from commit a0ef27c59310bbbdb591dbd0e4f356e946f731dd) (cherry-pick was not clean) Signed-off-by: Akihiro Suda --- executor/oci/user.go | 13 +++++++++ frontend/dockerfile/dockerfile_test.go | 37 ++++++++++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/executor/oci/user.go b/executor/oci/user.go index eb459f391fbe..bb58e834f634 100644 --- 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 index 5bdd9cc20cb5..f97436a45cc6 100644 --- 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)