diff --git a/pkg/cmd/run/download/zip.go b/pkg/cmd/run/download/zip.go index d7a2613bc..bf56ea081 100644 --- a/pkg/cmd/run/download/zip.go +++ b/pkg/cmd/run/download/zip.go @@ -16,14 +16,9 @@ const ( ) func extractZip(zr *zip.Reader, destDir string) error { - destDirAbs, err := filepath.Abs(destDir) - if err != nil { - return err - } - pathPrefix := destDirAbs + string(filepath.Separator) for _, zf := range zr.File { - fpath := filepath.Join(destDirAbs, filepath.FromSlash(zf.Name)) - if !strings.HasPrefix(fpath, pathPrefix) { + fpath := filepath.Join(destDir, filepath.FromSlash(zf.Name)) + if !filepathDescendsFrom(fpath, destDir) { continue } if err := extractZipFile(zf, fpath); err != nil { @@ -67,3 +62,15 @@ func getPerm(m os.FileMode) os.FileMode { } return execMode } + +func filepathDescendsFrom(p, dir string) bool { + p = filepath.Clean(p) + dir = filepath.Clean(dir) + if dir == "." && !filepath.IsAbs(p) { + return !strings.HasPrefix(p, ".."+string(filepath.Separator)) + } + if !strings.HasSuffix(dir, string(filepath.Separator)) { + dir += string(filepath.Separator) + } + return strings.HasPrefix(p, dir) +} diff --git a/pkg/cmd/run/download/zip_test.go b/pkg/cmd/run/download/zip_test.go index f859511ee..97861b183 100644 --- a/pkg/cmd/run/download/zip_test.go +++ b/pkg/cmd/run/download/zip_test.go @@ -30,3 +30,119 @@ func Test_extractZip(t *testing.T) { _, err = os.Stat(filepath.Join("src", "main.go")) require.NoError(t, err) } + +func Test_filepathDescendsFrom(t *testing.T) { + type args struct { + p string + dir string + } + tests := []struct { + name string + args args + want bool + }{ + { + name: "root child", + args: args{ + p: filepath.FromSlash("/hoi.txt"), + dir: filepath.FromSlash("/"), + }, + want: true, + }, + { + name: "abs descendant", + args: args{ + p: filepath.FromSlash("/var/logs/hoi.txt"), + dir: filepath.FromSlash("/"), + }, + want: true, + }, + { + name: "abs trailing slash", + args: args{ + p: filepath.FromSlash("/var/logs/hoi.txt"), + dir: filepath.FromSlash("/var/logs/"), + }, + want: true, + }, + { + name: "abs mismatch", + args: args{ + p: filepath.FromSlash("/var/logs/hoi.txt"), + dir: filepath.FromSlash("/var/pids"), + }, + want: false, + }, + { + name: "abs partial prefix", + args: args{ + p: filepath.FromSlash("/var/logs/hoi.txt"), + dir: filepath.FromSlash("/var/log"), + }, + want: false, + }, + { + name: "rel child", + args: args{ + p: filepath.FromSlash("hoi.txt"), + dir: filepath.FromSlash("."), + }, + want: true, + }, + { + name: "rel descendant", + args: args{ + p: filepath.FromSlash("./log/hoi.txt"), + dir: filepath.FromSlash("."), + }, + want: true, + }, + { + name: "mixed rel styles", + args: args{ + p: filepath.FromSlash("./log/hoi.txt"), + dir: filepath.FromSlash("log"), + }, + want: true, + }, + { + name: "rel clean", + args: args{ + p: filepath.FromSlash("cats/../dogs/pug.txt"), + dir: filepath.FromSlash("dogs"), + }, + want: true, + }, + { + name: "rel mismatch", + args: args{ + p: filepath.FromSlash("dogs/pug.txt"), + dir: filepath.FromSlash("dog"), + }, + want: false, + }, + { + name: "rel breakout", + args: args{ + p: filepath.FromSlash("../escape.txt"), + dir: filepath.FromSlash("."), + }, + want: false, + }, + { + name: "rel sneaky breakout", + args: args{ + p: filepath.FromSlash("dogs/../../escape.txt"), + dir: filepath.FromSlash("dogs"), + }, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := filepathDescendsFrom(tt.args.p, tt.args.dir); got != tt.want { + t.Errorf("filepathDescendsFrom() = %v, want %v", got, tt.want) + } + }) + } +}