1From f8896a97a0630b0f2f8c488310147f7f20b3ec7d Mon Sep 17 00:00:00 2001 2From: Damien Neil <dneil@google.com> 3Date: Thu, 10 Nov 2022 12:16:27 -0800 4Subject: [PATCH] os, net/http: avoid escapes from os.DirFS and http.Dir on 5 Windows 6 7Do not permit access to Windows reserved device names (NUL, COM1, etc.) 8via os.DirFS and http.Dir filesystems. 9 10Avoid escapes from os.DirFS(`\`) on Windows. DirFS would join the 11the root to the relative path with a path separator, making 12os.DirFS(`\`).Open(`/foo/bar`) open the path `\\foo\bar`, which is 13a UNC name. Not only does this not open the intended file, but permits 14reference to any file on the system rather than only files on the 15current drive. 16 17Make os.DirFS("") invalid, with all file access failing. Previously, 18a root of "" was interpreted as "/", which is surprising and probably 19unintentional. 20 21Fixes CVE-2022-41720. 22Fixes #56694. 23 24Change-Id: I275b5fa391e6ad7404309ea98ccc97405942e0f0 25Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1663832 26Reviewed-by: Julie Qiu <julieqiu@google.com> 27Reviewed-by: Tatiana Bradley <tatianabradley@google.com> 28Reviewed-on: https://go-review.googlesource.com/c/go/+/455360 29Reviewed-by: Michael Pratt <mpratt@google.com> 30TryBot-Result: Gopher Robot <gobot@golang.org> 31Run-TryBot: Jenny Rakoczy <jenny@golang.org> 32 33CVE: CVE-2022-41720 34Upstream-Status: Backport [7013a4f5f816af62033ad63dd06b77c30d7a62a7] 35Signed-off-by: Sakib Sajal <sakib.sajal@windriver.com> 36--- 37 src/go/build/deps_test.go | 1 + 38 src/internal/safefilepath/path.go | 21 +++++ 39 src/internal/safefilepath/path_other.go | 23 ++++++ 40 src/internal/safefilepath/path_test.go | 88 +++++++++++++++++++++ 41 src/internal/safefilepath/path_windows.go | 95 +++++++++++++++++++++++ 42 src/net/http/fs.go | 8 +- 43 src/net/http/fs_test.go | 28 +++++++ 44 src/os/file.go | 36 +++++++-- 45 src/os/os_test.go | 38 +++++++++ 46 9 files changed, 328 insertions(+), 10 deletions(-) 47 create mode 100644 src/internal/safefilepath/path.go 48 create mode 100644 src/internal/safefilepath/path_other.go 49 create mode 100644 src/internal/safefilepath/path_test.go 50 create mode 100644 src/internal/safefilepath/path_windows.go 51 52diff --git a/src/go/build/deps_test.go b/src/go/build/deps_test.go 53index 45e2f25..dc3bb8c 100644 54--- a/src/go/build/deps_test.go 55+++ b/src/go/build/deps_test.go 56@@ -165,6 +165,7 @@ var depsRules = ` 57 io/fs 58 < internal/testlog 59 < internal/poll 60+ < internal/safefilepath 61 < os 62 < os/signal; 63 64diff --git a/src/internal/safefilepath/path.go b/src/internal/safefilepath/path.go 65new file mode 100644 66index 0000000..0f0a270 67--- /dev/null 68+++ b/src/internal/safefilepath/path.go 69@@ -0,0 +1,21 @@ 70+// Copyright 2022 The Go Authors. All rights reserved. 71+// Use of this source code is governed by a BSD-style 72+// license that can be found in the LICENSE file. 73+ 74+// Package safefilepath manipulates operating-system file paths. 75+package safefilepath 76+ 77+import ( 78+ "errors" 79+) 80+ 81+var errInvalidPath = errors.New("invalid path") 82+ 83+// FromFS converts a slash-separated path into an operating-system path. 84+// 85+// FromFS returns an error if the path cannot be represented by the operating 86+// system. For example, paths containing '\' and ':' characters are rejected 87+// on Windows. 88+func FromFS(path string) (string, error) { 89+ return fromFS(path) 90+} 91diff --git a/src/internal/safefilepath/path_other.go b/src/internal/safefilepath/path_other.go 92new file mode 100644 93index 0000000..f93da18 94--- /dev/null 95+++ b/src/internal/safefilepath/path_other.go 96@@ -0,0 +1,23 @@ 97+// Copyright 2022 The Go Authors. All rights reserved. 98+// Use of this source code is governed by a BSD-style 99+// license that can be found in the LICENSE file. 100+ 101+//go:build !windows 102+ 103+package safefilepath 104+ 105+import "runtime" 106+ 107+func fromFS(path string) (string, error) { 108+ if runtime.GOOS == "plan9" { 109+ if len(path) > 0 && path[0] == '#' { 110+ return path, errInvalidPath 111+ } 112+ } 113+ for i := range path { 114+ if path[i] == 0 { 115+ return "", errInvalidPath 116+ } 117+ } 118+ return path, nil 119+} 120diff --git a/src/internal/safefilepath/path_test.go b/src/internal/safefilepath/path_test.go 121new file mode 100644 122index 0000000..dc662c1 123--- /dev/null 124+++ b/src/internal/safefilepath/path_test.go 125@@ -0,0 +1,88 @@ 126+// Copyright 2022 The Go Authors. All rights reserved. 127+// Use of this source code is governed by a BSD-style 128+// license that can be found in the LICENSE file. 129+ 130+package safefilepath_test 131+ 132+import ( 133+ "internal/safefilepath" 134+ "os" 135+ "path/filepath" 136+ "runtime" 137+ "testing" 138+) 139+ 140+type PathTest struct { 141+ path, result string 142+} 143+ 144+const invalid = "" 145+ 146+var fspathtests = []PathTest{ 147+ {".", "."}, 148+ {"/a/b/c", "/a/b/c"}, 149+ {"a\x00b", invalid}, 150+} 151+ 152+var winreservedpathtests = []PathTest{ 153+ {`a\b`, `a\b`}, 154+ {`a:b`, `a:b`}, 155+ {`a/b:c`, `a/b:c`}, 156+ {`NUL`, `NUL`}, 157+ {`./com1`, `./com1`}, 158+ {`a/nul/b`, `a/nul/b`}, 159+} 160+ 161+// Whether a reserved name with an extension is reserved or not varies by 162+// Windows version. 163+var winreservedextpathtests = []PathTest{ 164+ {"nul.txt", "nul.txt"}, 165+ {"a/nul.txt/b", "a/nul.txt/b"}, 166+} 167+ 168+var plan9reservedpathtests = []PathTest{ 169+ {`#c`, `#c`}, 170+} 171+ 172+func TestFromFS(t *testing.T) { 173+ switch runtime.GOOS { 174+ case "windows": 175+ if canWriteFile(t, "NUL") { 176+ t.Errorf("can unexpectedly write a file named NUL on Windows") 177+ } 178+ if canWriteFile(t, "nul.txt") { 179+ fspathtests = append(fspathtests, winreservedextpathtests...) 180+ } else { 181+ winreservedpathtests = append(winreservedpathtests, winreservedextpathtests...) 182+ } 183+ for i := range winreservedpathtests { 184+ winreservedpathtests[i].result = invalid 185+ } 186+ for i := range fspathtests { 187+ fspathtests[i].result = filepath.FromSlash(fspathtests[i].result) 188+ } 189+ case "plan9": 190+ for i := range plan9reservedpathtests { 191+ plan9reservedpathtests[i].result = invalid 192+ } 193+ } 194+ tests := fspathtests 195+ tests = append(tests, winreservedpathtests...) 196+ tests = append(tests, plan9reservedpathtests...) 197+ for _, test := range tests { 198+ got, err := safefilepath.FromFS(test.path) 199+ if (got == "") != (err != nil) { 200+ t.Errorf(`FromFS(%q) = %q, %v; want "" only if err != nil`, test.path, got, err) 201+ } 202+ if got != test.result { 203+ t.Errorf("FromFS(%q) = %q, %v; want %q", test.path, got, err, test.result) 204+ } 205+ } 206+} 207+ 208+func canWriteFile(t *testing.T, name string) bool { 209+ path := filepath.Join(t.TempDir(), name) 210+ os.WriteFile(path, []byte("ok"), 0666) 211+ b, _ := os.ReadFile(path) 212+ return string(b) == "ok" 213+} 214diff --git a/src/internal/safefilepath/path_windows.go b/src/internal/safefilepath/path_windows.go 215new file mode 100644 216index 0000000..909c150 217--- /dev/null 218+++ b/src/internal/safefilepath/path_windows.go 219@@ -0,0 +1,95 @@ 220+// Copyright 2022 The Go Authors. All rights reserved. 221+// Use of this source code is governed by a BSD-style 222+// license that can be found in the LICENSE file. 223+ 224+package safefilepath 225+ 226+import ( 227+ "syscall" 228+ "unicode/utf8" 229+) 230+ 231+func fromFS(path string) (string, error) { 232+ if !utf8.ValidString(path) { 233+ return "", errInvalidPath 234+ } 235+ for len(path) > 1 && path[0] == '/' && path[1] == '/' { 236+ path = path[1:] 237+ } 238+ containsSlash := false 239+ for p := path; p != ""; { 240+ // Find the next path element. 241+ i := 0 242+ dot := -1 243+ for i < len(p) && p[i] != '/' { 244+ switch p[i] { 245+ case 0, '\\', ':': 246+ return "", errInvalidPath 247+ case '.': 248+ if dot < 0 { 249+ dot = i 250+ } 251+ } 252+ i++ 253+ } 254+ part := p[:i] 255+ if i < len(p) { 256+ containsSlash = true 257+ p = p[i+1:] 258+ } else { 259+ p = "" 260+ } 261+ // Trim the extension and look for a reserved name. 262+ base := part 263+ if dot >= 0 { 264+ base = part[:dot] 265+ } 266+ if isReservedName(base) { 267+ if dot < 0 { 268+ return "", errInvalidPath 269+ } 270+ // The path element is a reserved name with an extension. 271+ // Some Windows versions consider this a reserved name, 272+ // while others do not. Use FullPath to see if the name is 273+ // reserved. 274+ if p, _ := syscall.FullPath(part); len(p) >= 4 && p[:4] == `\\.\` { 275+ return "", errInvalidPath 276+ } 277+ } 278+ } 279+ if containsSlash { 280+ // We can't depend on strings, so substitute \ for / manually. 281+ buf := []byte(path) 282+ for i, b := range buf { 283+ if b == '/' { 284+ buf[i] = '\\' 285+ } 286+ } 287+ path = string(buf) 288+ } 289+ return path, nil 290+} 291+ 292+// isReservedName reports if name is a Windows reserved device name. 293+// It does not detect names with an extension, which are also reserved on some Windows versions. 294+// 295+// For details, search for PRN in 296+// https://docs.microsoft.com/en-us/windows/desktop/fileio/naming-a-file. 297+func isReservedName(name string) bool { 298+ if 3 <= len(name) && len(name) <= 4 { 299+ switch string([]byte{toUpper(name[0]), toUpper(name[1]), toUpper(name[2])}) { 300+ case "CON", "PRN", "AUX", "NUL": 301+ return len(name) == 3 302+ case "COM", "LPT": 303+ return len(name) == 4 && '1' <= name[3] && name[3] <= '9' 304+ } 305+ } 306+ return false 307+} 308+ 309+func toUpper(c byte) byte { 310+ if 'a' <= c && c <= 'z' { 311+ return c - ('a' - 'A') 312+ } 313+ return c 314+} 315diff --git a/src/net/http/fs.go b/src/net/http/fs.go 316index 57e731e..43ee4b5 100644 317--- a/src/net/http/fs.go 318+++ b/src/net/http/fs.go 319@@ -9,6 +9,7 @@ package http 320 import ( 321 "errors" 322 "fmt" 323+ "internal/safefilepath" 324 "io" 325 "io/fs" 326 "mime" 327@@ -69,14 +70,15 @@ func mapDirOpenError(originalErr error, name string) error { 328 // Open implements FileSystem using os.Open, opening files for reading rooted 329 // and relative to the directory d. 330 func (d Dir) Open(name string) (File, error) { 331- if filepath.Separator != '/' && strings.ContainsRune(name, filepath.Separator) { 332- return nil, errors.New("http: invalid character in file path") 333+ path, err := safefilepath.FromFS(path.Clean("/" + name)) 334+ if err != nil { 335+ return nil, errors.New("http: invalid or unsafe file path") 336 } 337 dir := string(d) 338 if dir == "" { 339 dir = "." 340 } 341- fullName := filepath.Join(dir, filepath.FromSlash(path.Clean("/"+name))) 342+ fullName := filepath.Join(dir, path) 343 f, err := os.Open(fullName) 344 if err != nil { 345 return nil, mapDirOpenError(err, fullName) 346diff --git a/src/net/http/fs_test.go b/src/net/http/fs_test.go 347index b42ade1..941448a 100644 348--- a/src/net/http/fs_test.go 349+++ b/src/net/http/fs_test.go 350@@ -648,6 +648,34 @@ func TestFileServerZeroByte(t *testing.T) { 351 } 352 } 353 354+func TestFileServerNamesEscape(t *testing.T) { 355+ t.Run("h1", func(t *testing.T) { 356+ testFileServerNamesEscape(t, h1Mode) 357+ }) 358+ t.Run("h2", func(t *testing.T) { 359+ testFileServerNamesEscape(t, h2Mode) 360+ }) 361+} 362+func testFileServerNamesEscape(t *testing.T, h2 bool) { 363+ defer afterTest(t) 364+ ts := newClientServerTest(t, h2, FileServer(Dir("testdata"))).ts 365+ defer ts.Close() 366+ for _, path := range []string{ 367+ "/../testdata/file", 368+ "/NUL", // don't read from device files on Windows 369+ } { 370+ res, err := ts.Client().Get(ts.URL + path) 371+ if err != nil { 372+ t.Fatal(err) 373+ } 374+ res.Body.Close() 375+ if res.StatusCode < 400 || res.StatusCode > 599 { 376+ t.Errorf("Get(%q): got status %v, want 4xx or 5xx", path, res.StatusCode) 377+ } 378+ 379+ } 380+} 381+ 382 type fakeFileInfo struct { 383 dir bool 384 basename string 385diff --git a/src/os/file.go b/src/os/file.go 386index e717f17..cb87158 100644 387--- a/src/os/file.go 388+++ b/src/os/file.go 389@@ -37,12 +37,12 @@ 390 // Note: The maximum number of concurrent operations on a File may be limited by 391 // the OS or the system. The number should be high, but exceeding it may degrade 392 // performance or cause other issues. 393-// 394 package os 395 396 import ( 397 "errors" 398 "internal/poll" 399+ "internal/safefilepath" 400 "internal/testlog" 401 "internal/unsafeheader" 402 "io" 403@@ -623,6 +623,8 @@ func isWindowsNulName(name string) bool { 404 // the /prefix tree, then using DirFS does not stop the access any more than using 405 // os.Open does. DirFS is therefore not a general substitute for a chroot-style security 406 // mechanism when the directory tree contains arbitrary content. 407+// 408+// The directory dir must not be "". 409 func DirFS(dir string) fs.FS { 410 return dirFS(dir) 411 } 412@@ -641,10 +643,11 @@ func containsAny(s, chars string) bool { 413 type dirFS string 414 415 func (dir dirFS) Open(name string) (fs.File, error) { 416- if !fs.ValidPath(name) || runtime.GOOS == "windows" && containsAny(name, `\:`) { 417- return nil, &PathError{Op: "open", Path: name, Err: ErrInvalid} 418+ fullname, err := dir.join(name) 419+ if err != nil { 420+ return nil, &PathError{Op: "stat", Path: name, Err: err} 421 } 422- f, err := Open(string(dir) + "/" + name) 423+ f, err := Open(fullname) 424 if err != nil { 425 return nil, err // nil fs.File 426 } 427@@ -652,16 +655,35 @@ func (dir dirFS) Open(name string) (fs.File, error) { 428 } 429 430 func (dir dirFS) Stat(name string) (fs.FileInfo, error) { 431- if !fs.ValidPath(name) || runtime.GOOS == "windows" && containsAny(name, `\:`) { 432- return nil, &PathError{Op: "stat", Path: name, Err: ErrInvalid} 433+ fullname, err := dir.join(name) 434+ if err != nil { 435+ return nil, &PathError{Op: "stat", Path: name, Err: err} 436 } 437- f, err := Stat(string(dir) + "/" + name) 438+ f, err := Stat(fullname) 439 if err != nil { 440 return nil, err 441 } 442 return f, nil 443 } 444 445+// join returns the path for name in dir. 446+func (dir dirFS) join(name string) (string, error) { 447+ if dir == "" { 448+ return "", errors.New("os: DirFS with empty root") 449+ } 450+ if !fs.ValidPath(name) { 451+ return "", ErrInvalid 452+ } 453+ name, err := safefilepath.FromFS(name) 454+ if err != nil { 455+ return "", ErrInvalid 456+ } 457+ if IsPathSeparator(dir[len(dir)-1]) { 458+ return string(dir) + name, nil 459+ } 460+ return string(dir) + string(PathSeparator) + name, nil 461+} 462+ 463 // ReadFile reads the named file and returns the contents. 464 // A successful call returns err == nil, not err == EOF. 465 // Because ReadFile reads the whole file, it does not treat an EOF from Read 466diff --git a/src/os/os_test.go b/src/os/os_test.go 467index 506f1fb..be269bb 100644 468--- a/src/os/os_test.go 469+++ b/src/os/os_test.go 470@@ -2702,6 +2702,44 @@ func TestDirFS(t *testing.T) { 471 if err == nil { 472 t.Fatalf(`Open testdata\dirfs succeeded`) 473 } 474+ 475+ // Test that Open does not open Windows device files. 476+ _, err = d.Open(`NUL`) 477+ if err == nil { 478+ t.Errorf(`Open NUL succeeded`) 479+ } 480+} 481+ 482+func TestDirFSRootDir(t *testing.T) { 483+ cwd, err := os.Getwd() 484+ if err != nil { 485+ t.Fatal(err) 486+ } 487+ cwd = cwd[len(filepath.VolumeName(cwd)):] // trim volume prefix (C:) on Windows 488+ cwd = filepath.ToSlash(cwd) // convert \ to / 489+ cwd = strings.TrimPrefix(cwd, "/") // trim leading / 490+ 491+ // Test that Open can open a path starting at /. 492+ d := DirFS("/") 493+ f, err := d.Open(cwd + "/testdata/dirfs/a") 494+ if err != nil { 495+ t.Fatal(err) 496+ } 497+ f.Close() 498+} 499+ 500+func TestDirFSEmptyDir(t *testing.T) { 501+ d := DirFS("") 502+ cwd, _ := os.Getwd() 503+ for _, path := range []string{ 504+ "testdata/dirfs/a", // not DirFS(".") 505+ filepath.ToSlash(cwd) + "/testdata/dirfs/a", // not DirFS("/") 506+ } { 507+ _, err := d.Open(path) 508+ if err == nil { 509+ t.Fatalf(`DirFS("").Open(%q) succeeded`, path) 510+ } 511+ } 512 } 513 514 func TestDirFSPathsValid(t *testing.T) { 515