Skip to content

Commit 9ce6f62

Browse files
tonistiigicrazy-max
authored andcommitted
source/http: sanitize downloaded filenames
Add safeFileName and route all getFileName sources through it. Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com> (cherry picked from commit 9d117af5ab1e1032f75658884384328fea440843) (cherry picked from commit ee4de4c2aa53a76fb2ba135cfcb2daa8e45c5b80)
1 parent 099cf80 commit 9ce6f62

File tree

2 files changed

+64
-4
lines changed

2 files changed

+64
-4
lines changed

source/http/source.go

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"strconv"
1919
"strings"
2020
"time"
21+
"unicode"
2122

2223
"github.com/moby/buildkit/cache"
2324
"github.com/moby/buildkit/session"
@@ -955,16 +956,30 @@ func (hs *httpSourceHandler) newHTTPRequest(ctx context.Context, g session.Group
955956
return req, nil
956957
}
957958

959+
func safeFileName(s string) string {
960+
defaultName := "download"
961+
name := filepath.Base(filepath.FromSlash(strings.TrimSpace(s)))
962+
if name == "" || name == "." || name == ".." {
963+
return defaultName
964+
}
965+
for _, r := range name {
966+
if r == 0 || unicode.IsControl(r) {
967+
return defaultName
968+
}
969+
}
970+
return name
971+
}
972+
958973
func getFileName(urlStr, manualFilename string, resp *http.Response) string {
959974
if manualFilename != "" {
960-
return manualFilename
975+
return safeFileName(manualFilename)
961976
}
962977
if resp != nil {
963978
if contentDisposition := resp.Header.Get("Content-Disposition"); contentDisposition != "" {
964979
if _, params, err := mime.ParseMediaType(contentDisposition); err == nil {
965980
if params["filename"] != "" && !strings.HasSuffix(params["filename"], "/") {
966981
if filename := filepath.Base(filepath.FromSlash(params["filename"])); filename != "" {
967-
return filename
982+
return safeFileName(filename)
968983
}
969984
}
970985
}
@@ -973,10 +988,10 @@ func getFileName(urlStr, manualFilename string, resp *http.Response) string {
973988
u, err := url.Parse(urlStr)
974989
if err == nil {
975990
if base := path.Base(u.Path); base != "." && base != "/" {
976-
return base
991+
return safeFileName(base)
977992
}
978993
}
979-
return "download"
994+
return safeFileName("")
980995
}
981996

982997
func searchHTTPURLDigest(ctx context.Context, store cache.MetadataStore, dgst digest.Digest) ([]cacheRefMetadata, error) {

source/http/source_test.go

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"os"
66
"path/filepath"
7+
"runtime"
78
"testing"
89

910
"github.com/containerd/containerd/v2/core/diff/apply"
@@ -31,6 +32,50 @@ import (
3132

3233
const signFixturesPathEnv = "BUILDKIT_TEST_SIGN_FIXTURES"
3334

35+
func TestSafeFileName(t *testing.T) {
36+
t.Parallel()
37+
38+
type testCase struct {
39+
name string
40+
in string
41+
want string
42+
}
43+
44+
tests := []testCase{
45+
{name: "simple", in: "foo", want: "foo"},
46+
{name: "simple_ext", in: "foo.txt", want: "foo.txt"},
47+
{name: "unicode_cjk", in: "資料.txt", want: "資料.txt"},
48+
{name: "unicode_cyrillic", in: "тест-файл", want: "тест-файл"},
49+
{name: "spaces_allowed", in: "name with spaces.txt", want: "name with spaces.txt"},
50+
{name: "trim_outer_whitespace", in: " foo.txt ", want: "foo.txt"},
51+
{name: "unix_path", in: "a/b/c.txt", want: "c.txt"},
52+
{name: "empty", in: "", want: "download"},
53+
{name: "dot", in: ".", want: "download"},
54+
{name: "dot_dot", in: "..", want: "download"},
55+
{name: "traversal_unix", in: "../", want: "download"},
56+
{name: "nul_byte", in: "a\x00b", want: "download"},
57+
{name: "control", in: "a\nb", want: "download"},
58+
}
59+
if runtime.GOOS == "windows" {
60+
tests = append(tests,
61+
testCase{name: "windows_traversal", in: "..\\", want: "download"},
62+
testCase{name: "windows_path_basename", in: "a\\b\\c.txt", want: "c.txt"},
63+
)
64+
} else {
65+
tests = append(tests,
66+
testCase{name: "windows_traversal_literal", in: "..\\", want: "..\\"},
67+
testCase{name: "windows_path_literal", in: "a\\b\\c.txt", want: "a\\b\\c.txt"},
68+
)
69+
}
70+
71+
for _, tt := range tests {
72+
t.Run(tt.name, func(t *testing.T) {
73+
t.Parallel()
74+
require.Equal(t, tt.want, safeFileName(tt.in))
75+
})
76+
}
77+
}
78+
3479
func TestHTTPSource(t *testing.T) {
3580
t.Parallel()
3681
ctx := context.TODO()

0 commit comments

Comments
 (0)