From: Ian Lance Taylor Date: Fri, 1 Mar 2019 20:31:18 +0000 (-0800) Subject: [release-branch.go1.12] path/filepath: don't discard .. in EvalSymlinks X-Git-Tag: go1.12.1~14 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=0ea746023f4c6ce9d889e521af8756de0c9674e4;p=gostls13.git [release-branch.go1.12] 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. Updates #30520 Fixes #30586 Change-Id: I07525575f83009032fa1a99aa270c8d42007d276 Reviewed-on: https://go-review.googlesource.com/c/go/+/164762 Reviewed-by: Bryan C. Mills (cherry picked from commit 294edb272d5d145665bdf8b4254609eae0363a8d) Reviewed-on: https://go-review.googlesource.com/c/go/+/165197 Reviewed-by: Brad Fitzpatrick Run-TryBot: Bryan C. Mills TryBot-Result: Gobot Gobot --- diff --git a/src/path/filepath/path_test.go b/src/path/filepath/path_test.go index 7a434a4292..adda8c5d10 100644 --- a/src/path/filepath/path_test.go +++ b/src/path/filepath/path_test.go @@ -1410,3 +1410,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 4b41039e25..de043c1677 100644 --- a/src/path/filepath/symlink.go +++ b/src/path/filepath/symlink.go @@ -44,18 +44,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