Browse Source

fix(db): sanitize user full name after find (#7353)

Joe Chen 2 years ago
parent
commit
8f9895acaf
5 changed files with 14 additions and 0 deletions
  1. 1 0
      CHANGELOG.md
  2. 4 0
      internal/db/issue.go
  3. 5 0
      internal/db/repo.go
  4. 2 0
      internal/db/users.go
  5. 2 0
      internal/db/users_test.go

+ 1 - 0
CHANGELOG.md

@@ -28,6 +28,7 @@ All notable changes to Gogs are documented in this file.
 
 ### Fixed
 
+- _Security:_ Stored XSS for issue assignees. [#7145](https://github.com/gogs/gogs/issues/7145)
 - Unable to use LDAP authentication on ARM machines. [#6761](https://github.com/gogs/gogs/issues/6761)
 - Unable to choose "Lookup Avatar by mail" in user settings without deleting custom avatar. [#7267](https://github.com/gogs/gogs/pull/7267)
 - Mistakenly include the "data" directory under the custom directory in the Docker setup. [#7343](https://github.com/gogs/gogs/pull/7343)

+ 4 - 0
internal/db/issue.go

@@ -19,6 +19,7 @@ import (
 	"gogs.io/gogs/internal/conf"
 	"gogs.io/gogs/internal/db/errors"
 	"gogs.io/gogs/internal/errutil"
+	"gogs.io/gogs/internal/markup"
 	"gogs.io/gogs/internal/tool"
 )
 
@@ -88,6 +89,9 @@ func getUserByID(e Engine, id int64) (*User, error) {
 	} else if !has {
 		return nil, ErrUserNotExist{args: errutil.Args{"userID": id}}
 	}
+
+	// TODO(unknwon): Rely on AfterFind hook to sanitize user full name.
+	u.FullName = markup.Sanitize(u.FullName)
 	return u, nil
 }
 

+ 5 - 0
internal/db/repo.go

@@ -503,6 +503,11 @@ func (repo *Repository) getUsersWithAccesMode(e Engine, mode AccessMode) (_ []*U
 		if err = e.In("id", userIDs).Find(&users); err != nil {
 			return nil, err
 		}
+
+		// TODO(unknwon): Rely on AfterFind hook to sanitize user full name.
+		for _, u := range users {
+			u.FullName = markup.Sanitize(u.FullName)
+		}
 	}
 	if !repo.Owner.IsOrganization() {
 		users = append(users, repo.Owner)

+ 2 - 0
internal/db/users.go

@@ -24,6 +24,7 @@ import (
 	"gogs.io/gogs/internal/cryptoutil"
 	"gogs.io/gogs/internal/dbutil"
 	"gogs.io/gogs/internal/errutil"
+	"gogs.io/gogs/internal/markup"
 	"gogs.io/gogs/internal/osutil"
 	"gogs.io/gogs/internal/repoutil"
 	"gogs.io/gogs/internal/strutil"
@@ -1132,6 +1133,7 @@ func (u *User) BeforeCreate(tx *gorm.DB) error {
 
 // AfterFind implements the GORM query hook.
 func (u *User) AfterFind(_ *gorm.DB) error {
+	u.FullName = markup.Sanitize(u.FullName)
 	u.Created = time.Unix(u.CreatedUnix, 0).Local()
 	u.Updated = time.Unix(u.UpdatedUnix, 0).Local()
 	return nil

+ 2 - 0
internal/db/users_test.go

@@ -68,10 +68,12 @@ func TestUser_AfterFind(t *testing.T) {
 	}
 
 	user := &User{
+		FullName:    "user1<script src=http://localhost:8181/xss.js>",
 		CreatedUnix: now.Unix(),
 		UpdatedUnix: now.Unix(),
 	}
 	_ = user.AfterFind(db)
+	assert.Equal(t, "user1", user.FullName)
 	assert.Equal(t, user.CreatedUnix, user.Created.Unix())
 	assert.Equal(t, user.UpdatedUnix, user.Updated.Unix())
 }