Browse Source

Fix username rendering bug (#2122)

* Fix username rendering bug

* XSS integration test

* Migration to unescape user full names
Ethan Koenig 1 year ago
parent
commit
858324c21a
4 changed files with 71 additions and 4 deletions
  1. 37 0
      integrations/xss_test.go
  2. 2 0
      models/migrations/migrations.go
  3. 32 0
      models/migrations/v37.go
  4. 0 4
      models/user.go

+ 37 - 0
integrations/xss_test.go

@@ -0,0 +1,37 @@
1
+// Copyright 2017 The Gitea Authors. All rights reserved.
2
+// Use of this source code is governed by a MIT-style
3
+// license that can be found in the LICENSE file.
4
+
5
+package integrations
6
+
7
+import (
8
+	"net/http"
9
+	"testing"
10
+
11
+	"code.gitea.io/gitea/models"
12
+
13
+	"github.com/stretchr/testify/assert"
14
+)
15
+
16
+func TestXSSUserFullName(t *testing.T) {
17
+	prepareTestEnv(t)
18
+	user := models.AssertExistsAndLoadBean(t, &models.User{ID: 2}).(*models.User)
19
+	const fullName = `name & <script class="evil">alert('Oh no!');</script>`
20
+
21
+	session := loginUser(t, user.Name)
22
+	req := NewRequestWithValues(t, "POST", "/user/settings", map[string]string{
23
+		"_csrf":     GetCSRF(t, session, "/user/settings"),
24
+		"name":      user.Name,
25
+		"full_name": fullName,
26
+		"email":     user.Email,
27
+	})
28
+	session.MakeRequest(t, req, http.StatusFound)
29
+
30
+	req = NewRequestf(t, "GET", "/%s", user.Name)
31
+	resp := session.MakeRequest(t, req, http.StatusOK)
32
+	htmlDoc := NewHTMLParser(t, resp.Body)
33
+	assert.EqualValues(t, 0, htmlDoc.doc.Find("script.evil").Length())
34
+	assert.EqualValues(t, fullName,
35
+		htmlDoc.doc.Find("div.content").Find(".header.text.center").Text(),
36
+	)
37
+}

+ 2 - 0
models/migrations/migrations.go

@@ -122,6 +122,8 @@ var migrations = []Migration{
122 122
 	NewMigration("adds comment to an action", addCommentIDToAction),
123 123
 	// v36 -> v37
124 124
 	NewMigration("regenerate git hooks", regenerateGitHooks36),
125
+	// v37 -> v38
126
+	NewMigration("unescape user full names", unescapeUserFullNames),
125 127
 }
126 128
 
127 129
 // Migrate database to current version

+ 32 - 0
models/migrations/v37.go

@@ -0,0 +1,32 @@
1
+// Copyright 2017 The Gitea Authors. All rights reserved.
2
+// Use of this source code is governed by a MIT-style
3
+// license that can be found in the LICENSE file.
4
+
5
+package migrations
6
+
7
+import (
8
+	"html"
9
+
10
+	"code.gitea.io/gitea/models"
11
+
12
+	"github.com/go-xorm/xorm"
13
+)
14
+
15
+func unescapeUserFullNames(x *xorm.Engine) (err error) {
16
+	const batchSize = 100
17
+	for start := 0; ; start += batchSize {
18
+		users := make([]*models.User, 0, batchSize)
19
+		if err := x.Limit(start, batchSize).Find(users); err != nil {
20
+			return err
21
+		}
22
+		if len(users) == 0 {
23
+			return nil
24
+		}
25
+		for _, user := range users {
26
+			user.FullName = html.UnescapeString(user.FullName)
27
+			if _, err := x.Cols("full_name").Update(user); err != nil {
28
+				return err
29
+			}
30
+		}
31
+	}
32
+}

+ 0 - 4
models/user.go

@@ -35,7 +35,6 @@ import (
35 35
 	"code.gitea.io/gitea/modules/avatar"
36 36
 	"code.gitea.io/gitea/modules/base"
37 37
 	"code.gitea.io/gitea/modules/log"
38
-	"code.gitea.io/gitea/modules/markdown"
39 38
 	"code.gitea.io/gitea/modules/setting"
40 39
 )
41 40
 
@@ -164,8 +163,6 @@ func (u *User) UpdateDiffViewStyle(style string) error {
164 163
 // AfterSet is invoked from XORM after setting the value of a field of this object.
165 164
 func (u *User) AfterSet(colName string, _ xorm.Cell) {
166 165
 	switch colName {
167
-	case "full_name":
168
-		u.FullName = markdown.Sanitize(u.FullName)
169 166
 	case "created_unix":
170 167
 		u.Created = time.Unix(u.CreatedUnix, 0).Local()
171 168
 	case "updated_unix":
@@ -871,7 +868,6 @@ func updateUser(e Engine, u *User) error {
871 868
 	u.Website = base.TruncateString(u.Website, 255)
872 869
 	u.Description = base.TruncateString(u.Description, 255)
873 870
 
874
-	u.FullName = markdown.Sanitize(u.FullName)
875 871
 	_, err := e.Id(u.ID).AllCols().Update(u)
876 872
 	return err
877 873
 }