Harden unzip against symlink chain escapes

Author: cursoragent

server/lib/ziputil/ziputil.go

@@ -108,11 +108,20 @@ func Unzip(zipFilePath, destDir string) error {
 
 	// Extract each file
 	for _, file := range reader.File {
+		entryPath := filepath.FromSlash(file.Name)
+
 		// Create the full destination path
-		destPath := filepath.Join(destDir, file.Name)
+		destPath := filepath.Join(cleanDestDir, entryPath)
 
 		// Check for directory traversal vulnerabilities
-		if !strings.HasPrefix(destPath, cleanDestDir+string(os.PathSeparator)) {
+		if !isPathWithinDir(cleanDestDir, destPath) {
+			return fmt.Errorf("illegal file path: %s", file.Name)
+		}
+		resolvedDestPath, err := resolvePathWithSymlinks(cleanDestDir, entryPath)
+		if err != nil {
+			return fmt.Errorf("failed to resolve destination path %s: %w", file.Name, err)
+		}
+		if !isPathWithinDir(cleanDestDir, resolvedDestPath) {
 			return fmt.Errorf("illegal file path: %s", file.Name)
 		}
 
@@ -143,8 +152,15 @@ func Unzip(zipFilePath, destDir string) error {
 			}
 			targetPath := string(target)
 			if !filepath.IsAbs(targetPath) {
-				resolvedTarget := filepath.Clean(filepath.Join(filepath.Dir(destPath), targetPath))
-				if resolvedTarget != cleanDestDir && !strings.HasPrefix(resolvedTarget, cleanDestDir+string(os.PathSeparator)) {
+				resolvedParentPath, err := resolvePathWithSymlinks(cleanDestDir, filepath.Dir(entryPath))
+				if err != nil {
+					return fmt.Errorf("failed to resolve symlink parent path: %w", err)
+				}
+				resolvedTarget, err := resolvePathWithSymlinks(resolvedParentPath, targetPath)
+				if err != nil {
+					return fmt.Errorf("failed to resolve symlink target: %w", err)
+				}
+				if !isPathWithinDir(cleanDestDir, resolvedTarget) {
 					return fmt.Errorf("illegal symlink target: %s -> %s", file.Name, targetPath)
 				}
 			}
@@ -172,3 +188,34 @@ func Unzip(zipFilePath, destDir string) error {
 
 	return nil
 }
+
+func isPathWithinDir(baseDir, path string) bool {
+	return path == baseDir || strings.HasPrefix(path, baseDir+string(os.PathSeparator))
+}
+
+func resolvePathWithSymlinks(baseDir, relPath string) (string, error) {
+	currentPath := filepath.Clean(baseDir)
+	for _, part := range strings.Split(filepath.FromSlash(relPath), string(os.PathSeparator)) {
+		switch part {
+		case "", ".":
+			continue
+		case "..":
+			currentPath = filepath.Dir(currentPath)
+			continue
+		}
+
+		nextPath := filepath.Join(currentPath, part)
+		resolvedPath, err := filepath.EvalSymlinks(nextPath)
+		if err == nil {
+			currentPath = resolvedPath
+			continue
+		}
+		if !os.IsNotExist(err) {
+			return "", fmt.Errorf("evaluate symlinks for %s: %w", nextPath, err)
+		}
+
+		currentPath = nextPath
+	}
+
+	return filepath.Clean(currentPath), nil
+}

server/lib/ziputil/ziputil_test.go

@@ -44,6 +44,20 @@ func TestUnzipRejectsEscapingSymlink(t *testing.T) {
 	assert.Contains(t, err.Error(), "illegal symlink target")
 }
 
+func TestUnzipRejectsSymlinkChainEscape(t *testing.T) {
+	zipPath := createSymlinkChainEscapeZip(t)
+	destParent := t.TempDir()
+	destDir := filepath.Join(destParent, "extract")
+	outsideFile := filepath.Join(destParent, "pwned.txt")
+
+	err := Unzip(zipPath, destDir)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "illegal symlink target")
+	_, statErr := os.Stat(outsideFile)
+	require.Error(t, statErr)
+	assert.True(t, os.IsNotExist(statErr))
+}
+
 func TestUnzipOverwritesFileWithSymlink(t *testing.T) {
 	zipPath := createSymlinkZip(t, "target.txt")
 	destDir := t.TempDir()
@@ -80,6 +94,40 @@ func createSymlinkZip(t *testing.T, target string) string {
 	return zipPath
 }
 
+func createSymlinkChainEscapeZip(t *testing.T) string {
+	t.Helper()
+
+	zipPath := filepath.Join(t.TempDir(), "chain-escape.zip")
+	zipFile, err := os.Create(zipPath)
+	require.NoError(t, err)
+
+	zipWriter := zip.NewWriter(zipFile)
+
+	linkHeader := &zip.FileHeader{Name: "link", Method: zip.Store}
+	linkHeader.SetMode(os.ModeSymlink | 0777)
+	linkWriter, err := zipWriter.CreateHeader(linkHeader)
+	require.NoError(t, err)
+	_, err = linkWriter.Write([]byte("."))
+	require.NoError(t, err)
+
+	escapeHeader := &zip.FileHeader{Name: "escape", Method: zip.Store}
+	escapeHeader.SetMode(os.ModeSymlink | 0777)
+	escapeWriter, err := zipWriter.CreateHeader(escapeHeader)
+	require.NoError(t, err)
+	_, err = escapeWriter.Write([]byte("link/.."))
+	require.NoError(t, err)
+
+	fileWriter, err := zipWriter.Create("escape/pwned.txt")
+	require.NoError(t, err)
+	_, err = fileWriter.Write([]byte("pwned"))
+	require.NoError(t, err)
+
+	require.NoError(t, zipWriter.Close())
+	require.NoError(t, zipFile.Close())
+
+	return zipPath
+}
+
 func TestUnzipFile(t *testing.T) {
 	// Create a temporary directory for test files
 	sourceDir, err := os.MkdirTemp("", "zip-source-*")