Skip to content

Commit cc323e5

Browse files
authored
Merge commit from fork
resolveWritePath enforced the workingDir write boundary with a lexical filepath.Rel check. This prevented "../" path escapes but did not resolve symlinks: a path component under workingDir pointing to an external directory (e.g. "out" -> "/outside") passed the check, causing writes to land outside workingDir when AllowPathTraversalOnWrite=false. Fix: after the lexical check, call checkSymlinkEscape, which resolves symlinks in workingDir and in the deepest existing ancestor of the write target, then verifies the resulting real path is still under the real workingDir. The helper realPathForWrite walks up to the deepest existing ancestor before calling EvalSymlinks so it handles write targets whose parent directories do not yet exist. Add TestStore_resolveWritePath_SymlinkTraversal with two sub-tests: - a symlink component under workingDir escaping the boundary is blocked - a regular subdirectory write within workingDir continues to work Signed-off-by: Terry Howe <terrylhowe@gmail.com>
1 parent 7a9f4b0 commit cc323e5

2 files changed

Lines changed: 113 additions & 0 deletions

File tree

content/file/file.go

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -625,6 +625,13 @@ func (s *Store) resolveWritePath(name string) (string, error) {
625625
if strings.HasPrefix(rel, "../") || rel == ".." {
626626
return "", ErrPathTraversalDisallowed
627627
}
628+
// The lexical check above prevents "../" escapes but does not resolve
629+
// symlinks. A symlink component under workingDir (e.g. "out" -> "/outside")
630+
// passes the lexical check yet directs writes outside workingDir.
631+
// Re-check after resolving symlinks in the parent path to close that gap.
632+
if err := checkSymlinkEscape(base, target); err != nil {
633+
return "", err
634+
}
628635
}
629636
if s.DisableOverwrite {
630637
if _, err := os.Stat(path); err == nil {
@@ -686,3 +693,52 @@ func (s *Store) setClosed() {
686693
func ensureDir(path string) error {
687694
return os.MkdirAll(path, 0777)
688695
}
696+
697+
// checkSymlinkEscape returns ErrPathTraversalDisallowed if resolving symlinks
698+
// in target's ancestor directories causes it to escape base. target may not
699+
// yet exist, so symlinks are resolved on its deepest existing ancestor.
700+
func checkSymlinkEscape(base, target string) error {
701+
realBase, err := filepath.EvalSymlinks(base)
702+
if err != nil {
703+
if os.IsNotExist(err) {
704+
return nil // base doesn't exist yet; no symlinks to follow
705+
}
706+
return err
707+
}
708+
realTarget, err := realPathForWrite(target)
709+
if err != nil {
710+
return err
711+
}
712+
rel, err := filepath.Rel(realBase, realTarget)
713+
if err != nil {
714+
return ErrPathTraversalDisallowed
715+
}
716+
rel = filepath.ToSlash(rel)
717+
if strings.HasPrefix(rel, "../") || rel == ".." {
718+
return ErrPathTraversalDisallowed
719+
}
720+
return nil
721+
}
722+
723+
// realPathForWrite resolves symlinks in the deepest existing ancestor of path
724+
// and returns the resulting absolute path. Non-existent path components are
725+
// appended verbatim, matching the semantics of a file about to be created.
726+
func realPathForWrite(path string) (string, error) {
727+
dir := filepath.Dir(path)
728+
suffix := filepath.Base(path)
729+
for {
730+
real, err := filepath.EvalSymlinks(dir)
731+
if err == nil {
732+
return filepath.Join(real, suffix), nil
733+
}
734+
if !os.IsNotExist(err) {
735+
return "", err
736+
}
737+
parent := filepath.Dir(dir)
738+
if parent == dir {
739+
return path, nil // reached filesystem root
740+
}
741+
suffix = filepath.Join(filepath.Base(dir), suffix)
742+
dir = parent
743+
}
744+
}

content/file/file_test.go

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3295,6 +3295,63 @@ func TestStore_resolveWritePath_PathTraversal(t *testing.T) {
32953295
}
32963296
}
32973297

3298+
func TestStore_resolveWritePath_SymlinkTraversal(t *testing.T) {
3299+
// GHSA-8xwf-rjm4-xvhv: resolveWritePath used a lexical filepath.Rel check
3300+
// that passed for names like "out/secret.txt" even when "out" is a symlink
3301+
// pointing outside workingDir.
3302+
t.Run("symlink component in workingDir escaping write boundary is blocked", func(t *testing.T) {
3303+
tempDir := t.TempDir()
3304+
outside := filepath.Join(tempDir, "outside")
3305+
if err := os.MkdirAll(outside, 0755); err != nil {
3306+
t.Fatal(err)
3307+
}
3308+
workingDir := filepath.Join(tempDir, "store")
3309+
if err := os.MkdirAll(workingDir, 0755); err != nil {
3310+
t.Fatal(err)
3311+
}
3312+
// "store/out" is a symlink to a directory outside workingDir.
3313+
if err := os.Symlink(outside, filepath.Join(workingDir, "out")); err != nil {
3314+
t.Skip("symlinks not available:", err)
3315+
}
3316+
3317+
s, err := New(workingDir)
3318+
if err != nil {
3319+
t.Fatal(err)
3320+
}
3321+
defer s.Close()
3322+
3323+
// "out/secret.txt" passes the lexical check but follows the symlink
3324+
// to outside/secret.txt, escaping workingDir.
3325+
_, err = s.resolveWritePath("out/secret.txt")
3326+
if !errors.Is(err, ErrPathTraversalDisallowed) {
3327+
t.Errorf("resolveWritePath() error = %v, want %v", err, ErrPathTraversalDisallowed)
3328+
}
3329+
})
3330+
3331+
t.Run("regular subdirectory write within workingDir still works", func(t *testing.T) {
3332+
tempDir := t.TempDir()
3333+
workingDir := filepath.Join(tempDir, "store")
3334+
subDir := filepath.Join(workingDir, "sub")
3335+
if err := os.MkdirAll(subDir, 0755); err != nil {
3336+
t.Fatal(err)
3337+
}
3338+
3339+
s, err := New(workingDir)
3340+
if err != nil {
3341+
t.Fatal(err)
3342+
}
3343+
defer s.Close()
3344+
3345+
got, err := s.resolveWritePath("sub/file.txt")
3346+
if err != nil {
3347+
t.Fatalf("resolveWritePath() unexpected error: %v", err)
3348+
}
3349+
if want := filepath.Join(workingDir, "sub/file.txt"); got != want {
3350+
t.Errorf("resolveWritePath() = %v, want %v", got, want)
3351+
}
3352+
})
3353+
}
3354+
32983355
func TestStore_resolveWritePath_Overwrite(t *testing.T) {
32993356
t.Run("Target file already exists", func(t *testing.T) {
33003357
tempDir := t.TempDir()

0 commit comments

Comments
 (0)