Enforce compose files from OCI artifact all get into the same target (cache) folder

Signed-off-by: Guillaume Lours <[email protected]>
Signed-off-by: Nicolas De Loof <[email protected]>

Author: glours

pkg/remote/oci.go

@@ -39,6 +39,32 @@ const (
 	OciPrefix          = "oci://"
 )
 
+// validatePathInBase ensures a file path is contained within the base directory,
+// as OCI artifacts resources must all live within the same folder.
+func validatePathInBase(base, unsafePath string) error {
+	// Reject paths with path separators regardless of OS
+	if strings.ContainsAny(unsafePath, "\\/") {
+		return fmt.Errorf("invalid OCI artifact")
+	}
+
+	// Join the base with the untrusted path
+	targetPath := filepath.Join(base, unsafePath)
+
+	// Get the directory of the target path
+	targetDir := filepath.Dir(targetPath)
+
+	// Clean both paths to resolve any .. or . components
+	cleanBase := filepath.Clean(base)
+	cleanTargetDir := filepath.Clean(targetDir)
+
+	// Check if the target directory is the same as base directory
+	if cleanTargetDir != cleanBase {
+		return fmt.Errorf("invalid OCI artifact")
+	}
+
+	return nil
+}
+
 func ociRemoteLoaderEnabled() (bool, error) {
 	if v := os.Getenv(OCI_REMOTE_ENABLED); v != "" {
 		enabled, err := strconv.ParseBool(v)
@@ -158,12 +184,6 @@ func (g ociRemoteLoader) pullComposeFiles(ctx context.Context, local string, man
 	if err != nil {
 		return err
 	}
-	composeFile := filepath.Join(local, "compose.yaml")
-	f, err := os.Create(composeFile)
-	if err != nil {
-		return err
-	}
-	defer f.Close() //nolint:errcheck
 	if (manifest.ArtifactType != "" && manifest.ArtifactType != oci.ComposeProjectArtifactType) ||
 		(manifest.ArtifactType == "" && manifest.Config.MediaType != oci.ComposeEmptyConfigMediaType) {
 		return fmt.Errorf("%s is not a compose project OCI artifact, but %s", ref.String(), manifest.ArtifactType)
@@ -182,15 +202,7 @@ func (g ociRemoteLoader) pullComposeFiles(ctx context.Context, local string, man
 
 		switch layer.MediaType {
 		case oci.ComposeYAMLMediaType:
-			target := f
-			_, extends := layer.Annotations["com.docker.compose.extends"]
-			if extends {
-				target, err = os.Create(filepath.Join(local, layer.Annotations["com.docker.compose.file"]))
-				if err != nil {
-					return err
-				}
-			}
-			if err := writeComposeFile(layer, i, target, content); err != nil {
+			if err := writeComposeFile(layer, i, local, content); err != nil {
 				return err
 			}
 		case oci.ComposeEnvFileMediaType:
@@ -203,14 +215,25 @@ func (g ociRemoteLoader) pullComposeFiles(ctx context.Context, local string, man
 	return nil
 }
 
-func writeComposeFile(layer spec.Descriptor, i int, f *os.File, content []byte) error {
+func writeComposeFile(layer spec.Descriptor, i int, local string, content []byte) error {
+	file := "compose.yaml"
+	if extends, ok := layer.Annotations["com.docker.compose.extends"]; ok {
+		if err := validatePathInBase(local, extends); err != nil {
+			return err
+		}
+	}
+	f, err := os.Create(filepath.Join(local, file))
+	if err != nil {
+		return err
+	}
+	defer func() { _ = f.Close() }()
 	if _, ok := layer.Annotations["com.docker.compose.file"]; i > 0 && ok {
 		_, err := f.Write([]byte("\n---\n"))
 		if err != nil {
 			return err
 		}
 	}
-	_, err := f.Write(content)
+	_, err = f.Write(content)
 	return err
 }
 
@@ -219,15 +242,16 @@ func writeEnvFile(layer spec.Descriptor, local string, content []byte) error {
 	if !ok {
 		return fmt.Errorf("missing annotation com.docker.compose.envfile in layer %q", layer.Digest)
 	}
-	otherFile, err := os.Create(filepath.Join(local, envfilePath))
-	if err != nil {
+	if err := validatePathInBase(local, envfilePath); err != nil {
 		return err
 	}
-	_, err = otherFile.Write(content)
+	otherFile, err := os.Create(filepath.Join(local, envfilePath))
 	if err != nil {
 		return err
 	}
-	return nil
+	defer func() { _ = otherFile.Close() }()
+	_, err = otherFile.Write(content)
+	return err
 }
 
 var _ loader.ResourceLoader = ociRemoteLoader{}

pkg/remote/oci_test.go

@@ -0,0 +1,125 @@
+/*
+   Copyright 2020 Docker Compose CLI authors
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package remote
+
+import (
+	"path/filepath"
+	"testing"
+)
+
+func TestValidatePathInBase(t *testing.T) {
+	base := "/tmp/cache/compose"
+
+	tests := []struct {
+		name       string
+		unsafePath string
+		wantErr    bool
+	}{
+		{
+			name:       "valid simple filename",
+			unsafePath: "compose.yaml",
+			wantErr:    false,
+		},
+		{
+			name:       "valid hashed filename",
+			unsafePath: "f8f9ede3d201ec37d5a5e3a77bbadab79af26035e53135e19571f50d541d390c.yaml",
+			wantErr:    false,
+		},
+		{
+			name:       "valid env file",
+			unsafePath: ".env",
+			wantErr:    false,
+		},
+		{
+			name:       "valid env file with suffix",
+			unsafePath: ".env.prod",
+			wantErr:    false,
+		},
+		{
+			name:       "unix path traversal",
+			unsafePath: "../../../etc/passwd",
+			wantErr:    true,
+		},
+		{
+			name:       "windows path traversal",
+			unsafePath: "..\\..\\..\\windows\\system32\\config\\sam",
+			wantErr:    true,
+		},
+		{
+			name:       "subdirectory unix",
+			unsafePath: "config/base.yaml",
+			wantErr:    true,
+		},
+		{
+			name:       "subdirectory windows",
+			unsafePath: "config\\base.yaml",
+			wantErr:    true,
+		},
+		{
+			name:       "absolute unix path",
+			unsafePath: "/etc/passwd",
+			wantErr:    true,
+		},
+		{
+			name:       "absolute windows path",
+			unsafePath: "C:\\windows\\system32\\config\\sam",
+			wantErr:    true,
+		},
+		{
+			name:       "parent reference only",
+			unsafePath: "..",
+			wantErr:    true,
+		},
+		{
+			name:       "current directory reference",
+			unsafePath: "./file.yaml",
+			wantErr:    false, // ./ resolves to base dir
+		},
+		{
+			name:       "mixed separators",
+			unsafePath: "config/sub\\file.yaml",
+			wantErr:    true,
+		},
+		{
+			name:       "filename with spaces",
+			unsafePath: "my file.yaml",
+			wantErr:    false,
+		},
+		{
+			name:       "filename with special chars",
+			unsafePath: "file-name_v1.2.3.yaml",
+			wantErr:    false,
+		},
+		{
+			name:       "single parent then back",
+			unsafePath: "../compose/file.yaml",
+			wantErr:    false, // Resolves back to base dir, which is fine
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := validatePathInBase(base, tt.unsafePath)
+			if (err != nil) != tt.wantErr {
+				targetPath := filepath.Join(base, tt.unsafePath)
+				targetDir := filepath.Dir(targetPath)
+				t.Errorf("validatePathInBase(%q, %q) error = %v, wantErr %v\ntargetDir=%q base=%q",
+					base, tt.unsafePath, err, tt.wantErr, targetDir, base)
+			}
+		})
+	}
+}