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
Этот коммит содержится в:
коммит произвёл
GitHub
родитель
e4aa729a0c
Коммит
e989ff0c2a
12
utils/api.go
12
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>
|
||||
`)
|
||||
|
||||
@@ -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()
|
||||
|
||||
Ссылка в новой задаче
Block a user