[release-branch.go1.10] cmd/go: restrict meta imports to valid schemes

Before this change, when using -insecure, we permitted any meta import
repo root as long as it contained "://". When not using -insecure, we
restrict meta import repo roots to be valid URLs. People may depend on
that somehow, so permit meta import repo roots to be invalid URLs, but
require them to have valid schemes per RFC 3986.

Fixes #23867

Change-Id: Iac666dfc75ac321bf8639dda5b0dba7c8840922d
Reviewed-on: https://go-review.googlesource.com/94603
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-on: https://go-review.googlesource.com/102778
Run-TryBot: Andrew Bonventre <andybons@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/cmd/go/internal/get/vcs.go b/src/cmd/go/internal/get/vcs.go
index 26693b13a93d82d1ceaa4c586d857d373b06d2e7..0b2a04e04f81ba472dedcf3058be87831dd7bd09 100644 (file)
--- a/src/cmd/go/internal/get/vcs.go
+++ b/src/cmd/go/internal/get/vcs.go
@@ -809,8 +809,8 @@ func repoRootForImportDynamic(importPath string, security web.SecurityMode) (*re
                }
        }
 
-       if !strings.Contains(mmi.RepoRoot, "://") {
-               return nil, fmt.Errorf("%s: invalid repo root %q; no scheme", urlStr, mmi.RepoRoot)
+       if err := validateRepoRootScheme(mmi.RepoRoot); err != nil {
+               return nil, fmt.Errorf("%s: invalid repo root %q: %v", urlStr, mmi.RepoRoot, err)
        }
        rr := &repoRoot{
                vcs:      vcsByCmd(mmi.VCS),
@@ -824,6 +824,36 @@ func repoRootForImportDynamic(importPath string, security web.SecurityMode) (*re
        return rr, nil
 }
 
+// validateRepoRootScheme returns an error if repoRoot does not seem
+// to have a valid URL scheme. At this point we permit things that
+// aren't valid URLs, although later, if not using -insecure, we will
+// restrict repoRoots to be valid URLs. This is only because we've
+// historically permitted them, and people may depend on that.
+func validateRepoRootScheme(repoRoot string) error {
+       end := strings.Index(repoRoot, "://")
+       if end <= 0 {
+               return errors.New("no scheme")
+       }
+
+       // RFC 3986 section 3.1.
+       for i := 0; i < end; i++ {
+               c := repoRoot[i]
+               switch {
+               case 'a' <= c && c <= 'z' || 'A' <= c && c <= 'Z':
+                       // OK.
+               case '0' <= c && c <= '9' || c == '+' || c == '-' || c == '.':
+                       // OK except at start.
+                       if i == 0 {
+                               return errors.New("invalid scheme")
+                       }
+               default:
+                       return errors.New("invalid scheme")
+               }
+       }
+
+       return nil
+}
+
 var fetchGroup singleflight.Group
 var (
        fetchCacheMu sync.Mutex

diff --git a/src/cmd/go/internal/get/vcs_test.go b/src/cmd/go/internal/get/vcs_test.go
index e29338aec19464e460a048ef91e03b414474eb95..a6f8642026cc12dffef168b54f710d4bd8295a2d 100644 (file)
--- a/src/cmd/go/internal/get/vcs_test.go
+++ b/src/cmd/go/internal/get/vcs_test.go
@@ -408,3 +408,46 @@ func TestMatchGoImport(t *testing.T) {
                }
        }
 }
+
+func TestValidateRepoRootScheme(t *testing.T) {
+       tests := []struct {
+               root string
+               err  string
+       }{
+               {
+                       root: "",
+                       err:  "no scheme",
+               },
+               {
+                       root: "http://",
+                       err:  "",
+               },
+               {
+                       root: "a://",
+                       err:  "",
+               },
+               {
+                       root: "a#://",
+                       err:  "invalid scheme",
+               },
+               {
+                       root: "-config://",
+                       err:  "invalid scheme",
+               },
+       }
+
+       for _, test := range tests {
+               err := validateRepoRootScheme(test.root)
+               if err == nil {
+                       if test.err != "" {
+                               t.Errorf("validateRepoRootScheme(%q) = nil, want %q", test.root, test.err)
+                       }
+               } else if test.err == "" {
+                       if err != nil {
+                               t.Errorf("validateRepoRootScheme(%q) = %q, want nil", test.root, test.err)
+                       }
+               } else if err.Error() != test.err {
+                       t.Errorf("validateRepoRootScheme(%q) = %q, want %q", test.root, err, test.err)
+               }
+       }
+}