Merge pull request cli#5225 from cli/extract-to-root

mislav · web-flow · commit 07cd7496c5e4 · 2022-03-29T16:16:34.000+02:00
run download: fix extracting to root path
diff --git 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
@@ -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)
+			}
+		})
+	}
+}
