Skip to content
Permalink
Browse files

[release-branch.go1.11-security] cmd/go: reject 'get' of paths contai…

…ning 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>
  • Loading branch information...
bcmills committed Nov 30, 2018
1 parent df52396 commit 8954addb3294a5e664a9833354bafa58f163fe8f
@@ -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)
@@ -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, "..") {

This comment has been minimized.

Copy link
@tomislavmitic2012

tomislavmitic2012 Dec 14, 2018

You have just broken every one that uses packages such as gobuffalo/pop/ which were gotten previously with go get github.com/gobuffalo/pop/... the only way to fix it is to downgrade to go 1.11.2. How go you suggest we proceed.

This comment has been minimized.

Copy link
@bcmills

bcmills Dec 14, 2018

Author Member

For the next day or two, use go get -d github.com/gobuffalo/pop && go get github.com/gobuffalo/pop/....

1.11.4 will be out shortly, and will fix this regression.

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",
}
@@ -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
@@ -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

0 comments on commit 8954add

Please sign in to comment.
You can’t perform that action at this time.