Skip to content

Commit 46fb781

Browse files
neildgopherbot
authored andcommitted
[release-branch.go1.20] path/filepath: fix various issues in parsing Windows paths
On Windows, A root local device path is a path which begins with \\?\ or \??\. A root local device path accesses the DosDevices object directory, and permits access to any file or device on the system. For example \??\C:\foo is equivalent to common C:\foo. The Clean, IsAbs, IsLocal, and VolumeName functions did not recognize root local device paths beginning with \??\. Clean could convert a rooted path such as \a\..\??\b into the root local device path \??\b. It will now convert this path into .\??\b. IsAbs now correctly reports paths beginning with \??\ as absolute. IsLocal now correctly reports paths beginning with \??\ as non-local. VolumeName now reports the \??\ prefix as a volume name. Join(`\`, `??`, `b`) could convert a seemingly innocent sequence of path elements into the root local device path \??\b. It will now convert this to \.\??\b. In addition, the IsLocal function did not correctly detect reserved names in some cases: - reserved names followed by spaces, such as "COM1 ". - "COM" or "LPT" followed by a superscript 1, 2, or 3. IsLocal now correctly reports these names as non-local. For #63713 Fixes #63714 Fixes CVE-2023-45283 Fixes CVE-2023-45284 Change-Id: I446674a58977adfa54de7267d716ac23ab496c54 Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/2040691 Reviewed-by: Roland Shoemaker <[email protected]> Reviewed-by: Tatiana Bradley <[email protected]> Run-TryBot: Damien Neil <[email protected]> Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/2072597 Reviewed-by: Cherry Mui <[email protected]> Reviewed-on: https://go-review.googlesource.com/c/go/+/539276 Auto-Submit: Heschi Kreinick <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
1 parent 998fdce commit 46fb781

File tree

6 files changed

+269
-118
lines changed

6 files changed

+269
-118
lines changed

src/go/build/deps_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ var depsRules = `
153153
154154
unicode, fmt !< net, os, os/signal;
155155
156-
os/signal, STR
156+
os/signal, internal/safefilepath, STR
157157
< path/filepath
158158
< io/ioutil;
159159

src/internal/safefilepath/path_windows.go

Lines changed: 72 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,10 @@ func fromFS(path string) (string, error) {
2020
for p := path; p != ""; {
2121
// Find the next path element.
2222
i := 0
23-
dot := -1
2423
for i < len(p) && p[i] != '/' {
2524
switch p[i] {
2625
case 0, '\\', ':':
2726
return "", errInvalidPath
28-
case '.':
29-
if dot < 0 {
30-
dot = i
31-
}
3227
}
3328
i++
3429
}
@@ -39,22 +34,8 @@ func fromFS(path string) (string, error) {
3934
} else {
4035
p = ""
4136
}
42-
// Trim the extension and look for a reserved name.
43-
base := part
44-
if dot >= 0 {
45-
base = part[:dot]
46-
}
47-
if isReservedName(base) {
48-
if dot < 0 {
49-
return "", errInvalidPath
50-
}
51-
// The path element is a reserved name with an extension.
52-
// Some Windows versions consider this a reserved name,
53-
// while others do not. Use FullPath to see if the name is
54-
// reserved.
55-
if p, _ := syscall.FullPath(part); len(p) >= 4 && p[:4] == `\\.\` {
56-
return "", errInvalidPath
57-
}
37+
if IsReservedName(part) {
38+
return "", errInvalidPath
5839
}
5940
}
6041
if containsSlash {
@@ -70,23 +51,88 @@ func fromFS(path string) (string, error) {
7051
return path, nil
7152
}
7253

73-
// isReservedName reports if name is a Windows reserved device name.
54+
// IsReservedName reports if name is a Windows reserved device name.
7455
// It does not detect names with an extension, which are also reserved on some Windows versions.
7556
//
7657
// For details, search for PRN in
7758
// https://docs.microsoft.com/en-us/windows/desktop/fileio/naming-a-file.
78-
func isReservedName(name string) bool {
79-
if 3 <= len(name) && len(name) <= 4 {
59+
func IsReservedName(name string) bool {
60+
// Device names can have arbitrary trailing characters following a dot or colon.
61+
base := name
62+
for i := 0; i < len(base); i++ {
63+
switch base[i] {
64+
case ':', '.':
65+
base = base[:i]
66+
}
67+
}
68+
// Trailing spaces in the last path element are ignored.
69+
for len(base) > 0 && base[len(base)-1] == ' ' {
70+
base = base[:len(base)-1]
71+
}
72+
if !isReservedBaseName(base) {
73+
return false
74+
}
75+
if len(base) == len(name) {
76+
return true
77+
}
78+
// The path element is a reserved name with an extension.
79+
// Some Windows versions consider this a reserved name,
80+
// while others do not. Use FullPath to see if the name is
81+
// reserved.
82+
if p, _ := syscall.FullPath(name); len(p) >= 4 && p[:4] == `\\.\` {
83+
return true
84+
}
85+
return false
86+
}
87+
88+
func isReservedBaseName(name string) bool {
89+
if len(name) == 3 {
8090
switch string([]byte{toUpper(name[0]), toUpper(name[1]), toUpper(name[2])}) {
8191
case "CON", "PRN", "AUX", "NUL":
82-
return len(name) == 3
92+
return true
93+
}
94+
}
95+
if len(name) >= 4 {
96+
switch string([]byte{toUpper(name[0]), toUpper(name[1]), toUpper(name[2])}) {
8397
case "COM", "LPT":
84-
return len(name) == 4 && '1' <= name[3] && name[3] <= '9'
98+
if len(name) == 4 && '1' <= name[3] && name[3] <= '9' {
99+
return true
100+
}
101+
// Superscript ¹, ², and ³ are considered numbers as well.
102+
switch name[3:] {
103+
case "\u00b2", "\u00b3", "\u00b9":
104+
return true
105+
}
106+
return false
85107
}
86108
}
109+
110+
// Passing CONIN$ or CONOUT$ to CreateFile opens a console handle.
111+
// https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-createfilea#consoles
112+
//
113+
// While CONIN$ and CONOUT$ aren't documented as being files,
114+
// they behave the same as CON. For example, ./CONIN$ also opens the console input.
115+
if len(name) == 6 && name[5] == '$' && equalFold(name, "CONIN$") {
116+
return true
117+
}
118+
if len(name) == 7 && name[6] == '$' && equalFold(name, "CONOUT$") {
119+
return true
120+
}
87121
return false
88122
}
89123

124+
func equalFold(a, b string) bool {
125+
if len(a) != len(b) {
126+
return false
127+
}
128+
for i := 0; i < len(a); i++ {
129+
if toUpper(a[i]) != toUpper(b[i]) {
130+
return false
131+
}
132+
}
133+
return true
134+
}
135+
90136
func toUpper(c byte) byte {
91137
if 'a' <= c && c <= 'z' {
92138
return c - ('a' - 'A')

src/path/filepath/path.go

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import (
1515
"errors"
1616
"io/fs"
1717
"os"
18-
"runtime"
1918
"sort"
2019
"strings"
2120
)
@@ -163,21 +162,7 @@ func Clean(path string) string {
163162
out.append('.')
164163
}
165164

166-
if runtime.GOOS == "windows" && out.volLen == 0 && out.buf != nil {
167-
// If a ':' appears in the path element at the start of a Windows path,
168-
// insert a .\ at the beginning to avoid converting relative paths
169-
// like a/../c: into c:.
170-
for _, c := range out.buf {
171-
if os.IsPathSeparator(c) {
172-
break
173-
}
174-
if c == ':' {
175-
out.prepend('.', Separator)
176-
break
177-
}
178-
}
179-
}
180-
165+
postClean(&out) // avoid creating absolute paths on Windows
181166
return FromSlash(out.string())
182167
}
183168

src/path/filepath/path_nonwindows.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
// Copyright 2023 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
//go:build !windows
6+
7+
package filepath
8+
9+
func postClean(out *lazybuf) {}

src/path/filepath/path_test.go

Lines changed: 65 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,9 @@ var wincleantests = []PathTest{
115115
{`a/../c:/a`, `.\c:\a`},
116116
{`a/../../c:`, `..\c:`},
117117
{`foo:bar`, `foo:bar`},
118+
119+
// Don't allow cleaning to create a Root Local Device path like \??\a.
120+
{`/a/../??/a`, `\.\??\a`},
118121
}
119122

120123
func TestClean(t *testing.T) {
@@ -176,8 +179,28 @@ var islocaltests = []IsLocalTest{
176179
var winislocaltests = []IsLocalTest{
177180
{"NUL", false},
178181
{"nul", false},
182+
{"nul ", false},
179183
{"nul.", false},
184+
{"a/nul:", false},
185+
{"a/nul : a", false},
186+
{"com0", true},
180187
{"com1", false},
188+
{"com2", false},
189+
{"com3", false},
190+
{"com4", false},
191+
{"com5", false},
192+
{"com6", false},
193+
{"com7", false},
194+
{"com8", false},
195+
{"com9", false},
196+
{"com¹", false},
197+
{"com²", false},
198+
{"com³", false},
199+
{"com¹ : a", false},
200+
{"cOm1", false},
201+
{"lpt1", false},
202+
{"LPT1", false},
203+
{"lpt³", false},
181204
{"./nul", false},
182205
{`\`, false},
183206
{`\a`, false},
@@ -383,6 +406,7 @@ var winjointests = []JoinTest{
383406
{[]string{`\\a\`, `b`, `c`}, `\\a\b\c`},
384407
{[]string{`//`, `a`}, `\\a`},
385408
{[]string{`a:\b\c`, `x\..\y:\..\..\z`}, `a:\b\z`},
409+
{[]string{`\`, `??\a`}, `\.\??\a`},
386410
}
387411

388412
func TestJoin(t *testing.T) {
@@ -953,6 +977,8 @@ var winisabstests = []IsAbsTest{
953977
{`\\host\share\`, true},
954978
{`\\host\share\foo`, true},
955979
{`//host/share/foo/bar`, true},
980+
{`\\?\a\b\c`, true},
981+
{`\??\a\b\c`, true},
956982
}
957983

958984
func TestIsAbs(t *testing.T) {
@@ -1422,7 +1448,8 @@ type VolumeNameTest struct {
14221448
var volumenametests = []VolumeNameTest{
14231449
{`c:/foo/bar`, `c:`},
14241450
{`c:`, `c:`},
1425-
{`2:`, ``},
1451+
{`c:\`, `c:`},
1452+
{`2:`, `2:`},
14261453
{``, ``},
14271454
{`\\\host`, `\\\host`},
14281455
{`\\\host\`, `\\\host`},
@@ -1442,12 +1469,23 @@ var volumenametests = []VolumeNameTest{
14421469
{`//host/share//foo///bar////baz`, `\\host\share`},
14431470
{`\\host\share\foo\..\bar`, `\\host\share`},
14441471
{`//host/share/foo/../bar`, `\\host\share`},
1472+
{`//.`, `\\.`},
1473+
{`//./`, `\\.\`},
14451474
{`//./NUL`, `\\.\NUL`},
1446-
{`//?/NUL`, `\\?\NUL`},
1475+
{`//?/`, `\\?`},
1476+
{`//./a/b`, `\\.\a`},
1477+
{`//?/`, `\\?`},
1478+
{`//?/`, `\\?`},
14471479
{`//./C:`, `\\.\C:`},
1480+
{`//./C:/`, `\\.\C:`},
14481481
{`//./C:/a/b/c`, `\\.\C:`},
14491482
{`//./UNC/host/share/a/b/c`, `\\.\UNC\host\share`},
14501483
{`//./UNC/host`, `\\.\UNC\host`},
1484+
{`//./UNC/host\`, `\\.\UNC\host\`},
1485+
{`//./UNC`, `\\.\UNC`},
1486+
{`//./UNC/`, `\\.\UNC\`},
1487+
{`\\?\x`, `\\?`},
1488+
{`\??\x`, `\??`},
14511489
}
14521490

14531491
func TestVolumeName(t *testing.T) {
@@ -1720,3 +1758,28 @@ func TestIssue51617(t *testing.T) {
17201758
t.Errorf("got directories %v, want %v", saw, want)
17211759
}
17221760
}
1761+
1762+
func TestEscaping(t *testing.T) {
1763+
dir1 := t.TempDir()
1764+
dir2 := t.TempDir()
1765+
chdir(t, dir1)
1766+
1767+
for _, p := range []string{
1768+
filepath.Join(dir2, "x"),
1769+
} {
1770+
if !filepath.IsLocal(p) {
1771+
continue
1772+
}
1773+
f, err := os.Create(p)
1774+
if err != nil {
1775+
f.Close()
1776+
}
1777+
ents, err := os.ReadDir(dir2)
1778+
if err != nil {
1779+
t.Fatal(err)
1780+
}
1781+
for _, e := range ents {
1782+
t.Fatalf("found: %v", e.Name())
1783+
}
1784+
}
1785+
}

0 commit comments

Comments
 (0)