Merge pull request #6910 from tonistiigi/platforms-id-sanitize

exporter: sanitize platform IDs in paths

Author: tonistiigi

client/client_test.go

@@ -23,6 +23,7 @@ import (
 	"path"
 	"path/filepath"
 	"runtime"
+	"sort"
 	"strconv"
 	"strings"
 	"sync"
@@ -246,6 +247,7 @@ var allTests = []func(t *testing.T, sb integration.Sandbox){
 	testExportLocalNoPlatformSplit,
 	testExportLocalNoPlatformSplitOverwrite,
 	testExportLocalForcePlatformSplit,
+	testExportTarPlatformIDSanitized,
 	testExportLocalModeCopyKeepsStaleDestinationFiles,
 	testExportLocalModeDeleteRemovesStaleDestinationFiles,
 	testExportLocalModeCopyMultiPlatformKeepsAllPlatforms,
@@ -8093,6 +8095,90 @@ func testExportLocalForcePlatformSplit(t *testing.T, sb integration.Sandbox) {
 	require.Equal(t, "hello", string(dt))
 }
 
+func testExportTarPlatformIDSanitized(t *testing.T, sb integration.Sandbox) {
+	workers.CheckFeatureCompat(t, sb, workers.FeatureOCIExporter, workers.FeatureMultiPlatform)
+	c, err := New(sb.Context(), sb.Address())
+	require.NoError(t, err)
+	defer c.Close()
+
+	const platformID = `..\buildkit-outside`
+	platform := platforms.DefaultSpec()
+
+	frontend := func(ctx context.Context, c gateway.Client) (*gateway.Result, error) {
+		st := llb.Scratch().File(
+			llb.Mkfile("payload.txt", 0600, []byte("payload")),
+		)
+
+		def, err := st.Marshal(ctx)
+		if err != nil {
+			return nil, err
+		}
+
+		r, err := c.Solve(ctx, gateway.SolveRequest{
+			Definition: def.ToPB(),
+		})
+		if err != nil {
+			return nil, err
+		}
+
+		ref, err := r.SingleRef()
+		if err != nil {
+			return nil, err
+		}
+
+		res := gateway.NewResult()
+		res.AddRef(platformID, ref)
+
+		dt, err := json.Marshal(&exptypes.Platforms{
+			Platforms: []exptypes.Platform{{
+				ID:       platformID,
+				Platform: platform,
+			}},
+		})
+		if err != nil {
+			return nil, err
+		}
+		res.AddMeta(exptypes.ExporterPlatformsKey, dt)
+
+		return res, nil
+	}
+
+	outW := bytes.NewBuffer(nil)
+	_, err = c.Build(sb.Context(), SolveOpt{
+		Exports: []ExportEntry{
+			{
+				Type:   ExporterTar,
+				Output: fixedWriteCloser(&iohelper.NopWriteCloser{Writer: outW}),
+			},
+		},
+	}, "", frontend, nil)
+	require.NoError(t, err)
+
+	m, err := testutil.ReadTarToMap(outW.Bytes(), false)
+	require.NoError(t, err)
+
+	for name := range m {
+		require.Falsef(t, strings.HasPrefix(name, "../") ||
+			strings.HasPrefix(name, `..\`) ||
+			strings.Contains(name, `/../`) ||
+			strings.Contains(name, `\..\`) ||
+			strings.Contains(name, `\`) ||
+			strings.Contains(name, ":"),
+			"tar exporter emitted unsafe platform path %q", name)
+	}
+
+	tarPaths := make([]string, 0, len(m))
+	for name := range m {
+		tarPaths = append(tarPaths, name)
+	}
+	sort.Strings(tarPaths)
+
+	expectedPath := path.Join(".._buildkit-outside", integration.UnixOrWindows("payload.txt", "Files/payload.txt"))
+	item := m[expectedPath]
+	require.NotNilf(t, item, "expected sanitized tar path %q in %v", expectedPath, tarPaths)
+	require.Equal(t, "payload", string(item.Data))
+}
+
 func testExportLocalModeCopyKeepsStaleDestinationFiles(t *testing.T, sb integration.Sandbox) {
 	c, err := New(sb.Context(), sb.Address())
 	require.NoError(t, err)

exporter/local/export.go

@@ -3,7 +3,6 @@ package local
 import (
 	"context"
 	"os"
-	"strings"
 	"sync"
 	"time"
 
@@ -127,7 +126,7 @@ func (e *localExporterInstance) Export(ctx context.Context, inp *exporter.Source
 	platformDirStat := func(k string, opt CreateFSOpts) *fstypes.Stat {
 		st := &fstypes.Stat{
 			Mode: uint32(os.ModeDir | 0755),
-			Path: strings.ReplaceAll(k, "/", "_"),
+			Path: PlatformIDToPath(k),
 		}
 		if opt.Epoch != nil && opt.Epoch.Value != nil {
 			st.ModTime = opt.Epoch.Value.UnixNano()

exporter/local/fs.go

@@ -199,7 +199,7 @@ func CreateFS(ctx context.Context, sessionID string, k string, ref cache.Immutab
 			if addPlatformToFilename {
 				nameExt := path.Ext(name)
 				namBase := strings.TrimSuffix(name, nameExt)
-				name = fmt.Sprintf("%s.%s%s", namBase, strings.ReplaceAll(k, "/", "_"), nameExt)
+				name = fmt.Sprintf("%s.%s%s", namBase, PlatformIDToPath(k), nameExt)
 			}
 			if _, ok := names[name]; ok {
 				return nil, nil, errors.Errorf("duplicate attestation path name %s", name)

exporter/local/platform.go

@@ -0,0 +1,14 @@
+package local
+
+import "strings"
+
+// PlatformIDToPath maps an exporter platform ID to a single path component.
+func PlatformIDToPath(id string) string {
+	id = strings.NewReplacer("/", "_", `\`, "_", ":", "_").Replace(id)
+	switch id {
+	case ".", "..":
+		return strings.Repeat("_", len(id))
+	default:
+		return id
+	}
+}

exporter/local/platform_test.go

@@ -0,0 +1,50 @@
+package local
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestPlatformIDToPath(t *testing.T) {
+	for _, tc := range []struct {
+		name string
+		id   string
+		want string
+	}{
+		{
+			name: "slashes",
+			id:   "linux/amd64",
+			want: "linux_amd64",
+		},
+		{
+			name: "windows separators",
+			id:   `..\buildkit-outside`,
+			want: ".._buildkit-outside",
+		},
+		{
+			name: "drive separator",
+			id:   `windows/amd64:C`,
+			want: "windows_amd64_C",
+		},
+		{
+			name: "dot",
+			id:   ".",
+			want: "_",
+		},
+		{
+			name: "dot dot",
+			id:   "..",
+			want: "__",
+		},
+		{
+			name: "embedded dots",
+			id:   "linux.v2/amd64",
+			want: "linux.v2_amd64",
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			require.Equal(t, tc.want, PlatformIDToPath(tc.id))
+		})
+	}
+}

exporter/tar/export.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"os"
 	"slices"
-	"strings"
 	"time"
 
 	"github.com/moby/buildkit/cache"
@@ -109,7 +108,7 @@ func (e *localExporterInstance) Export(ctx context.Context, inp *exporter.Source
 
 		st := &fstypes.Stat{
 			Mode: uint32(os.ModeDir | 0755),
-			Path: strings.ReplaceAll(k, "/", "_"),
+			Path: local.PlatformIDToPath(k),
 		}
 		if opt.Epoch != nil && opt.Epoch.Value != nil {
 			st.ModTime = opt.Epoch.Value.UnixNano()