From 973d93daeaa0e31f0cec2e09db8838cf38c67dc5 Mon Sep 17 00:00:00 2001
From: ngala <ngala@gitlab.com>
Date: Tue, 29 Nov 2022 11:15:11 +0530
Subject: Restrict arbitrary protocol redirection to only https or http URLs

This commit restricts arbitrary protocol redirection to only https or http URLs and introduces nolint: gocyclo for auth.handleProxyingAuth method.

Changelog: security
---
 internal/auth/auth.go      | 19 +++++++++++++++----
 internal/auth/auth_test.go | 15 +++++++++++++++
 2 files changed, 30 insertions(+), 4 deletions(-)

(limited to 'internal')

diff --git a/internal/auth/auth.go b/internal/auth/auth.go
index b4dc73aa..4f085e38 100644
--- a/internal/auth/auth.go
+++ b/internal/auth/auth.go
@@ -38,10 +38,11 @@ const (
 	callbackPath           = "/auth"
 	authorizeProxyTemplate = "%s?domain=%s&state=%s"
 
-	failAuthErrMsg         = "failed to authenticate request"
-	fetchAccessTokenErrMsg = "fetching access token failed"
-	queryParameterErrMsg   = "failed to parse domain query parameter"
-	saveSessionErrMsg      = "failed to save the session"
+	failAuthErrMsg             = "failed to authenticate request"
+	fetchAccessTokenErrMsg     = "fetching access token failed"
+	queryParameterErrMsg       = "failed to parse domain query parameter"
+	saveSessionErrMsg          = "failed to save the session"
+	domainQueryParameterErrMsg = "domain query parameter only supports http/https protocol"
 )
 
 var (
@@ -197,6 +198,7 @@ func (a *Auth) domainAllowed(ctx context.Context, name string, domains source.So
 	return (domain != nil && err == nil)
 }
 
+// nolint: gocyclo // TODO refactor this function https://gitlab.com/gitlab-org/gitlab-pages/-/issues/813
 func (a *Auth) handleProxyingAuth(session *hostSession, w http.ResponseWriter, r *http.Request, domains source.Source) bool {
 	// handle auth callback e.g. https://gitlab.io/auth?domain=domain&state=state
 	if shouldProxyAuthToGitlab(r) {
@@ -211,6 +213,15 @@ func (a *Auth) handleProxyingAuth(session *hostSession, w http.ResponseWriter, r
 			httperrors.Serve500(w)
 			return true
 		}
+
+		// domain query param can only contain https or http URLs.
+		if proxyurl.Scheme != "http" && proxyurl.Scheme != "https" {
+			logRequest(r).WithField("domain_query", domain).Warn(domainQueryParameterErrMsg)
+
+			httperrors.Serve401(w)
+			return true
+		}
+
 		host, _, err := net.SplitHostPort(proxyurl.Host)
 		if err != nil {
 			host = proxyurl.Host
diff --git a/internal/auth/auth_test.go b/internal/auth/auth_test.go
index 47a43d4c..c2b25635 100644
--- a/internal/auth/auth_test.go
+++ b/internal/auth/auth_test.go
@@ -173,6 +173,21 @@ func TestTryAuthenticateWithDomainAndState(t *testing.T) {
 	require.Equal(t, "/public-gitlab.example.com/oauth/authorize?client_id=id&redirect_uri=http://pages.gitlab-example.com/auth&response_type=code&state=state&scope=scope", redirect.String())
 }
 
+func TestTryAuthenticateWithNonHttpDomainAndState(t *testing.T) {
+	auth := createTestAuth(t, "", "")
+
+	result := httptest.NewRecorder()
+
+	r, err := http.NewRequest("Get", "https://example.com/auth?domain=mailto://example.com?body=TESTBODY&state=state", nil)
+	require.NoError(t, err)
+
+	mockCtrl := gomock.NewController(t)
+
+	mockSource := mock.NewMockSource(mockCtrl)
+	require.True(t, auth.TryAuthenticate(result, r, mockSource))
+	require.Equal(t, http.StatusUnauthorized, result.Code)
+}
+
 func testTryAuthenticateWithCodeAndState(t *testing.T, https bool) {
 	t.Helper()
 
-- 
cgit v1.2.3