path/filepath: don't discard .. in EvalSymlinks

EvalSymlinks was mishandling cases like "/x/../../y" or "../../../x"
where there is an extra ".." that goes past the start of the path.

Fixes #30520

Change-Id: I07525575f83009032fa1a99aa270c8d42007d276
Reviewed-on: https://go-review.googlesource.com/c/go/+/164762
Reviewed-by: Bryan C. Mills <bcmills@google.com>

diff --git a/src/path/filepath/path_test.go b/src/path/filepath/path_test.go
index 709dccb61b7acca0d9376d8b988f466cbb3aab9e..2d13149f3f62026adc075c0b120078f4d3346134 100644 (file)
--- a/src/path/filepath/path_test.go
+++ b/src/path/filepath/path_test.go
@@ -1400,3 +1400,103 @@ func TestIssue29372(t *testing.T) {
                }
        }
 }
+
+// Issue 30520 part 1.
+func TestEvalSymlinksAboveRoot(t *testing.T) {
+       testenv.MustHaveSymlink(t)
+
+       t.Parallel()
+
+       tmpDir, err := ioutil.TempDir("", "TestEvalSymlinksAboveRoot")
+       if err != nil {
+               t.Fatal(err)
+       }
+       defer os.RemoveAll(tmpDir)
+
+       evalTmpDir, err := filepath.EvalSymlinks(tmpDir)
+       if err != nil {
+               t.Fatal(err)
+       }
+
+       if err := os.Mkdir(filepath.Join(evalTmpDir, "a"), 0777); err != nil {
+               t.Fatal(err)
+       }
+       if err := os.Symlink(filepath.Join(evalTmpDir, "a"), filepath.Join(evalTmpDir, "b")); err != nil {
+               t.Fatal(err)
+       }
+       if err := ioutil.WriteFile(filepath.Join(evalTmpDir, "a", "file"), nil, 0666); err != nil {
+               t.Fatal(err)
+       }
+
+       // Count the number of ".." elements to get to the root directory.
+       vol := filepath.VolumeName(evalTmpDir)
+       c := strings.Count(evalTmpDir[len(vol):], string(os.PathSeparator))
+       var dd []string
+       for i := 0; i < c+2; i++ {
+               dd = append(dd, "..")
+       }
+
+       wantSuffix := strings.Join([]string{"a", "file"}, string(os.PathSeparator))
+
+       // Try different numbers of "..".
+       for _, i := range []int{c, c + 1, c + 2} {
+               check := strings.Join([]string{evalTmpDir, strings.Join(dd[:i], string(os.PathSeparator)), evalTmpDir[len(vol)+1:], "b", "file"}, string(os.PathSeparator))
+               if resolved, err := filepath.EvalSymlinks(check); err != nil {
+                       t.Errorf("EvalSymlinks(%q) failed: %v", check, err)
+               } else if !strings.HasSuffix(resolved, wantSuffix) {
+                       t.Errorf("EvalSymlinks(%q) = %q does not end with %q", check, resolved, wantSuffix)
+               } else {
+                       t.Logf("EvalSymlinks(%q) = %q", check, resolved)
+               }
+       }
+}
+
+// Issue 30520 part 2.
+func TestEvalSymlinksAboveRootChdir(t *testing.T) {
+       testenv.MustHaveSymlink(t)
+
+       tmpDir, err := ioutil.TempDir("", "TestEvalSymlinksAboveRootChdir")
+       if err != nil {
+               t.Fatal(err)
+       }
+       defer os.RemoveAll(tmpDir)
+
+       wd, err := os.Getwd()
+       if err != nil {
+               t.Fatal(err)
+       }
+       defer os.Chdir(wd)
+
+       if err := os.Chdir(tmpDir); err != nil {
+               t.Fatal(err)
+       }
+
+       subdir := filepath.Join("a", "b")
+       if err := os.MkdirAll(subdir, 0777); err != nil {
+               t.Fatal(err)
+       }
+       if err := os.Symlink(subdir, "c"); err != nil {
+               t.Fatal(err)
+       }
+       if err := ioutil.WriteFile(filepath.Join(subdir, "file"), nil, 0666); err != nil {
+               t.Fatal(err)
+       }
+
+       subdir = filepath.Join("d", "e", "f")
+       if err := os.MkdirAll(subdir, 0777); err != nil {
+               t.Fatal(err)
+       }
+       if err := os.Chdir(subdir); err != nil {
+               t.Fatal(err)
+       }
+
+       check := filepath.Join("..", "..", "..", "c", "file")
+       wantSuffix := filepath.Join("a", "b", "file")
+       if resolved, err := filepath.EvalSymlinks(check); err != nil {
+               t.Errorf("EvalSymlinks(%q) failed: %v", check, err)
+       } else if !strings.HasSuffix(resolved, wantSuffix) {
+               t.Errorf("EvalSymlinks(%q) = %q does not end with %q", check, resolved, wantSuffix)
+       } else {
+               t.Logf("EvalSymlinks(%q) = %q", check, resolved)
+       }
+}

diff --git a/src/path/filepath/symlink.go b/src/path/filepath/symlink.go
index a08b85a29cdb3c9dc7aea6d95e5088f2f26fb3bb..335b315a202104fb5d11c0d172a2ee5587e22a26 100644 (file)
--- a/src/path/filepath/symlink.go
+++ b/src/path/filepath/symlink.go
@@ -45,18 +45,26 @@ func walkSymlinks(path string) (string, error) {
                } else if path[start:end] == ".." {
                        // Back up to previous component if possible.
                        // Note that volLen includes any leading slash.
+
+                       // Set r to the index of the last slash in dest,
+                       // after the volume.
                        var r int
                        for r = len(dest) - 1; r >= volLen; r-- {
                                if os.IsPathSeparator(dest[r]) {
                                        break
                                }
                        }
-                       if r < volLen {
+                       if r < volLen || dest[r+1:] == ".." {
+                               // Either path has no slashes
+                               // (it's empty or just "C:")
+                               // or it ends in a ".." we had to keep.
+                               // Either way, keep this "..".
                                if len(dest) > volLen {
                                        dest += pathSeparator
                                }
                                dest += ".."
                        } else {
+                               // Discard everything since the last slash.
                                dest = dest[:r]
                        }
                        continue