diff --git a/pkg/cmd/run/download/download.go b/pkg/cmd/run/download/download.go index 99ec45bbe..168cb6fcc 100644 --- a/pkg/cmd/run/download/download.go +++ b/pkg/cmd/run/download/download.go @@ -169,6 +169,11 @@ func runDownload(opts *DownloadOptions) error { if len(wantPatterns) != 0 || len(wantNames) != 1 { destDir = filepath.Join(destDir, a.Name) } + + if !filepathDescendsFrom(destDir, opts.DestinationDir) { + return fmt.Errorf("error downloading %s: would result in path traversal", a.Name) + } + err := opts.Platform.Download(a.DownloadURL, destDir) if err != nil { return fmt.Errorf("error downloading %s: %w", a.Name, err) diff --git a/pkg/cmd/run/download/download_test.go b/pkg/cmd/run/download/download_test.go index 3c1c8f2d8..f07d66128 100644 --- a/pkg/cmd/run/download/download_test.go +++ b/pkg/cmd/run/download/download_test.go @@ -289,6 +289,108 @@ func Test_runDownload(t *testing.T) { }) }, }, + { + name: "given artifact name contains `..`, verify an error about path traversal is returned", + opts: DownloadOptions{ + RunID: "2345", + DestinationDir: ".", + }, + mockAPI: func(p *mockPlatform) { + p.On("List", "2345").Return([]shared.Artifact{ + { + Name: "..", + DownloadURL: "http://download.com/artifact1.zip", + Expired: false, + }, + }, nil) + }, + wantErr: "error downloading ..: would result in path traversal", + }, + { + name: "given artifact name contains `..`, verify an error about path traversal is returned", + opts: DownloadOptions{ + RunID: "2345", + DestinationDir: "imaginary-dir", + }, + mockAPI: func(p *mockPlatform) { + p.On("List", "2345").Return([]shared.Artifact{ + { + Name: "..", + DownloadURL: "http://download.com/artifact1.zip", + Expired: false, + }, + }, nil) + }, + wantErr: "error downloading ..: would result in path traversal", + }, + { + name: "given artifact name contains `../etc/passwd`, verify an error about path traversal is returned", + opts: DownloadOptions{ + RunID: "2345", + DestinationDir: ".", + }, + mockAPI: func(p *mockPlatform) { + p.On("List", "2345").Return([]shared.Artifact{ + { + Name: "../etc/passwd", + DownloadURL: "http://download.com/artifact1.zip", + Expired: false, + }, + }, nil) + }, + wantErr: "error downloading ../etc/passwd: would result in path traversal", + }, + { + name: "given artifact name contains `../etc/passwd`, verify an error about path traversal is returned", + opts: DownloadOptions{ + RunID: "2345", + DestinationDir: "imaginary-dir", + }, + mockAPI: func(p *mockPlatform) { + p.On("List", "2345").Return([]shared.Artifact{ + { + Name: "../etc/passwd", + DownloadURL: "http://download.com/artifact1.zip", + Expired: false, + }, + }, nil) + }, + wantErr: "error downloading ../etc/passwd: would result in path traversal", + }, + { + name: "given artifact name contains `../../etc/passwd`, verify an error about path traversal is returned", + opts: DownloadOptions{ + RunID: "2345", + DestinationDir: ".", + }, + mockAPI: func(p *mockPlatform) { + p.On("List", "2345").Return([]shared.Artifact{ + { + Name: "../../etc/passwd", + DownloadURL: "http://download.com/artifact1.zip", + Expired: false, + }, + }, nil) + }, + wantErr: "error downloading ../../etc/passwd: would result in path traversal", + }, + { + name: "given artifact name contains `../../etc/passwd`, verify an error about path traversal is returned", + opts: DownloadOptions{ + RunID: "2345", + DestinationDir: "imaginary-dir", + }, + mockAPI: func(p *mockPlatform) { + p.On("List", "2345").Return([]shared.Artifact{ + { + Name: "../../etc/passwd", + DownloadURL: "http://download.com/artifact1.zip", + Expired: false, + }, + }, nil) + }, + wantErr: "error downloading ../../etc/passwd: would result in path traversal", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/cmd/run/download/zip.go b/pkg/cmd/run/download/zip.go index ab5723e94..f6a27afdd 100644 --- a/pkg/cmd/run/download/zip.go +++ b/pkg/cmd/run/download/zip.go @@ -73,6 +73,9 @@ func getPerm(m os.FileMode) os.FileMode { func filepathDescendsFrom(p, dir string) bool { p = filepath.Clean(p) dir = filepath.Clean(dir) + if dir == "." && p == ".." { + return false + } if dir == "." && !filepath.IsAbs(p) { return !strings.HasPrefix(p, ".."+string(filepath.Separator)) }