From e989ff0c2a8bb80d23f1baf5d59a3295aa22ee61 Mon Sep 17 00:00:00 2001
From: Anurag Shivarathri <anurag6713@gmail.com>
Date: Thu, 17 Jun 2021 17:50:43 +0530
Subject: [PATCH] Prevent JS Execution on Pre-login OAUTH screen  (#17789)

* Fixed invalid js execution

* Added data & vbscript

* Added javascript protocol in mixed case

* Updated the test
---
 utils/api.go      | 12 +++++++++++-
 web/oauth_test.go | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+), 1 deletion(-)

diff --git a/utils/api.go b/utils/api.go
index c5d1d9b36a..30718e8e90 100644
--- a/utils/api.go
+++ b/utils/api.go
@@ -101,6 +101,16 @@ func RenderMobileAuthComplete(w http.ResponseWriter, redirectURL string) {
 }
 
 func RenderMobileError(config *model.Config, w http.ResponseWriter, err *model.AppError, redirectURL string) {
+	var link = redirectURL
+	var invalidSchemes = map[string]bool{
+		"data":       true,
+		"javascript": true,
+		"vbscript":   true,
+	}
+	u, redirectErr := url.Parse(redirectURL)
+	if redirectErr != nil || invalidSchemes[u.Scheme] {
+		link = *config.ServiceSettings.SiteURL
+	}
 	RenderMobileMessage(w, `
 		<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 576 512" style="width: 64px; height: 64px; fill: #ccc">
 			<!-- Font Awesome Free 5.15.3 by @fontawesome - https://fontawesome.com License - https://fontawesome.com/license/free (Icons: CC BY 4.0, Fonts: SIL OFL 1.1, Code: MIT License) -->
@@ -108,7 +118,7 @@ func RenderMobileError(config *model.Config, w http.ResponseWriter, err *model.A
 		</svg>
 		<h2> `+i18n.T("error")+` </h2>
 		<p> `+err.Message+` </p>
-		<a href="`+redirectURL+`">
+		<a href="`+link+`">
 			`+i18n.T("api.back_to_app", map[string]interface{}{"SiteName": config.TeamSettings.SiteName})+`
 		</a>
 	`)
diff --git a/web/oauth_test.go b/web/oauth_test.go
index 325f78d98d..5d2893e802 100644
--- a/web/oauth_test.go
+++ b/web/oauth_test.go
@@ -367,6 +367,52 @@ func TestOAuthAccessToken(t *testing.T) {
 	ApiClient.ClearOAuthToken()
 }
 
+func TestMobileLoginWithOAuth(t *testing.T) {
+	th := Setup(t).InitBasic()
+	defer th.TearDown()
+	c := &Context{
+		App:        th.App,
+		AppContext: &request.Context{},
+		Params: &Params{
+			Service: "gitlab",
+		},
+	}
+
+	var siteURL = "http://localhost:8065"
+	th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.SiteURL = siteURL })
+
+	translationFunc := i18n.GetUserTranslations("en")
+	c.AppContext.SetT(translationFunc)
+	buffer := &bytes.Buffer{}
+	c.Logger = mlog.NewTestingLogger(t, buffer)
+	provider := &MattermostTestProvider{}
+	einterfaces.RegisterOauthProvider(model.SERVICE_GITLAB, provider)
+
+	t.Run("Should include redirect URL in the output when valid URL Scheme is passed", func(t *testing.T) {
+		responseWriter := httptest.NewRecorder()
+		request, _ := http.NewRequest(http.MethodGet, th.App.GetSiteURL()+"/oauth/gitlab/mobile_login?redirect_to="+url.QueryEscape("randomScheme://"), nil)
+		mobileLoginWithOAuth(c, responseWriter, request)
+		assert.Contains(t, responseWriter.Body.String(), "randomScheme://")
+		assert.NotContains(t, responseWriter.Body.String(), siteURL)
+	})
+
+	t.Run("Should not include the redirect URL consisting of javascript protocol", func(t *testing.T) {
+		responseWriter := httptest.NewRecorder()
+		request, _ := http.NewRequest(http.MethodGet, th.App.GetSiteURL()+"/oauth/gitlab/mobile_login?redirect_to="+url.QueryEscape("javascript:alert('hello')"), nil)
+		mobileLoginWithOAuth(c, responseWriter, request)
+		assert.NotContains(t, responseWriter.Body.String(), "javascript:alert('hello')")
+		assert.Contains(t, responseWriter.Body.String(), siteURL)
+	})
+
+	t.Run("Should not include the redirect URL consisting of javascript protocol in mixed case", func(t *testing.T) {
+		responseWriter := httptest.NewRecorder()
+		request, _ := http.NewRequest(http.MethodGet, th.App.GetSiteURL()+"/oauth/gitlab/mobile_login?redirect_to="+url.QueryEscape("JaVasCript:alert('hello')"), nil)
+		mobileLoginWithOAuth(c, responseWriter, request)
+		assert.NotContains(t, responseWriter.Body.String(), "JaVasCript:alert('hello')")
+		assert.Contains(t, responseWriter.Body.String(), siteURL)
+	})
+}
+
 func TestOAuthComplete(t *testing.T) {
 	if testing.Short() {
 		t.SkipNow()