]> Cypherpunks repositories - gostls13.git/commitdiff
[release-branch.go1.11-security] cmd/go: reject 'get' of paths containing leading...
authorBryan C. Mills <bcmills@google.com>
Fri, 30 Nov 2018 19:04:35 +0000 (14:04 -0500)
committerBryan C. Mills <bcmills@google.com>
Fri, 7 Dec 2018 14:48:35 +0000 (14:48 +0000)
On some platforms, directories beginning with dot are treated as
hidden files, and filenames containing unusual characters can be
confusing for users to manipulate (and delete).

Change-Id: Ia1f5a65b9cff4eeb51cc4dba3ff7c7afabc343f2
Reviewed-on: https://team-review.git.corp.google.com/c/368442
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
src/cmd/go/internal/get/get.go
src/cmd/go/internal/get/path.go [new file with mode: 0644]
src/cmd/go/testdata/script/get_brace.txt [new file with mode: 0644]
src/cmd/go/testdata/script/get_dotfiles.txt [new file with mode: 0644]

index e4148bceb048472d743cdd17d1e88e8aba10ef65..f4b969fcb22c6c26b07e3b1494e0dcbaf2432f2d 100644 (file)
@@ -402,6 +402,10 @@ func downloadPackage(p *load.Package) error {
                security = web.Insecure
        }
 
+       if err := CheckImportPath(p.ImportPath); err != nil {
+               return fmt.Errorf("%s: invalid import path: %v", p.ImportPath, err)
+       }
+
        if p.Internal.Build.SrcRoot != "" {
                // Directory exists. Look for checkout along path to src.
                vcs, rootPath, err = vcsFromDir(p.Dir, p.Internal.Build.SrcRoot)
diff --git a/src/cmd/go/internal/get/path.go b/src/cmd/go/internal/get/path.go
new file mode 100644 (file)
index 0000000..2920fc2
--- /dev/null
@@ -0,0 +1,172 @@
+// Copyright 2018 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 get
+
+import (
+       "fmt"
+       "strings"
+       "unicode"
+       "unicode/utf8"
+)
+
+// The following functions are copied verbatim from cmd/go/internal/module/module.go.
+//
+// TODO(bcmills): After the call site for this function is backported,
+// consolidate this back down to a single copy.
+
+// CheckImportPath checks that an import path is valid.
+func CheckImportPath(path string) error {
+       if err := checkPath(path, false); err != nil {
+               return fmt.Errorf("malformed import path %q: %v", path, err)
+       }
+       return nil
+}
+
+// checkPath checks that a general path is valid.
+// It returns an error describing why but not mentioning path.
+// Because these checks apply to both module paths and import paths,
+// the caller is expected to add the "malformed ___ path %q: " prefix.
+// fileName indicates whether the final element of the path is a file name
+// (as opposed to a directory name).
+func checkPath(path string, fileName bool) error {
+       if !utf8.ValidString(path) {
+               return fmt.Errorf("invalid UTF-8")
+       }
+       if path == "" {
+               return fmt.Errorf("empty string")
+       }
+       if strings.Contains(path, "..") {
+               return fmt.Errorf("double dot")
+       }
+       if strings.Contains(path, "//") {
+               return fmt.Errorf("double slash")
+       }
+       if path[len(path)-1] == '/' {
+               return fmt.Errorf("trailing slash")
+       }
+       elemStart := 0
+       for i, r := range path {
+               if r == '/' {
+                       if err := checkElem(path[elemStart:i], fileName); err != nil {
+                               return err
+                       }
+                       elemStart = i + 1
+               }
+       }
+       if err := checkElem(path[elemStart:], fileName); err != nil {
+               return err
+       }
+       return nil
+}
+
+// checkElem checks whether an individual path element is valid.
+// fileName indicates whether the element is a file name (not a directory name).
+func checkElem(elem string, fileName bool) error {
+       if elem == "" {
+               return fmt.Errorf("empty path element")
+       }
+       if strings.Count(elem, ".") == len(elem) {
+               return fmt.Errorf("invalid path element %q", elem)
+       }
+       if elem[0] == '.' && !fileName {
+               return fmt.Errorf("leading dot in path element")
+       }
+       if elem[len(elem)-1] == '.' {
+               return fmt.Errorf("trailing dot in path element")
+       }
+       charOK := pathOK
+       if fileName {
+               charOK = fileNameOK
+       }
+       for _, r := range elem {
+               if !charOK(r) {
+                       return fmt.Errorf("invalid char %q", r)
+               }
+       }
+
+       // Windows disallows a bunch of path elements, sadly.
+       // See https://docs.microsoft.com/en-us/windows/desktop/fileio/naming-a-file
+       short := elem
+       if i := strings.Index(short, "."); i >= 0 {
+               short = short[:i]
+       }
+       for _, bad := range badWindowsNames {
+               if strings.EqualFold(bad, short) {
+                       return fmt.Errorf("disallowed path element %q", elem)
+               }
+       }
+       return nil
+}
+
+// pathOK reports whether r can appear in an import path element.
+// Paths can be ASCII letters, ASCII digits, and limited ASCII punctuation: + - . _ and ~.
+// This matches what "go get" has historically recognized in import paths.
+// TODO(rsc): We would like to allow Unicode letters, but that requires additional
+// care in the safe encoding (see note below).
+func pathOK(r rune) bool {
+       if r < utf8.RuneSelf {
+               return r == '+' || r == '-' || r == '.' || r == '_' || r == '~' ||
+                       '0' <= r && r <= '9' ||
+                       'A' <= r && r <= 'Z' ||
+                       'a' <= r && r <= 'z'
+       }
+       return false
+}
+
+// fileNameOK reports whether r can appear in a file name.
+// For now we allow all Unicode letters but otherwise limit to pathOK plus a few more punctuation characters.
+// If we expand the set of allowed characters here, we have to
+// work harder at detecting potential case-folding and normalization collisions.
+// See note about "safe encoding" below.
+func fileNameOK(r rune) bool {
+       if r < utf8.RuneSelf {
+               // Entire set of ASCII punctuation, from which we remove characters:
+               //     ! " # $ % & ' ( ) * + , - . / : ; < = > ? @ [ \ ] ^ _ ` { | } ~
+               // We disallow some shell special characters: " ' * < > ? ` |
+               // (Note that some of those are disallowed by the Windows file system as well.)
+               // We also disallow path separators / : and \ (fileNameOK is only called on path element characters).
+               // We allow spaces (U+0020) in file names.
+               const allowed = "!#$%&()+,-.=@[]^_{}~ "
+               if '0' <= r && r <= '9' || 'A' <= r && r <= 'Z' || 'a' <= r && r <= 'z' {
+                       return true
+               }
+               for i := 0; i < len(allowed); i++ {
+                       if rune(allowed[i]) == r {
+                               return true
+                       }
+               }
+               return false
+       }
+       // It may be OK to add more ASCII punctuation here, but only carefully.
+       // For example Windows disallows < > \, and macOS disallows :, so we must not allow those.
+       return unicode.IsLetter(r)
+}
+
+// badWindowsNames are the reserved file path elements on Windows.
+// See https://docs.microsoft.com/en-us/windows/desktop/fileio/naming-a-file
+var badWindowsNames = []string{
+       "CON",
+       "PRN",
+       "AUX",
+       "NUL",
+       "COM1",
+       "COM2",
+       "COM3",
+       "COM4",
+       "COM5",
+       "COM6",
+       "COM7",
+       "COM8",
+       "COM9",
+       "LPT1",
+       "LPT2",
+       "LPT3",
+       "LPT4",
+       "LPT5",
+       "LPT6",
+       "LPT7",
+       "LPT8",
+       "LPT9",
+}
diff --git a/src/cmd/go/testdata/script/get_brace.txt b/src/cmd/go/testdata/script/get_brace.txt
new file mode 100644 (file)
index 0000000..36414d7
--- /dev/null
@@ -0,0 +1,45 @@
+[!exec:git] skip
+
+# Set up some empty repositories.
+cd $WORK/_origin/foo
+exec git init
+exec git commit --allow-empty -m 'create master branch'
+
+cd $WORK
+cd '_origin/{confusing}'
+exec git init
+exec git commit --allow-empty -m 'create master branch'
+
+# Clone the empty repositories into GOPATH.
+# This tells the Go command where to find them: it takes the place of a user's meta-tag redirector.
+mkdir $GOPATH/src/example.com
+cd $GOPATH/src/example.com
+exec git clone $WORK/_origin/foo
+exec git clone $WORK/_origin/{confusing}
+
+# Commit contents to the repositories.
+cd $WORK/_origin/foo
+exec git add main.go
+exec git commit -m 'add main'
+
+cd $WORK
+cd '_origin/{confusing}'
+exec git add confusing.go
+exec git commit -m 'just try to delete this!'
+
+# 'go get' should refuse to download or update the confusingly-named repo.
+cd $GOPATH/src/example.com/foo
+! go get -u 'example.com/{confusing}'
+stderr 'invalid char'
+! go get -u example.com/foo
+stderr 'invalid import path'
+! exists example.com/{confusing}
+
+-- $WORK/_origin/foo/main.go --
+package main
+import _ "example.com/{confusing}"
+
+func main() {}
+
+-- $WORK/_origin/{confusing}/confusing.go --
+package confusing
diff --git a/src/cmd/go/testdata/script/get_dotfiles.txt b/src/cmd/go/testdata/script/get_dotfiles.txt
new file mode 100644 (file)
index 0000000..c09da8b
--- /dev/null
@@ -0,0 +1,57 @@
+[!exec:git] skip
+
+# Set up a benign repository and a repository with a dotfile name.
+cd $WORK/_origin/foo
+exec git init
+exec git commit --allow-empty -m 'create master branch'
+
+cd $WORK/_origin/.hidden
+exec git init
+exec git commit --allow-empty -m 'create master branch'
+
+# Clone the empty repositories into GOPATH.
+# This tells the Go command where to find them: it takes the place of a user's meta-tag redirector.
+mkdir $GOPATH/src/example.com
+cd $GOPATH/src/example.com
+exec git clone $WORK/_origin/foo
+exec git clone $WORK/_origin/.hidden
+
+# Add a benign commit.
+cd $WORK/_origin/foo
+cp _ok/main.go main.go
+exec git add main.go
+exec git commit -m 'add ok'
+
+# 'go get' should install the benign commit.
+cd $GOPATH
+go get -u example.com/foo
+
+# Now sneak in an import of a dotfile path.
+cd $WORK/_origin/.hidden
+exec git add hidden.go
+exec git commit -m 'nothing to see here, move along'
+
+cd $WORK/_origin/foo
+cp _sneaky/main.go main.go
+exec git add main.go
+exec git commit -m 'fix typo (heh heh heh)'
+
+# 'go get -u' should refuse to download or update the dotfile-named repo.
+cd $GOPATH/src/example.com/foo
+! go get -u example.com/foo
+stderr 'leading dot'
+! exists example.com/.hidden/hidden.go
+
+-- $WORK/_origin/foo/_ok/main.go --
+package main
+
+func main() {}
+
+-- $WORK/_origin/foo/_sneaky/main.go --
+package main
+import _ "example.com/.hidden"
+
+func main() {}
+
+-- $WORK/_origin/.hidden/hidden.go --
+package hidden