source: extract SafeFileName into shared pathutil package

Move safeFileName from source/http to source/util/pathutil
and apply it to the containerblob source as well. Harden
containerblob/pull.go to use os.OpenRoot for file writes,
preventing path traversal via crafted filenames.

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
(cherry picked from commit 3d6e587655d72c343f6fdc7268480a900ba45b0c)
(cherry picked from commit 45a6358b084f95ca715376935759c432840bc7bf)

Author: tonistiigi

source/containerblob/pull.go

@@ -7,7 +7,6 @@ import (
 	"fmt"
 	"io"
 	"os"
-	"path/filepath"
 	"time"
 
 	"github.com/containerd/containerd/v2/core/remotes"
@@ -18,6 +17,7 @@ import (
 	"github.com/moby/buildkit/snapshot"
 	"github.com/moby/buildkit/solver"
 	srctypes "github.com/moby/buildkit/source/types"
+	"github.com/moby/buildkit/source/util/pathutil"
 	"github.com/moby/buildkit/util/contentutil"
 	"github.com/moby/buildkit/util/iohelper"
 	"github.com/moby/buildkit/util/resolver"
@@ -225,10 +225,17 @@ func (p *puller) Snapshot(ctx context.Context, jobCtx solver.JobContext) (ir cac
 	fn := p.id.Filename
 	if fn == "" {
 		fn = p.dgst.Hex()
+	} else {
+		fn = pathutil.SafeFileName(fn)
+	}
+
+	root, err := os.OpenRoot(dir)
+	if err != nil {
+		return nil, err
 	}
+	defer root.Close()
 
-	fp := filepath.Join(dir, fn)
-	f, err := os.OpenFile(fp, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, os.FileMode(perm))
+	f, err := root.OpenFile(fn, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, os.FileMode(perm))
 	if err != nil {
 		return nil, err
 	}
@@ -257,13 +264,13 @@ func (p *puller) Snapshot(ctx context.Context, jobCtx solver.JobContext) (ir cac
 		}
 	}
 	if gid != 0 || uid != 0 {
-		if err := os.Chown(fp, uid, gid); err != nil {
+		if err := root.Chown(fn, uid, gid); err != nil {
 			return nil, err
 		}
 	}
 
 	mTime := time.Unix(0, 0)
-	if err := os.Chtimes(fp, mTime, mTime); err != nil {
+	if err := root.Chtimes(fn, mTime, mTime); err != nil {
 		return nil, err
 	}
 

source/containerblob/source.go

@@ -13,6 +13,7 @@ import (
 	"github.com/moby/buildkit/solver/pb"
 	"github.com/moby/buildkit/source"
 	srctypes "github.com/moby/buildkit/source/types"
+	"github.com/moby/buildkit/source/util/pathutil"
 	"github.com/pkg/errors"
 )
 
@@ -86,7 +87,7 @@ func (is *Source) parseIdentifierAttrs(id *ImageBlobIdentifier, attrs map[string
 	for k, v := range attrs {
 		switch k {
 		case pb.AttrHTTPFilename:
-			id.Filename = v
+			id.Filename = pathutil.SafeFileName(v)
 		case pb.AttrHTTPPerm:
 			i, err := strconv.ParseInt(v, 0, 64)
 			if err != nil {

source/http/source.go

@@ -18,7 +18,6 @@ import (
 	"strconv"
 	"strings"
 	"time"
-	"unicode"
 
 	"github.com/moby/buildkit/cache"
 	"github.com/moby/buildkit/session"
@@ -28,6 +27,7 @@ import (
 	"github.com/moby/buildkit/solver/pb"
 	"github.com/moby/buildkit/source"
 	srctypes "github.com/moby/buildkit/source/types"
+	"github.com/moby/buildkit/source/util/pathutil"
 	"github.com/moby/buildkit/util/bklog"
 	"github.com/moby/buildkit/util/cachedigest"
 	"github.com/moby/buildkit/util/pgpsign"
@@ -963,30 +963,16 @@ func (hs *httpSourceHandler) newHTTPRequest(ctx context.Context, g session.Group
 	return req, nil
 }
 
-func safeFileName(s string) string {
-	defaultName := "download"
-	name := filepath.Base(filepath.FromSlash(strings.TrimSpace(s)))
-	if name == "" || name == "." || name == ".." {
-		return defaultName
-	}
-	for _, r := range name {
-		if r == 0 || unicode.IsControl(r) {
-			return defaultName
-		}
-	}
-	return name
-}
-
 func getFileName(urlStr, manualFilename string, resp *http.Response) string {
 	if manualFilename != "" {
-		return safeFileName(manualFilename)
+		return pathutil.SafeFileName(manualFilename)
 	}
 	if resp != nil {
 		if contentDisposition := resp.Header.Get("Content-Disposition"); contentDisposition != "" {
 			if _, params, err := mime.ParseMediaType(contentDisposition); err == nil {
 				if params["filename"] != "" && !strings.HasSuffix(params["filename"], "/") {
 					if filename := filepath.Base(filepath.FromSlash(params["filename"])); filename != "" {
-						return safeFileName(filename)
+						return pathutil.SafeFileName(filename)
 					}
 				}
 			}
@@ -995,10 +981,10 @@ func getFileName(urlStr, manualFilename string, resp *http.Response) string {
 	u, err := url.Parse(urlStr)
 	if err == nil {
 		if base := path.Base(u.Path); base != "." && base != "/" {
-			return safeFileName(base)
+			return pathutil.SafeFileName(base)
 		}
 	}
-	return safeFileName("")
+	return pathutil.SafeFileName("")
 }
 
 func searchHTTPURLDigest(ctx context.Context, store cache.MetadataStore, dgst digest.Digest) ([]cacheRefMetadata, error) {

source/http/source_test.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"os"
 	"path/filepath"
-	"runtime"
 	"testing"
 
 	"github.com/containerd/containerd/v2/core/diff/apply"
@@ -32,50 +31,6 @@ import (
 
 const signFixturesPathEnv = "BUILDKIT_TEST_SIGN_FIXTURES"
 
-func TestSafeFileName(t *testing.T) {
-	t.Parallel()
-
-	type testCase struct {
-		name string
-		in   string
-		want string
-	}
-
-	tests := []testCase{
-		{name: "simple", in: "foo", want: "foo"},
-		{name: "simple_ext", in: "foo.txt", want: "foo.txt"},
-		{name: "unicode_cjk", in: "資料.txt", want: "資料.txt"},
-		{name: "unicode_cyrillic", in: "тест-файл", want: "тест-файл"},
-		{name: "spaces_allowed", in: "name with spaces.txt", want: "name with spaces.txt"},
-		{name: "trim_outer_whitespace", in: "  foo.txt  ", want: "foo.txt"},
-		{name: "unix_path", in: "a/b/c.txt", want: "c.txt"},
-		{name: "empty", in: "", want: "download"},
-		{name: "dot", in: ".", want: "download"},
-		{name: "dot_dot", in: "..", want: "download"},
-		{name: "traversal_unix", in: "../", want: "download"},
-		{name: "nul_byte", in: "a\x00b", want: "download"},
-		{name: "control", in: "a\nb", want: "download"},
-	}
-	if runtime.GOOS == "windows" {
-		tests = append(tests,
-			testCase{name: "windows_traversal", in: "..\\", want: "download"},
-			testCase{name: "windows_path_basename", in: "a\\b\\c.txt", want: "c.txt"},
-		)
-	} else {
-		tests = append(tests,
-			testCase{name: "windows_traversal_literal", in: "..\\", want: "..\\"},
-			testCase{name: "windows_path_literal", in: "a\\b\\c.txt", want: "a\\b\\c.txt"},
-		)
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-			require.Equal(t, tt.want, safeFileName(tt.in))
-		})
-	}
-}
-
 func TestHTTPSource(t *testing.T) {
 	t.Parallel()
 	ctx := context.TODO()

source/util/pathutil/pathutil.go

@@ -0,0 +1,21 @@
+package pathutil
+
+import (
+	"path/filepath"
+	"strings"
+	"unicode"
+)
+
+func SafeFileName(s string) string {
+	defaultName := "download"
+	name := filepath.Base(filepath.FromSlash(strings.TrimSpace(s)))
+	if name == "" || name == "." || name == ".." {
+		return defaultName
+	}
+	for _, r := range name {
+		if r == 0 || unicode.IsControl(r) {
+			return defaultName
+		}
+	}
+	return name
+}

source/util/pathutil/pathutil_test.go

@@ -0,0 +1,52 @@
+package pathutil
+
+import (
+	"runtime"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestSafeFileName(t *testing.T) {
+	t.Parallel()
+
+	type testCase struct {
+		name string
+		in   string
+		want string
+	}
+
+	tests := []testCase{
+		{name: "simple", in: "foo", want: "foo"},
+		{name: "simple_ext", in: "foo.txt", want: "foo.txt"},
+		{name: "unicode_cjk", in: "資料.txt", want: "資料.txt"},
+		{name: "unicode_cyrillic", in: "тест-файл", want: "тест-файл"},
+		{name: "spaces_allowed", in: "name with spaces.txt", want: "name with spaces.txt"},
+		{name: "trim_outer_whitespace", in: "  foo.txt  ", want: "foo.txt"},
+		{name: "unix_path", in: "a/b/c.txt", want: "c.txt"},
+		{name: "empty", in: "", want: "download"},
+		{name: "dot", in: ".", want: "download"},
+		{name: "dot_dot", in: "..", want: "download"},
+		{name: "traversal_unix", in: "../", want: "download"},
+		{name: "nul_byte", in: "a\x00b", want: "download"},
+		{name: "control", in: "a\nb", want: "download"},
+	}
+	if runtime.GOOS == "windows" {
+		tests = append(tests,
+			testCase{name: "windows_traversal", in: "..\\", want: "download"},
+			testCase{name: "windows_path_basename", in: "a\\b\\c.txt", want: "c.txt"},
+		)
+	} else {
+		tests = append(tests,
+			testCase{name: "windows_traversal_literal", in: "..\\", want: "..\\"},
+			testCase{name: "windows_path_literal", in: "a\\b\\c.txt", want: "a\\b\\c.txt"},
+		)
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			require.Equal(t, tt.want, SafeFileName(tt.in))
+		})
+	}
+}