Skip to content

Commit cae35cd

Browse files
neildgopherbot
authored andcommitted
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. Fixes #63713 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://go-review.googlesource.com/c/go/+/540277 Reviewed-by: Cherry Mui <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Auto-Submit: Heschi Kreinick <[email protected]>
1 parent dc74a3d commit cae35cd

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
@@ -164,7 +164,7 @@ var depsRules = `
164164
165165
unicode, fmt !< net, os, os/signal;
166166
167-
os/signal, STR
167+
os/signal, internal/safefilepath, STR
168168
< path/filepath
169169
< io/ioutil;
170170

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
"slices"
2019
"sort"
2120
"strings"
@@ -168,21 +167,7 @@ func Clean(path string) string {
168167
out.append('.')
169168
}
170169

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

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
@@ -116,6 +116,9 @@ var wincleantests = []PathTest{
116116
{`a/../c:/a`, `.\c:\a`},
117117
{`a/../../c:`, `..\c:`},
118118
{`foo:bar`, `foo:bar`},
119+
120+
// Don't allow cleaning to create a Root Local Device path like \??\a.
121+
{`/a/../??/a`, `\.\??\a`},
119122
}
120123

121124
func TestClean(t *testing.T) {
@@ -177,8 +180,28 @@ var islocaltests = []IsLocalTest{
177180
var winislocaltests = []IsLocalTest{
178181
{"NUL", false},
179182
{"nul", false},
183+
{"nul ", false},
180184
{"nul.", false},
185+
{"a/nul:", false},
186+
{"a/nul : a", false},
187+
{"com0", true},
181188
{"com1", false},
189+
{"com2", false},
190+
{"com3", false},
191+
{"com4", false},
192+
{"com5", false},
193+
{"com6", false},
194+
{"com7", false},
195+
{"com8", false},
196+
{"com9", false},
197+
{"com¹", false},
198+
{"com²", false},
199+
{"com³", false},
200+
{"com¹ : a", false},
201+
{"cOm1", false},
202+
{"lpt1", false},
203+
{"LPT1", false},
204+
{"lpt³", false},
182205
{"./nul", false},
183206
{`\`, false},
184207
{`\a`, false},
@@ -384,6 +407,7 @@ var winjointests = []JoinTest{
384407
{[]string{`\\a\`, `b`, `c`}, `\\a\b\c`},
385408
{[]string{`//`, `a`}, `\\a`},
386409
{[]string{`a:\b\c`, `x\..\y:\..\..\z`}, `a:\b\z`},
410+
{[]string{`\`, `??\a`}, `\.\??\a`},
387411
}
388412

389413
func TestJoin(t *testing.T) {
@@ -1047,6 +1071,8 @@ var winisabstests = []IsAbsTest{
10471071
{`\\host\share\`, true},
10481072
{`\\host\share\foo`, true},
10491073
{`//host/share/foo/bar`, true},
1074+
{`\\?\a\b\c`, true},
1075+
{`\??\a\b\c`, true},
10501076
}
10511077

10521078
func TestIsAbs(t *testing.T) {
@@ -1547,7 +1573,8 @@ type VolumeNameTest struct {
15471573
var volumenametests = []VolumeNameTest{
15481574
{`c:/foo/bar`, `c:`},
15491575
{`c:`, `c:`},
1550-
{`2:`, ``},
1576+
{`c:\`, `c:`},
1577+
{`2:`, `2:`},
15511578
{``, ``},
15521579
{`\\\host`, `\\\host`},
15531580
{`\\\host\`, `\\\host`},
@@ -1567,12 +1594,23 @@ var volumenametests = []VolumeNameTest{
15671594
{`//host/share//foo///bar////baz`, `\\host\share`},
15681595
{`\\host\share\foo\..\bar`, `\\host\share`},
15691596
{`//host/share/foo/../bar`, `\\host\share`},
1597+
{`//.`, `\\.`},
1598+
{`//./`, `\\.\`},
15701599
{`//./NUL`, `\\.\NUL`},
1571-
{`//?/NUL`, `\\?\NUL`},
1600+
{`//?/`, `\\?`},
1601+
{`//./a/b`, `\\.\a`},
1602+
{`//?/`, `\\?`},
1603+
{`//?/`, `\\?`},
15721604
{`//./C:`, `\\.\C:`},
1605+
{`//./C:/`, `\\.\C:`},
15731606
{`//./C:/a/b/c`, `\\.\C:`},
15741607
{`//./UNC/host/share/a/b/c`, `\\.\UNC\host\share`},
15751608
{`//./UNC/host`, `\\.\UNC\host`},
1609+
{`//./UNC/host\`, `\\.\UNC\host\`},
1610+
{`//./UNC`, `\\.\UNC`},
1611+
{`//./UNC/`, `\\.\UNC\`},
1612+
{`\\?\x`, `\\?`},
1613+
{`\??\x`, `\??`},
15761614
}
15771615

15781616
func TestVolumeName(t *testing.T) {
@@ -1842,3 +1880,28 @@ func TestIssue51617(t *testing.T) {
18421880
t.Errorf("got directories %v, want %v", saw, want)
18431881
}
18441882
}
1883+
1884+
func TestEscaping(t *testing.T) {
1885+
dir1 := t.TempDir()
1886+
dir2 := t.TempDir()
1887+
chdir(t, dir1)
1888+
1889+
for _, p := range []string{
1890+
filepath.Join(dir2, "x"),
1891+
} {
1892+
if !filepath.IsLocal(p) {
1893+
continue
1894+
}
1895+
f, err := os.Create(p)
1896+
if err != nil {
1897+
f.Close()
1898+
}
1899+
ents, err := os.ReadDir(dir2)
1900+
if err != nil {
1901+
t.Fatal(err)
1902+
}
1903+
for _, e := range ents {
1904+
t.Fatalf("found: %v", e.Name())
1905+
}
1906+
}
1907+
}

0 commit comments

Comments
 (0)