From 858324c21ab95bb46d881cac6f824d8f9b7d2b87 Mon Sep 17 00:00:00 2001 From: Ethan Koenig Date: Wed, 12 Jul 2017 10:58:52 -0400 Subject: [PATCH] Fix username rendering bug (#2122) * Fix username rendering bug * XSS integration test * Migration to unescape user full names --- integrations/xss_test.go | 37 +++++++++++++++++++++++++++++++++ models/migrations/migrations.go | 2 ++ models/migrations/v37.go | 32 ++++++++++++++++++++++++++++ models/user.go | 4 ---- 4 files changed, 71 insertions(+), 4 deletions(-) create mode 100644 integrations/xss_test.go create mode 100644 models/migrations/v37.go diff --git a/integrations/xss_test.go b/integrations/xss_test.go new file mode 100644 index 0000000000..d71c680d6f --- /dev/null +++ b/integrations/xss_test.go @@ -0,0 +1,37 @@ +// Copyright 2017 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package integrations + +import ( + "net/http" + "testing" + + "code.gitea.io/gitea/models" + + "github.com/stretchr/testify/assert" +) + +func TestXSSUserFullName(t *testing.T) { + prepareTestEnv(t) + user := models.AssertExistsAndLoadBean(t, &models.User{ID: 2}).(*models.User) + const fullName = `name & ` + + session := loginUser(t, user.Name) + req := NewRequestWithValues(t, "POST", "/user/settings", map[string]string{ + "_csrf": GetCSRF(t, session, "/user/settings"), + "name": user.Name, + "full_name": fullName, + "email": user.Email, + }) + session.MakeRequest(t, req, http.StatusFound) + + req = NewRequestf(t, "GET", "/%s", user.Name) + resp := session.MakeRequest(t, req, http.StatusOK) + htmlDoc := NewHTMLParser(t, resp.Body) + assert.EqualValues(t, 0, htmlDoc.doc.Find("script.evil").Length()) + assert.EqualValues(t, fullName, + htmlDoc.doc.Find("div.content").Find(".header.text.center").Text(), + ) +} diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 437b63f604..e5b6473687 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -122,6 +122,8 @@ var migrations = []Migration{ NewMigration("adds comment to an action", addCommentIDToAction), // v36 -> v37 NewMigration("regenerate git hooks", regenerateGitHooks36), + // v37 -> v38 + NewMigration("unescape user full names", unescapeUserFullNames), } // Migrate database to current version diff --git a/models/migrations/v37.go b/models/migrations/v37.go new file mode 100644 index 0000000000..aac00e84cb --- /dev/null +++ b/models/migrations/v37.go @@ -0,0 +1,32 @@ +// Copyright 2017 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package migrations + +import ( + "html" + + "code.gitea.io/gitea/models" + + "github.com/go-xorm/xorm" +) + +func unescapeUserFullNames(x *xorm.Engine) (err error) { + const batchSize = 100 + for start := 0; ; start += batchSize { + users := make([]*models.User, 0, batchSize) + if err := x.Limit(start, batchSize).Find(users); err != nil { + return err + } + if len(users) == 0 { + return nil + } + for _, user := range users { + user.FullName = html.UnescapeString(user.FullName) + if _, err := x.Cols("full_name").Update(user); err != nil { + return err + } + } + } +} diff --git a/models/user.go b/models/user.go index 2a4fb557db..7c523f8266 100644 --- a/models/user.go +++ b/models/user.go @@ -35,7 +35,6 @@ import ( "code.gitea.io/gitea/modules/avatar" "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/log" - "code.gitea.io/gitea/modules/markdown" "code.gitea.io/gitea/modules/setting" ) @@ -164,8 +163,6 @@ func (u *User) UpdateDiffViewStyle(style string) error { // AfterSet is invoked from XORM after setting the value of a field of this object. func (u *User) AfterSet(colName string, _ xorm.Cell) { switch colName { - case "full_name": - u.FullName = markdown.Sanitize(u.FullName) case "created_unix": u.Created = time.Unix(u.CreatedUnix, 0).Local() case "updated_unix": @@ -871,7 +868,6 @@ func updateUser(e Engine, u *User) error { u.Website = base.TruncateString(u.Website, 255) u.Description = base.TruncateString(u.Description, 255) - u.FullName = markdown.Sanitize(u.FullName) _, err := e.Id(u.ID).AllCols().Update(u) return err }