fix for drive backend symlink traversal

2026-08-24 02:34:20 -05:00 · 2026-04-10 21:10:41 -04:00
parent 1206f25822
commit 16061d2fc4
4 changed files with 356 additions and 18 deletions
@@ -4,7 +4,8 @@
 .vscode
 .contexts
 CLAUDE.md
-/.claude/
+.claude
+.codex
 AGENTS.md
 *.db
 /automated-release-build/
@@ -1,5 +1,9 @@
 # CHANGELOG

+## v2.0.2
+
+FIX: The `drive` backend mode WebDAV implementation now prevents symlink traversal outside the configured shared directory. `Stat`, `OpenFile`, `Mkdir`, `RemoveAll`, and `Rename` now reject symlinks that resolve outside the drive root while continuing to allow symlinks that resolve within that tree. This fixes GHSA-74m3-9qvm-rp9h.
+
 ## v2.0.1

 FEATURE: Added several new `admin` API endpoints for interfacing with additional management controls: finding limit classes by label, finding applied/applying/removing limit classes from accounts, getting/setting skip interstitial status for an account (https://github.com/openziti/zrok/issues/1210)
@@ -57,6 +57,28 @@ type File interface {
 	io.Writer
 }

+type namedFileInfo struct {
+	os.FileInfo
+	name string
+}
+
+func (fi namedFileInfo) Name() string {
+	return fi.name
+}
+
+type osDirFile struct {
+	*os.File
+	statName string
+}
+
+func (f *osDirFile) Stat() (os.FileInfo, error) {
+	info, err := f.File.Stat()
+	if err != nil {
+		return nil, err
+	}
+	return namedFileInfo{FileInfo: info, name: f.statName}, nil
+}
+
 type webdavFile struct {
 	File
 	name string
@@ -130,54 +152,195 @@ func (d Dir) resolve(name string) string {
 	return filepath.Join(dir, filepath.FromSlash(slashClean(name)))
 }

+func (d Dir) rootPath() (string, error) {
+	dir := string(d)
+	if dir == "" {
+		dir = "."
+	}
+	root, err := filepath.Abs(dir)
+	if err != nil {
+		return "", err
+	}
+	return filepath.EvalSymlinks(root)
+}
+
+func splitVirtualPath(name string) []string {
+	switch name {
+	case "", ".", "/":
+		return nil
+	}
+	name = strings.TrimPrefix(name, "/")
+	if name == "" {
+		return nil
+	}
+	return strings.Split(name, "/")
+}
+
+func joinVirtualPath(base string, segments []string) string {
+	for _, segment := range segments {
+		base = filepath.Join(base, filepath.FromSlash(segment))
+	}
+	return base
+}
+
+func isWithinRoot(root, target string) bool {
+	rel, err := filepath.Rel(root, target)
+	if err != nil {
+		return false
+	}
+	return rel == "." || (rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator)))
+}
+
+func virtualName(name string) string {
+	return path.Base(slashClean(name))
+}
+
+func (d Dir) resolveBoundedPath(name string, followLeaf, allowMissingLeaf, allowMissingTail bool) (string, error) {
+	if d.resolve(name) == "" {
+		return "", os.ErrNotExist
+	}
+	root, err := d.rootPath()
+	if err != nil {
+		return "", err
+	}
+
+	current := root
+	segments := splitVirtualPath(slashClean(name))
+	symlinks := 0
+
+	for len(segments) > 0 {
+		segment := segments[0]
+		segments = segments[1:]
+
+		next := filepath.Join(current, filepath.FromSlash(segment))
+		final := len(segments) == 0
+
+		info, err := os.Lstat(next)
+		if err != nil {
+			if os.IsNotExist(err) && (allowMissingTail || (allowMissingLeaf && final)) {
+				return joinVirtualPath(current, append([]string{segment}, segments...)), nil
+			}
+			return "", err
+		}
+
+		if info.Mode()&os.ModeSymlink == 0 {
+			current = next
+			continue
+		}
+
+		if !followLeaf && final {
+			return next, nil
+		}
+
+		symlinks++
+		if symlinks > 255 {
+			return "", os.ErrInvalid
+		}
+
+		target, err := os.Readlink(next)
+		if err != nil {
+			return "", err
+		}
+		if !filepath.IsAbs(target) {
+			target = filepath.Join(current, target)
+		}
+		target = filepath.Clean(target)
+
+		if !isWithinRoot(root, target) {
+			return "", os.ErrPermission
+		}
+
+		relTarget, err := filepath.Rel(root, target)
+		if err != nil {
+			return "", os.ErrPermission
+		}
+		targetSegments := splitVirtualPath(filepath.ToSlash(relTarget))
+
+		current = root
+		segments = append(targetSegments, segments...)
+	}
+
+	return current, nil
+}
+
 func (d Dir) Mkdir(ctx context.Context, name string, perm os.FileMode) error {
-	if name = d.resolve(name); name == "" {
-		return os.ErrNotExist
+	name, err := d.resolveBoundedPath(name, false, true, false)
+	if err != nil {
+		return err
 	}
 	return os.Mkdir(name, perm)
 }

 func (d Dir) OpenFile(ctx context.Context, name string, flag int, perm os.FileMode) (File, error) {
-	if name = d.resolve(name); name == "" {
-		return nil, os.ErrNotExist
-	}
-	f, err := os.OpenFile(name, flag, perm)
+	resolvedName, err := d.resolveBoundedPath(name, true, flag&os.O_CREATE != 0, false)
 	if err != nil {
 		return nil, err
 	}
-	return &webdavFile{f, name}, nil
+	f, err := os.OpenFile(resolvedName, flag, perm)
+	if err != nil {
+		return nil, err
+	}
+	return &webdavFile{
+		File: &osDirFile{
+			File:     f,
+			statName: virtualName(name),
+		},
+		name: resolvedName,
+	}, nil
 }

 func (d Dir) RemoveAll(ctx context.Context, name string) error {
-	if name = d.resolve(name); name == "" {
+	resolvedName := d.resolve(name)
+	if resolvedName == "" {
 		return os.ErrNotExist
 	}
-	if name == filepath.Clean(string(d)) {
+	if resolvedName == filepath.Clean(string(d)) {
 		// Prohibit removing the virtual root directory.
 		return os.ErrInvalid
 	}
-	return os.RemoveAll(name)
+	var err error
+	resolvedName, err = d.resolveBoundedPath(name, false, false, true)
+	if err != nil {
+		return err
+	}
+	return os.RemoveAll(resolvedName)
 }

 func (d Dir) Rename(ctx context.Context, oldName, newName string) error {
-	if oldName = d.resolve(oldName); oldName == "" {
+	resolvedOldName := d.resolve(oldName)
+	if resolvedOldName == "" {
 		return os.ErrNotExist
 	}
-	if newName = d.resolve(newName); newName == "" {
+	resolvedNewName := d.resolve(newName)
+	if resolvedNewName == "" {
 		return os.ErrNotExist
 	}
-	if root := filepath.Clean(string(d)); root == oldName || root == newName {
+	if root := filepath.Clean(string(d)); root == resolvedOldName || root == resolvedNewName {
 		// Prohibit renaming from or to the virtual root directory.
 		return os.ErrInvalid
 	}
-	return os.Rename(oldName, newName)
+	var err error
+	resolvedOldName, err = d.resolveBoundedPath(oldName, false, false, false)
+	if err != nil {
+		return err
+	}
+	resolvedNewName, err = d.resolveBoundedPath(newName, false, true, false)
+	if err != nil {
+		return err
+	}
+	return os.Rename(resolvedOldName, resolvedNewName)
 }

 func (d Dir) Stat(ctx context.Context, name string) (os.FileInfo, error) {
-	if name = d.resolve(name); name == "" {
-		return nil, os.ErrNotExist
+	resolvedName, err := d.resolveBoundedPath(name, true, false, false)
+	if err != nil {
+		return nil, err
 	}
-	return os.Stat(name)
+	info, err := os.Stat(resolvedName)
+	if err != nil {
+		return nil, err
+	}
+	return namedFileInfo{FileInfo: info, name: virtualName(name)}, nil
 }

 // NewMemFS returns a new in-memory FileSystem implementation.
@@ -7,6 +7,7 @@ package davServer
 import (
 	"context"
 	"encoding/xml"
+	"errors"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -510,6 +511,13 @@ func testFS(t *testing.T, fs FileSystem) {
 	}
 }

+func symlinkOrSkip(t *testing.T, target, name string) {
+	t.Helper()
+	if err := os.Symlink(target, name); err != nil {
+		t.Skipf("symlinks unavailable: %v", err)
+	}
+}
+
 func TestDir(t *testing.T) {
 	switch runtime.GOOS {
 	case "nacl":
@@ -526,6 +534,168 @@ func TestDir(t *testing.T) {
 	testFS(t, Dir(td))
 }

+func TestDirRejectsSymlinkEscape(t *testing.T) {
+	ctx := context.Background()
+	root := t.TempDir()
+	outside := t.TempDir()
+
+	secretName := filepath.Join(outside, "secret.txt")
+	if err := os.WriteFile(secretName, []byte("secret"), 0600); err != nil {
+		t.Fatalf("WriteFile: %v", err)
+	}
+	symlinkOrSkip(t, secretName, filepath.Join(root, "escape.txt"))
+
+	fs := Dir(root)
+
+	if _, err := fs.Stat(ctx, "/escape.txt"); !errors.Is(err, os.ErrPermission) {
+		t.Fatalf("Stat: got %v, want permission error", err)
+	}
+
+	if _, err := fs.OpenFile(ctx, "/escape.txt", os.O_RDONLY, 0); !errors.Is(err, os.ErrPermission) {
+		t.Fatalf("OpenFile: got %v, want permission error", err)
+	}
+}
+
+func TestDirRejectsSymlinkParentEscape(t *testing.T) {
+	ctx := context.Background()
+	root := t.TempDir()
+	outside := t.TempDir()
+
+	if err := os.WriteFile(filepath.Join(root, "source.txt"), []byte("source"), 0600); err != nil {
+		t.Fatalf("WriteFile source: %v", err)
+	}
+
+	victimName := filepath.Join(outside, "victim.txt")
+	if err := os.WriteFile(victimName, []byte("victim"), 0600); err != nil {
+		t.Fatalf("WriteFile victim: %v", err)
+	}
+
+	symlinkOrSkip(t, outside, filepath.Join(root, "escape"))
+
+	fs := Dir(root)
+
+	if _, err := fs.OpenFile(ctx, "/escape/created.txt", os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0600); !errors.Is(err, os.ErrPermission) {
+		t.Fatalf("OpenFile create: got %v, want permission error", err)
+	}
+	if _, err := os.Stat(filepath.Join(outside, "created.txt")); !os.IsNotExist(err) {
+		t.Fatalf("outside create: got %v, want not exist", err)
+	}
+
+	if err := fs.Mkdir(ctx, "/escape/subdir", 0755); !errors.Is(err, os.ErrPermission) {
+		t.Fatalf("Mkdir: got %v, want permission error", err)
+	}
+	if _, err := os.Stat(filepath.Join(outside, "subdir")); !os.IsNotExist(err) {
+		t.Fatalf("outside mkdir: got %v, want not exist", err)
+	}
+
+	if err := fs.Rename(ctx, "/source.txt", "/escape/renamed.txt"); !errors.Is(err, os.ErrPermission) {
+		t.Fatalf("Rename: got %v, want permission error", err)
+	}
+	if _, err := os.Stat(filepath.Join(outside, "renamed.txt")); !os.IsNotExist(err) {
+		t.Fatalf("outside rename: got %v, want not exist", err)
+	}
+	if _, err := os.Stat(filepath.Join(root, "source.txt")); err != nil {
+		t.Fatalf("source retained: %v", err)
+	}
+
+	if err := fs.RemoveAll(ctx, "/escape/victim.txt"); !errors.Is(err, os.ErrPermission) {
+		t.Fatalf("RemoveAll: got %v, want permission error", err)
+	}
+	if _, err := os.Stat(victimName); err != nil {
+		t.Fatalf("victim retained: %v", err)
+	}
+}
+
+func TestDirAllowsInternalSymlinks(t *testing.T) {
+	ctx := context.Background()
+	root := t.TempDir()
+
+	targetName := filepath.Join(root, "target.txt")
+	if err := os.WriteFile(targetName, []byte("target"), 0600); err != nil {
+		t.Fatalf("WriteFile target: %v", err)
+	}
+
+	symlinkOrSkip(t, "target.txt", filepath.Join(root, "link.txt"))
+
+	fs := Dir(root)
+
+	stat, err := fs.Stat(ctx, "/link.txt")
+	if err != nil {
+		t.Fatalf("Stat: %v", err)
+	}
+	if stat.Name() != "link.txt" {
+		t.Fatalf("Stat.Name: got %q, want %q", stat.Name(), "link.txt")
+	}
+
+	f, err := fs.OpenFile(ctx, "/link.txt", os.O_RDONLY, 0)
+	if err != nil {
+		t.Fatalf("OpenFile: %v", err)
+	}
+	defer f.Close()
+
+	info, err := f.Stat()
+	if err != nil {
+		t.Fatalf("file Stat: %v", err)
+	}
+	if info.Name() != "link.txt" {
+		t.Fatalf("file Stat.Name: got %q, want %q", info.Name(), "link.txt")
+	}
+
+	data, err := io.ReadAll(f)
+	if err != nil {
+		t.Fatalf("ReadAll: %v", err)
+	}
+	if got := string(data); got != "target" {
+		t.Fatalf("ReadAll: got %q, want %q", got, "target")
+	}
+
+	actualDir := filepath.Join(root, "actual")
+	if err := os.Mkdir(actualDir, 0755); err != nil {
+		t.Fatalf("Mkdir actual: %v", err)
+	}
+	symlinkOrSkip(t, "actual", filepath.Join(root, "nested"))
+
+	g, err := fs.OpenFile(ctx, "/nested/created.txt", os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0600)
+	if err != nil {
+		t.Fatalf("OpenFile nested: %v", err)
+	}
+	if _, err := g.Write([]byte("created")); err != nil {
+		t.Fatalf("Write nested: %v", err)
+	}
+	if err := g.Close(); err != nil {
+		t.Fatalf("Close nested: %v", err)
+	}
+
+	created, err := os.ReadFile(filepath.Join(actualDir, "created.txt"))
+	if err != nil {
+		t.Fatalf("ReadFile created: %v", err)
+	}
+	if got := string(created); got != "created" {
+		t.Fatalf("ReadFile created: got %q, want %q", got, "created")
+	}
+
+	symlinkOrSkip(t, "created-via-link.txt", filepath.Join(root, "new-link.txt"))
+
+	h, err := fs.OpenFile(ctx, "/new-link.txt", os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0600)
+	if err != nil {
+		t.Fatalf("OpenFile broken link: %v", err)
+	}
+	if _, err := h.Write([]byte("via-link")); err != nil {
+		t.Fatalf("Write broken link: %v", err)
+	}
+	if err := h.Close(); err != nil {
+		t.Fatalf("Close broken link: %v", err)
+	}
+
+	viaLink, err := os.ReadFile(filepath.Join(root, "created-via-link.txt"))
+	if err != nil {
+		t.Fatalf("ReadFile broken link target: %v", err)
+	}
+	if got := string(viaLink); got != "via-link" {
+		t.Fatalf("ReadFile broken link target: got %q, want %q", got, "via-link")
+	}
+}
+
 func TestMemFS(t *testing.T) {
 	testFS(t, NewMemFS())
 }