Skip to content

Fix username rendering bug #2122

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 12, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions integrations/xss_test.go
Original file line number Diff line number Diff line change
@@ -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 & <script class="evil">alert('Oh no!');</script>`

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(),
)
}
2 changes: 2 additions & 0 deletions models/migrations/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
32 changes: 32 additions & 0 deletions models/migrations/v37.go
Original file line number Diff line number Diff line change
@@ -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
}
}
}
}
4 changes: 0 additions & 4 deletions models/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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":
Expand Down Expand Up @@ -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
}
Expand Down