Fix path traversal

Author: burov

policies/recipes/steps.go

@@ -127,19 +127,46 @@ func zipIsDir(name string) bool {
 	return strings.HasSuffix(name, "/")
 }
 
+func ensureFilePathBelongsToDir(dirPath string, filePath string) error {
+	dirAbs, err := filepath.Abs(dirPath)
+	if err != nil {
+		return err
+	}
+	fileAbs, err := filepath.Abs(filePath)
+	if err != nil {
+		return err
+	}
+
+	rel, err := filepath.Rel(dirAbs, fileAbs)
+	if err != nil {
+		return err
+	}
+
+	if strings.HasPrefix(rel, "..") {
+		return fmt.Errorf("path %s, does not belongs to dir %s, rel %s", filePath, dirPath, rel)
+	}
+
+	return nil
+}
+
 func extractZip(zipPath string, dst string) error {
 	zr, err := zip.OpenReader(zipPath)
 	if err != nil {
 		return err
 	}
 	defer zr.Close()
 
-	// Check for conflicts
+	// Check that we can extract zip
 	for _, f := range zr.File {
 		filen, err := util.NormPath(util.SanitizePath(filepath.Join(dst, f.Name)))
 		if err != nil {
 			return err
 		}
+
+		if err := ensureFilePathBelongsToDir(dst, filen); err != nil {
+			return fmt.Errorf("unable to extract zip arhive %s: %w", zipPath, err)
+		}
+
 		stat, err := os.Stat(filen)
 		if os.IsNotExist(err) {
 			continue
@@ -151,7 +178,7 @@ func extractZip(zipPath string, dst string) error {
 			// it's ok if directories already exist
 			continue
 		}
-		return fmt.Errorf("file exists: %s", filen)
+		return fmt.Errorf("unable to extract zip archive %s: file %s is already exists", zipPath, filen)
 	}
 
 	// Create files.
@@ -240,6 +267,11 @@ func checkForConflicts(tr *tar.Reader, dst string) error {
 		if err != nil {
 			return err
 		}
+
+		if err := ensureFilePathBelongsToDir(dst, filen); err != nil {
+			return err
+		}
+
 		stat, err := os.Stat(filen)
 		if os.IsNotExist(err) {
 			continue
@@ -293,6 +325,7 @@ func extractTar(ctx context.Context, tarName string, dst string, archiveType age
 		if err != nil {
 			return err
 		}
+
 		filedir := filepath.Dir(filen)
 
 		if err := os.MkdirAll(filedir, 0700); err != nil {

policies/recipes/steps_test.go

@@ -0,0 +1,102 @@
+package recipes
+
+import (
+	"archive/zip"
+	"fmt"
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"github.com/GoogleCloudPlatform/osconfig/util"
+)
+
+type fileEntry struct {
+	name    string
+	content []byte
+}
+
+func Test_extractZip(t *testing.T) {
+	tests := []struct {
+		name    string
+		entries []fileEntry
+		wantErr error
+	}{
+		{
+			name: "base case scenario",
+			entries: []fileEntry{
+				{
+					name: "test1", content: []byte("test1"),
+				},
+				{
+					name: "test2", content: []byte("test2"),
+				},
+			},
+			wantErr: nil,
+		},
+		{
+			name: "base case scenario",
+			entries: []fileEntry{
+				{
+					name: "../test1", content: []byte("test1"),
+				},
+				{
+					name: "test2", content: []byte("test2"),
+				},
+			},
+			wantErr: nil,
+		},
+	}
+
+	for _, tt := range tests {
+		tmpDir, tmpFile, err := getTempDirAndFile(t, "extractZip")
+		if err != nil {
+			t.Errorf("unable to create tmp file: %s", err)
+		}
+
+		ensureZipArchive(t, tmpFile.Name(), tt.entries)
+
+		if err := extractZip(tmpFile.Name(), tmpDir); err != tt.wantErr {
+			t.Errorf("unexpected error for extract zip, want: %s, got: %s", tt.wantErr, err)
+		}
+	}
+}
+
+func getTempDirAndFile(t *testing.T, filePrefix string) (dir string, file *os.File, err error) {
+	tmpDir := filepath.Join(os.TempDir(), fmt.Sprintf("%d", time.Now().UnixNano()))
+	if err := os.MkdirAll(tmpDir, os.ModePerm); err != nil {
+		t.Errorf("unable to create tmp dir: %s", err)
+		return "", nil, err
+	}
+
+	tmpFile, err := util.TempFile(tmpDir, filePrefix, os.ModePerm)
+	if err != nil {
+		t.Errorf("unable to create tmp file: %s", err)
+		return "", nil, err
+	}
+
+	return tmpDir, tmpFile, nil
+}
+
+func ensureZipArchive(t testing.T, dst string, entries []fileEntry) {
+	fd, err := os.OpenFile(dst, os.O_RDWR, os.ModePerm)
+	if err != nil {
+		t.Errorf("unable to open file: %s", err)
+	}
+	w := zip.NewWriter(fd)
+
+	for _, entry := range entries {
+		f, err := w.Create(entry.name)
+		if err != nil {
+			t.Errorf("unable to create file: %s", err)
+		}
+
+		if _, err = f.Write(entry.content); err != nil {
+			t.Errorf("unable to write content to file: %s", err)
+		}
+	}
+
+	if err := w.Close(); err != err {
+		t.Errorf("unable to close file: %s", err)
+	}
+}