]> Cypherpunks repositories - gostls13.git/commitdiff
[release-branch.go1.18] os, net/http: avoid escapes from os.DirFS and http.Dir on...
authorDamien Neil <dneil@google.com>
Thu, 10 Nov 2022 20:16:27 +0000 (12:16 -0800)
committerJenny Rakoczy <jenny@golang.org>
Tue, 6 Dec 2022 19:00:21 +0000 (19:00 +0000)
Do not permit access to Windows reserved device names (NUL, COM1, etc.)
via os.DirFS and http.Dir filesystems.

Avoid escapes from os.DirFS(`\`) on Windows. DirFS would join the
the root to the relative path with a path separator, making
os.DirFS(`\`).Open(`/foo/bar`) open the path `\\foo\bar`, which is
a UNC name. Not only does this not open the intended file, but permits
reference to any file on the system rather than only files on the
current drive.

Make os.DirFS("") invalid, with all file access failing. Previously,
a root of "" was interpreted as "/", which is surprising and probably
unintentional.

Fixes CVE-2022-41720.
Fixes #56694.

Change-Id: I275b5fa391e6ad7404309ea98ccc97405942e0f0
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1663832
Reviewed-by: Julie Qiu <julieqiu@google.com>
Reviewed-by: Tatiana Bradley <tatianabradley@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/455360
Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Jenny Rakoczy <jenny@golang.org>

src/go/build/deps_test.go
src/internal/safefilepath/path.go [new file with mode: 0644]
src/internal/safefilepath/path_other.go [new file with mode: 0644]
src/internal/safefilepath/path_test.go [new file with mode: 0644]
src/internal/safefilepath/path_windows.go [new file with mode: 0644]
src/net/http/fs.go
src/net/http/fs_test.go
src/os/file.go
src/os/os_test.go

index 72465659dc24e871b06fc56f923aba2a3bf73e5c..20a3d0fb86aa18bc1a6f7ae664cb91b27975a475 100644 (file)
@@ -168,6 +168,7 @@ var depsRules = `
        io/fs
        < internal/testlog
        < internal/poll
+       < internal/safefilepath
        < os
        < os/signal;
 
diff --git a/src/internal/safefilepath/path.go b/src/internal/safefilepath/path.go
new file mode 100644 (file)
index 0000000..0f0a270
--- /dev/null
@@ -0,0 +1,21 @@
+// Copyright 2022 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Package safefilepath manipulates operating-system file paths.
+package safefilepath
+
+import (
+       "errors"
+)
+
+var errInvalidPath = errors.New("invalid path")
+
+// FromFS converts a slash-separated path into an operating-system path.
+//
+// FromFS returns an error if the path cannot be represented by the operating
+// system. For example, paths containing '\' and ':' characters are rejected
+// on Windows.
+func FromFS(path string) (string, error) {
+       return fromFS(path)
+}
diff --git a/src/internal/safefilepath/path_other.go b/src/internal/safefilepath/path_other.go
new file mode 100644 (file)
index 0000000..f93da18
--- /dev/null
@@ -0,0 +1,23 @@
+// Copyright 2022 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+//go:build !windows
+
+package safefilepath
+
+import "runtime"
+
+func fromFS(path string) (string, error) {
+       if runtime.GOOS == "plan9" {
+               if len(path) > 0 && path[0] == '#' {
+                       return path, errInvalidPath
+               }
+       }
+       for i := range path {
+               if path[i] == 0 {
+                       return "", errInvalidPath
+               }
+       }
+       return path, nil
+}
diff --git a/src/internal/safefilepath/path_test.go b/src/internal/safefilepath/path_test.go
new file mode 100644 (file)
index 0000000..dc662c1
--- /dev/null
@@ -0,0 +1,88 @@
+// Copyright 2022 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package safefilepath_test
+
+import (
+       "internal/safefilepath"
+       "os"
+       "path/filepath"
+       "runtime"
+       "testing"
+)
+
+type PathTest struct {
+       path, result string
+}
+
+const invalid = ""
+
+var fspathtests = []PathTest{
+       {".", "."},
+       {"/a/b/c", "/a/b/c"},
+       {"a\x00b", invalid},
+}
+
+var winreservedpathtests = []PathTest{
+       {`a\b`, `a\b`},
+       {`a:b`, `a:b`},
+       {`a/b:c`, `a/b:c`},
+       {`NUL`, `NUL`},
+       {`./com1`, `./com1`},
+       {`a/nul/b`, `a/nul/b`},
+}
+
+// Whether a reserved name with an extension is reserved or not varies by
+// Windows version.
+var winreservedextpathtests = []PathTest{
+       {"nul.txt", "nul.txt"},
+       {"a/nul.txt/b", "a/nul.txt/b"},
+}
+
+var plan9reservedpathtests = []PathTest{
+       {`#c`, `#c`},
+}
+
+func TestFromFS(t *testing.T) {
+       switch runtime.GOOS {
+       case "windows":
+               if canWriteFile(t, "NUL") {
+                       t.Errorf("can unexpectedly write a file named NUL on Windows")
+               }
+               if canWriteFile(t, "nul.txt") {
+                       fspathtests = append(fspathtests, winreservedextpathtests...)
+               } else {
+                       winreservedpathtests = append(winreservedpathtests, winreservedextpathtests...)
+               }
+               for i := range winreservedpathtests {
+                       winreservedpathtests[i].result = invalid
+               }
+               for i := range fspathtests {
+                       fspathtests[i].result = filepath.FromSlash(fspathtests[i].result)
+               }
+       case "plan9":
+               for i := range plan9reservedpathtests {
+                       plan9reservedpathtests[i].result = invalid
+               }
+       }
+       tests := fspathtests
+       tests = append(tests, winreservedpathtests...)
+       tests = append(tests, plan9reservedpathtests...)
+       for _, test := range tests {
+               got, err := safefilepath.FromFS(test.path)
+               if (got == "") != (err != nil) {
+                       t.Errorf(`FromFS(%q) = %q, %v; want "" only if err != nil`, test.path, got, err)
+               }
+               if got != test.result {
+                       t.Errorf("FromFS(%q) = %q, %v; want %q", test.path, got, err, test.result)
+               }
+       }
+}
+
+func canWriteFile(t *testing.T, name string) bool {
+       path := filepath.Join(t.TempDir(), name)
+       os.WriteFile(path, []byte("ok"), 0666)
+       b, _ := os.ReadFile(path)
+       return string(b) == "ok"
+}
diff --git a/src/internal/safefilepath/path_windows.go b/src/internal/safefilepath/path_windows.go
new file mode 100644 (file)
index 0000000..909c150
--- /dev/null
@@ -0,0 +1,95 @@
+// Copyright 2022 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package safefilepath
+
+import (
+       "syscall"
+       "unicode/utf8"
+)
+
+func fromFS(path string) (string, error) {
+       if !utf8.ValidString(path) {
+               return "", errInvalidPath
+       }
+       for len(path) > 1 && path[0] == '/' && path[1] == '/' {
+               path = path[1:]
+       }
+       containsSlash := false
+       for p := path; p != ""; {
+               // Find the next path element.
+               i := 0
+               dot := -1
+               for i < len(p) && p[i] != '/' {
+                       switch p[i] {
+                       case 0, '\\', ':':
+                               return "", errInvalidPath
+                       case '.':
+                               if dot < 0 {
+                                       dot = i
+                               }
+                       }
+                       i++
+               }
+               part := p[:i]
+               if i < len(p) {
+                       containsSlash = true
+                       p = p[i+1:]
+               } else {
+                       p = ""
+               }
+               // Trim the extension and look for a reserved name.
+               base := part
+               if dot >= 0 {
+                       base = part[:dot]
+               }
+               if isReservedName(base) {
+                       if dot < 0 {
+                               return "", errInvalidPath
+                       }
+                       // The path element is a reserved name with an extension.
+                       // Some Windows versions consider this a reserved name,
+                       // while others do not. Use FullPath to see if the name is
+                       // reserved.
+                       if p, _ := syscall.FullPath(part); len(p) >= 4 && p[:4] == `\\.\` {
+                               return "", errInvalidPath
+                       }
+               }
+       }
+       if containsSlash {
+               // We can't depend on strings, so substitute \ for / manually.
+               buf := []byte(path)
+               for i, b := range buf {
+                       if b == '/' {
+                               buf[i] = '\\'
+                       }
+               }
+               path = string(buf)
+       }
+       return path, nil
+}
+
+// isReservedName reports if name is a Windows reserved device name.
+// It does not detect names with an extension, which are also reserved on some Windows versions.
+//
+// For details, search for PRN in
+// https://docs.microsoft.com/en-us/windows/desktop/fileio/naming-a-file.
+func isReservedName(name string) bool {
+       if 3 <= len(name) && len(name) <= 4 {
+               switch string([]byte{toUpper(name[0]), toUpper(name[1]), toUpper(name[2])}) {
+               case "CON", "PRN", "AUX", "NUL":
+                       return len(name) == 3
+               case "COM", "LPT":
+                       return len(name) == 4 && '1' <= name[3] && name[3] <= '9'
+               }
+       }
+       return false
+}
+
+func toUpper(c byte) byte {
+       if 'a' <= c && c <= 'z' {
+               return c - ('a' - 'A')
+       }
+       return c
+}
index 6caee9ed931e871337a56687c380bd8dc700c415..583203043f3ad170d3b2f9c3532f0d8923cbd76c 100644 (file)
@@ -9,6 +9,7 @@ package http
 import (
        "errors"
        "fmt"
+       "internal/safefilepath"
        "io"
        "io/fs"
        "mime"
@@ -69,14 +70,15 @@ func mapOpenError(originalErr error, name string, sep rune, stat func(string) (f
 // Open implements FileSystem using os.Open, opening files for reading rooted
 // and relative to the directory d.
 func (d Dir) Open(name string) (File, error) {
-       if filepath.Separator != '/' && strings.ContainsRune(name, filepath.Separator) {
-               return nil, errors.New("http: invalid character in file path")
+       path, err := safefilepath.FromFS(path.Clean("/" + name))
+       if err != nil {
+               return nil, errors.New("http: invalid or unsafe file path")
        }
        dir := string(d)
        if dir == "" {
                dir = "."
        }
-       fullName := filepath.Join(dir, filepath.FromSlash(path.Clean("/"+name)))
+       fullName := filepath.Join(dir, path)
        f, err := os.Open(fullName)
        if err != nil {
                return nil, mapOpenError(err, fullName, filepath.Separator, os.Stat)
index d627dfd4be965287cb1d929ba8dc5621c02525cc..323360d55014d02258847817f925e0e9fce8e679 100644 (file)
@@ -648,6 +648,34 @@ func TestFileServerZeroByte(t *testing.T) {
        }
 }
 
+func TestFileServerNamesEscape(t *testing.T) {
+       t.Run("h1", func(t *testing.T) {
+               testFileServerNamesEscape(t, h1Mode)
+       })
+       t.Run("h2", func(t *testing.T) {
+               testFileServerNamesEscape(t, h2Mode)
+       })
+}
+func testFileServerNamesEscape(t *testing.T, h2 bool) {
+       defer afterTest(t)
+       ts := newClientServerTest(t, h2, FileServer(Dir("testdata"))).ts
+       defer ts.Close()
+       for _, path := range []string{
+               "/../testdata/file",
+               "/NUL", // don't read from device files on Windows
+       } {
+               res, err := ts.Client().Get(ts.URL + path)
+               if err != nil {
+                       t.Fatal(err)
+               }
+               res.Body.Close()
+               if res.StatusCode < 400 || res.StatusCode > 599 {
+                       t.Errorf("Get(%q): got status %v, want 4xx or 5xx", path, res.StatusCode)
+               }
+
+       }
+}
+
 type fakeFileInfo struct {
        dir      bool
        basename string
index 28231285542a19b8ffd6d545ffb5d460cc3c8fe1..8afc6e3540937cf7e1d355ace95082fc473e6c24 100644 (file)
 // Note: The maximum number of concurrent operations on a File may be limited by
 // the OS or the system. The number should be high, but exceeding it may degrade
 // performance or cause other issues.
-//
 package os
 
 import (
        "errors"
        "internal/poll"
+       "internal/safefilepath"
        "internal/testlog"
        "internal/unsafeheader"
        "io"
@@ -623,6 +623,8 @@ func isWindowsNulName(name string) bool {
 // the /prefix tree, then using DirFS does not stop the access any more than using
 // os.Open does. DirFS is therefore not a general substitute for a chroot-style security
 // mechanism when the directory tree contains arbitrary content.
+//
+// The directory dir must not be "".
 func DirFS(dir string) fs.FS {
        return dirFS(dir)
 }
@@ -641,10 +643,11 @@ func containsAny(s, chars string) bool {
 type dirFS string
 
 func (dir dirFS) Open(name string) (fs.File, error) {
-       if !fs.ValidPath(name) || runtime.GOOS == "windows" && containsAny(name, `\:`) {
-               return nil, &PathError{Op: "open", Path: name, Err: ErrInvalid}
+       fullname, err := dir.join(name)
+       if err != nil {
+               return nil, &PathError{Op: "stat", Path: name, Err: err}
        }
-       f, err := Open(string(dir) + "/" + name)
+       f, err := Open(fullname)
        if err != nil {
                return nil, err // nil fs.File
        }
@@ -652,16 +655,35 @@ func (dir dirFS) Open(name string) (fs.File, error) {
 }
 
 func (dir dirFS) Stat(name string) (fs.FileInfo, error) {
-       if !fs.ValidPath(name) || runtime.GOOS == "windows" && containsAny(name, `\:`) {
-               return nil, &PathError{Op: "stat", Path: name, Err: ErrInvalid}
+       fullname, err := dir.join(name)
+       if err != nil {
+               return nil, &PathError{Op: "stat", Path: name, Err: err}
        }
-       f, err := Stat(string(dir) + "/" + name)
+       f, err := Stat(fullname)
        if err != nil {
                return nil, err
        }
        return f, nil
 }
 
+// join returns the path for name in dir.
+func (dir dirFS) join(name string) (string, error) {
+       if dir == "" {
+               return "", errors.New("os: DirFS with empty root")
+       }
+       if !fs.ValidPath(name) {
+               return "", ErrInvalid
+       }
+       name, err := safefilepath.FromFS(name)
+       if err != nil {
+               return "", ErrInvalid
+       }
+       if IsPathSeparator(dir[len(dir)-1]) {
+               return string(dir) + name, nil
+       }
+       return string(dir) + string(PathSeparator) + name, nil
+}
+
 // ReadFile reads the named file and returns the contents.
 // A successful call returns err == nil, not err == EOF.
 // Because ReadFile reads the whole file, it does not treat an EOF from Read
index 63427deb6e01cfbd0f3b8051428ffd5f817f2141..4124be13cc5421d5c0809405e7242830fb62aa03 100644 (file)
@@ -2696,6 +2696,44 @@ func TestDirFS(t *testing.T) {
        if err == nil {
                t.Fatalf(`Open testdata\dirfs succeeded`)
        }
+
+       // Test that Open does not open Windows device files.
+       _, err = d.Open(`NUL`)
+       if err == nil {
+               t.Errorf(`Open NUL succeeded`)
+       }
+}
+
+func TestDirFSRootDir(t *testing.T) {
+       cwd, err := os.Getwd()
+       if err != nil {
+               t.Fatal(err)
+       }
+       cwd = cwd[len(filepath.VolumeName(cwd)):] // trim volume prefix (C:) on Windows
+       cwd = filepath.ToSlash(cwd)               // convert \ to /
+       cwd = strings.TrimPrefix(cwd, "/")        // trim leading /
+
+       // Test that Open can open a path starting at /.
+       d := DirFS("/")
+       f, err := d.Open(cwd + "/testdata/dirfs/a")
+       if err != nil {
+               t.Fatal(err)
+       }
+       f.Close()
+}
+
+func TestDirFSEmptyDir(t *testing.T) {
+       d := DirFS("")
+       cwd, _ := os.Getwd()
+       for _, path := range []string{
+               "testdata/dirfs/a",                          // not DirFS(".")
+               filepath.ToSlash(cwd) + "/testdata/dirfs/a", // not DirFS("/")
+       } {
+               _, err := d.Open(path)
+               if err == nil {
+                       t.Fatalf(`DirFS("").Open(%q) succeeded`, path)
+               }
+       }
 }
 
 func TestDirFSPathsValid(t *testing.T) {