Merge branch 'security-pks-create-from-url-creds-13.0' into '13-0-stable'

Injection of `http.<url>.*` git config settings leading to SSRF

See merge request gitlab-org/security/gitaly!6

3 files changed, 14 insertions, 6 deletions

diff --git a/changelogs/unreleased/security-pks-create-from-url-creds.yml b/changelogs/unreleased/security-pks-create-from-url-creds.yml
new file mode 100644
index 000000000..392d0a731
--- /dev/null
+++ b/changelogs/unreleased/security-pks-create-from-url-creds.yml
@@ -0,0 +1,5 @@
+---
+title: Fix injection of arbitrary `http.*` options
+merge_request:
+author:
+type: security
diff --git a/internal/service/repository/create_from_url.go b/internal/service/repository/create_from_url.go
index 57cb89946..db76a4e25 100644
--- a/internal/service/repository/create_from_url.go
+++ b/internal/service/repository/create_from_url.go
@@ -23,10 +23,13 @@ func cloneFromURLCommand(ctx context.Context, repoURL, repositoryFullPath string
 		return nil, helper.ErrInternal(err)
 	}
 
-	flags := []git.Option{
+	globalFlags := []git.Option{
+		git.ValueFlag{Name: "-c", Value: "http.followRedirects=false"},
+	}
+
+	cloneFlags := []git.Option{
 		git.Flag{Name: "--bare"},
 		git.Flag{Name: "--quiet"},
-		git.ValueFlag{Name: "-c", Value: "http.followRedirects=false"},
 	}
 
 	if u.User != nil {
@@ -41,12 +44,12 @@ func cloneFromURLCommand(ctx context.Context, repoURL, repositoryFullPath string
 
 		u.User = nil
 		authHeader := fmt.Sprintf("Authorization: Basic %s", base64.StdEncoding.EncodeToString([]byte(creds)))
-		flags = append(flags, git.ValueFlag{Name: "-c", Value: fmt.Sprintf("http.%s.extraHeader=%s", u.String(), authHeader)})
+		globalFlags = append(globalFlags, git.ValueFlag{Name: "-c", Value: fmt.Sprintf("http.extraHeader=%s", authHeader)})
 	}
 
-	return git.SafeBareCmd(ctx, git.CmdStream{Err: stderr}, nil, nil, git.SubCmd{
+	return git.SafeBareCmd(ctx, git.CmdStream{Err: stderr}, nil, globalFlags, git.SubCmd{
 		Name:        "clone",
-		Flags:       flags,
+		Flags:       cloneFlags,
 		PostSepArgs: []string{u.String(), repositoryFullPath},
 	})
 }
diff --git a/internal/service/repository/create_from_url_test.go b/internal/service/repository/create_from_url_test.go
index 526c2cf64..b08737e38 100644
--- a/internal/service/repository/create_from_url_test.go
+++ b/internal/service/repository/create_from_url_test.go
@@ -77,7 +77,7 @@ func TestCloneRepositoryFromUrlCommand(t *testing.T) {
 
 	expectedScrubbedURL := "https://www.example.com/secretrepo.git"
 	expectedBasicAuthHeader := fmt.Sprintf("Authorization: Basic %s", base64.StdEncoding.EncodeToString([]byte("user:pass!?@")))
-	expectedHeader := fmt.Sprintf("http.%s.extraHeader=%s", expectedScrubbedURL, expectedBasicAuthHeader)
+	expectedHeader := fmt.Sprintf("http.extraHeader=%s", expectedBasicAuthHeader)
 
 	var args = cmd.Args()
 	require.Contains(t, args, expectedScrubbedURL)