summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBryan C. Mills <bcmills@google.com>2018-11-30 14:04:35 -0500
committerBryan C. Mills <bcmills@google.com>2018-12-07 14:48:35 +0000
commit8954addb3294a5e664a9833354bafa58f163fe8f (patch)
tree61458d603da0057d590dfecacf514d338e7fd9cf
parentdf523969435b8945d939c7e2a849b50910ef4c25 (diff)
downloadgo-git-8954addb3294a5e664a9833354bafa58f163fe8f.tar.gz
[release-branch.go1.11-security] cmd/go: reject 'get' of paths containing leading dots or unsupported characters
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>
-rw-r--r--src/cmd/go/internal/get/get.go4
-rw-r--r--src/cmd/go/internal/get/path.go172
-rw-r--r--src/cmd/go/testdata/script/get_brace.txt45
-rw-r--r--src/cmd/go/testdata/script/get_dotfiles.txt57
4 files changed, 278 insertions, 0 deletions
diff --git a/src/cmd/go/internal/get/get.go b/src/cmd/go/internal/get/get.go
index e4148bceb0..f4b969fcb2 100644
--- a/src/cmd/go/internal/get/get.go
+++ b/src/cmd/go/internal/get/get.go
@@ -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
index 0000000000..2920fc2085
--- /dev/null
+++ b/src/cmd/go/internal/get/path.go
@@ -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
index 0000000000..36414d7b55
--- /dev/null
+++ b/src/cmd/go/testdata/script/get_brace.txt
@@ -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
index 0000000000..c09da8beeb
--- /dev/null
+++ b/src/cmd/go/testdata/script/get_dotfiles.txt
@@ -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