From 80751bb029fa391280e1b8e5ac480c3db9e6e7db Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Thu, 15 Feb 2018 15:57:13 -0800 Subject: [PATCH] [PATCH] 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 Gbp-Pq: Name cve-2018-7187.patch --- src/cmd/go/vcs.go | 34 +++++++++++++++++++++++++++++++-- src/cmd/go/vcs_test.go | 43 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 2 deletions(-) diff --git a/src/cmd/go/vcs.go b/src/cmd/go/vcs.go index 53ddbe6..9cd4688 100644 --- a/src/cmd/go/vcs.go +++ b/src/cmd/go/vcs.go @@ -691,8 +691,8 @@ func repoRootForImportDynamic(importPath string, security securityMode) (*repoRo } } - 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), @@ -705,6 +705,36 @@ func repoRootForImportDynamic(importPath string, security securityMode) (*repoRo 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/vcs_test.go b/src/cmd/go/vcs_test.go index 25e3866..f7a6a83 100644 --- a/src/cmd/go/vcs_test.go +++ b/src/cmd/go/vcs_test.go @@ -321,3 +321,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) + } + } +} -- 2.30.2