Skip to content

Commit 0ea7460

Browse files
[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 <[email protected]> (cherry picked from commit 294edb2) Reviewed-on: https://go-review.googlesource.com/c/go/+/165197 Reviewed-by: Brad Fitzpatrick <[email protected]> Run-TryBot: Bryan C. Mills <[email protected]> TryBot-Result: Gobot Gobot <[email protected]>
1 parent 6ff06c1 commit 0ea7460

File tree

2 files changed

+109
-1
lines changed

2 files changed

+109
-1
lines changed

src/path/filepath/path_test.go

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1410,3 +1410,103 @@ func TestIssue29372(t *testing.T) {
14101410
}
14111411
}
14121412
}
1413+
1414+
// Issue 30520 part 1.
1415+
func TestEvalSymlinksAboveRoot(t *testing.T) {
1416+
testenv.MustHaveSymlink(t)
1417+
1418+
t.Parallel()
1419+
1420+
tmpDir, err := ioutil.TempDir("", "TestEvalSymlinksAboveRoot")
1421+
if err != nil {
1422+
t.Fatal(err)
1423+
}
1424+
defer os.RemoveAll(tmpDir)
1425+
1426+
evalTmpDir, err := filepath.EvalSymlinks(tmpDir)
1427+
if err != nil {
1428+
t.Fatal(err)
1429+
}
1430+
1431+
if err := os.Mkdir(filepath.Join(evalTmpDir, "a"), 0777); err != nil {
1432+
t.Fatal(err)
1433+
}
1434+
if err := os.Symlink(filepath.Join(evalTmpDir, "a"), filepath.Join(evalTmpDir, "b")); err != nil {
1435+
t.Fatal(err)
1436+
}
1437+
if err := ioutil.WriteFile(filepath.Join(evalTmpDir, "a", "file"), nil, 0666); err != nil {
1438+
t.Fatal(err)
1439+
}
1440+
1441+
// Count the number of ".." elements to get to the root directory.
1442+
vol := filepath.VolumeName(evalTmpDir)
1443+
c := strings.Count(evalTmpDir[len(vol):], string(os.PathSeparator))
1444+
var dd []string
1445+
for i := 0; i < c+2; i++ {
1446+
dd = append(dd, "..")
1447+
}
1448+
1449+
wantSuffix := strings.Join([]string{"a", "file"}, string(os.PathSeparator))
1450+
1451+
// Try different numbers of "..".
1452+
for _, i := range []int{c, c + 1, c + 2} {
1453+
check := strings.Join([]string{evalTmpDir, strings.Join(dd[:i], string(os.PathSeparator)), evalTmpDir[len(vol)+1:], "b", "file"}, string(os.PathSeparator))
1454+
if resolved, err := filepath.EvalSymlinks(check); err != nil {
1455+
t.Errorf("EvalSymlinks(%q) failed: %v", check, err)
1456+
} else if !strings.HasSuffix(resolved, wantSuffix) {
1457+
t.Errorf("EvalSymlinks(%q) = %q does not end with %q", check, resolved, wantSuffix)
1458+
} else {
1459+
t.Logf("EvalSymlinks(%q) = %q", check, resolved)
1460+
}
1461+
}
1462+
}
1463+
1464+
// Issue 30520 part 2.
1465+
func TestEvalSymlinksAboveRootChdir(t *testing.T) {
1466+
testenv.MustHaveSymlink(t)
1467+
1468+
tmpDir, err := ioutil.TempDir("", "TestEvalSymlinksAboveRootChdir")
1469+
if err != nil {
1470+
t.Fatal(err)
1471+
}
1472+
defer os.RemoveAll(tmpDir)
1473+
1474+
wd, err := os.Getwd()
1475+
if err != nil {
1476+
t.Fatal(err)
1477+
}
1478+
defer os.Chdir(wd)
1479+
1480+
if err := os.Chdir(tmpDir); err != nil {
1481+
t.Fatal(err)
1482+
}
1483+
1484+
subdir := filepath.Join("a", "b")
1485+
if err := os.MkdirAll(subdir, 0777); err != nil {
1486+
t.Fatal(err)
1487+
}
1488+
if err := os.Symlink(subdir, "c"); err != nil {
1489+
t.Fatal(err)
1490+
}
1491+
if err := ioutil.WriteFile(filepath.Join(subdir, "file"), nil, 0666); err != nil {
1492+
t.Fatal(err)
1493+
}
1494+
1495+
subdir = filepath.Join("d", "e", "f")
1496+
if err := os.MkdirAll(subdir, 0777); err != nil {
1497+
t.Fatal(err)
1498+
}
1499+
if err := os.Chdir(subdir); err != nil {
1500+
t.Fatal(err)
1501+
}
1502+
1503+
check := filepath.Join("..", "..", "..", "c", "file")
1504+
wantSuffix := filepath.Join("a", "b", "file")
1505+
if resolved, err := filepath.EvalSymlinks(check); err != nil {
1506+
t.Errorf("EvalSymlinks(%q) failed: %v", check, err)
1507+
} else if !strings.HasSuffix(resolved, wantSuffix) {
1508+
t.Errorf("EvalSymlinks(%q) = %q does not end with %q", check, resolved, wantSuffix)
1509+
} else {
1510+
t.Logf("EvalSymlinks(%q) = %q", check, resolved)
1511+
}
1512+
}

src/path/filepath/symlink.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,18 +44,26 @@ func walkSymlinks(path string) (string, error) {
4444
} else if path[start:end] == ".." {
4545
// Back up to previous component if possible.
4646
// Note that volLen includes any leading slash.
47+
48+
// Set r to the index of the last slash in dest,
49+
// after the volume.
4750
var r int
4851
for r = len(dest) - 1; r >= volLen; r-- {
4952
if os.IsPathSeparator(dest[r]) {
5053
break
5154
}
5255
}
53-
if r < volLen {
56+
if r < volLen || dest[r+1:] == ".." {
57+
// Either path has no slashes
58+
// (it's empty or just "C:")
59+
// or it ends in a ".." we had to keep.
60+
// Either way, keep this "..".
5461
if len(dest) > volLen {
5562
dest += pathSeparator
5663
}
5764
dest += ".."
5865
} else {
66+
// Discard everything since the last slash.
5967
dest = dest[:r]
6068
}
6169
continue

0 commit comments

Comments
 (0)