aboutsummaryrefslogtreecommitdiff
path: root/internal/admin
diff options
context:
space:
mode:
Diffstat (limited to 'internal/admin')
-rw-r--r--internal/admin/api_files.go53
-rw-r--r--internal/admin/correct_audit_fixes_test.go16
2 files changed, 66 insertions, 3 deletions
diff --git a/internal/admin/api_files.go b/internal/admin/api_files.go
index b1b3fff..9745b70 100644
--- a/internal/admin/api_files.go
+++ b/internal/admin/api_files.go
@@ -13,14 +13,61 @@ import (
)
// resolveDataPath resolves a request-supplied path inside the admin data
-// directory, rejecting paths that would escape it via .. segments. The
-// returned path is always filepath.Join(dataDir, cleaned relative path).
+// directory, rejecting paths that would escape it via .. segments or a symlink.
+// The returned path is always filepath.Join(dataDir, cleaned relative path).
func resolveDataPath(dataDir, relPath string) (string, error) {
clean := filepath.Clean(strings.TrimPrefix(strings.TrimPrefix(relPath, "/"), "data/"))
if clean == ".." || strings.HasPrefix(clean, ".."+string(filepath.Separator)) {
return "", fmt.Errorf("path escapes the data directory: %s", relPath)
}
- return filepath.Join(dataDir, clean), nil
+ full := filepath.Join(dataDir, clean)
+ // Defence in depth: a symlink inside data/ could redirect a read or write
+ // outside it, so resolve the longest existing prefix and re-check
+ // containment. The root is resolved too, since the data dir itself may be
+ // a symlink (e.g. a mounted volume).
+ resolved, err := evalExisting(full)
+ if err != nil {
+ return "", fmt.Errorf("resolve path %s: %w", relPath, err)
+ }
+ root := dataDir
+ if r, err := filepath.EvalSymlinks(dataDir); err == nil {
+ root = r
+ }
+ if !withinDir(root, resolved) {
+ return "", fmt.Errorf("path escapes the data directory via a symlink: %s", relPath)
+ }
+ return full, nil
+}
+
+// evalExisting resolves symlinks on the longest existing prefix of path and
+// re-appends the not-yet-existing suffix, so a create target is still checked.
+func evalExisting(path string) (string, error) {
+ p := path
+ var suffix []string
+ for {
+ resolved, err := filepath.EvalSymlinks(p)
+ if err == nil {
+ for i := len(suffix) - 1; i >= 0; i-- {
+ resolved = filepath.Join(resolved, suffix[i])
+ }
+ return resolved, nil
+ }
+ if !os.IsNotExist(err) {
+ return "", err
+ }
+ parent := filepath.Dir(p)
+ if parent == p {
+ return path, nil
+ }
+ suffix = append(suffix, filepath.Base(p))
+ p = parent
+ }
+}
+
+// withinDir reports whether path lies inside dir.
+func withinDir(dir, path string) bool {
+ rel, err := filepath.Rel(dir, path)
+ return err == nil && rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator))
}
// canonicalDataRelPath normalizes a client-supplied data-relative path so that
diff --git a/internal/admin/correct_audit_fixes_test.go b/internal/admin/correct_audit_fixes_test.go
index d97732d..d7b713e 100644
--- a/internal/admin/correct_audit_fixes_test.go
+++ b/internal/admin/correct_audit_fixes_test.go
@@ -73,3 +73,19 @@ func TestUndoFailureIsReportedDistinctly(t *testing.T) {
t.Fatal("empty history must be a clean nil change and nil error")
}
}
+
+// Regression: a symlink inside data/ pointing outside it must not let the raw
+// file API read or write outside the data directory.
+func TestResolveDataPathRejectsSymlinkEscape(t *testing.T) {
+ dir := t.TempDir()
+ outside := t.TempDir()
+ if err := os.Symlink(outside, filepath.Join(dir, "link")); err != nil {
+ t.Skipf("symlinks unsupported: %v", err)
+ }
+ if _, err := resolveDataPath(dir, "link/secret.yaml"); err == nil {
+ t.Fatal("a symlink escaping the data directory was accepted")
+ }
+ if _, err := resolveDataPath(dir, "rooms/1.yaml"); err != nil {
+ t.Fatalf("normal path rejected: %v", err)
+ }
+}