MM-23489 Fix poor Unicode handling in display names (#14214)

Filter blacklisted Unicode characters from: user: first name, last name, nickname, bot description, username team: name, display name, description, company name channel: name, display name
2020-04-07 16:56:07 -04:00
--- a/api4/channel_test.go
+++ b/api4/channel_test.go
@@ -317,6 +317,64 @@ func TestPatchChannel(t *testing.T) {
 	CheckForbiddenStatus(t, resp)
 }

+func TestChannelUnicodeNames(t *testing.T) {
+	th := Setup(t).InitBasic()
+	defer th.TearDown()
+	Client := th.Client
+	team := th.BasicTeam
+
+	t.Run("create channel unicode", func(t *testing.T) {
+		channel := &model.Channel{
+			Name:        "\u206cenglish\u206dchannel",
+			DisplayName: "The \u206cEnglish\u206d Channel",
+			Type:        model.CHANNEL_OPEN,
+			TeamId:      team.Id}
+
+		rchannel, resp := Client.CreateChannel(channel)
+		CheckNoError(t, resp)
+		CheckCreatedStatus(t, resp)
+
+		require.Equal(t, "englishchannel", rchannel.Name, "bad unicode should be filtered from name")
+		require.Equal(t, "The English Channel", rchannel.DisplayName, "bad unicode should be filtered from display name")
+	})
+
+	t.Run("update channel unicode", func(t *testing.T) {
+		channel := &model.Channel{
+			DisplayName: "Test API Name",
+			Name:        GenerateTestChannelName(),
+			Type:        model.CHANNEL_OPEN,
+			TeamId:      team.Id,
+		}
+		channel, _ = Client.CreateChannel(channel)
+
+		channel.Name = "\u206ahistorychannel"
+		channel.DisplayName = "UFO's and \ufff9stuff\ufffb."
+
+		newChannel, resp := Client.UpdateChannel(channel)
+		CheckNoError(t, resp)
+
+		require.Equal(t, "historychannel", newChannel.Name, "bad unicode should be filtered from name")
+		require.Equal(t, "UFO's and stuff.", newChannel.DisplayName, "bad unicode should be filtered from display name")
+	})
+
+	t.Run("patch channel unicode", func(t *testing.T) {
+		patch := &model.ChannelPatch{
+			Name:        new(string),
+			DisplayName: new(string),
+			Header:      new(string),
+			Purpose:     new(string),
+		}
+		*patch.Name = "\u206ecommunitychannel\u206f"
+		*patch.DisplayName = "Natalie Tran's \ufffcAwesome Channel"
+
+		channel, resp := Client.PatchChannel(th.BasicChannel.Id, patch)
+		CheckNoError(t, resp)
+
+		require.Equal(t, "communitychannel", channel.Name, "bad unicode should be filtered from name")
+		require.Equal(t, "Natalie Tran's Awesome Channel", channel.DisplayName, "bad unicode should be filtered from display name")
+	})
+}
+
 func TestCreateDirectChannel(t *testing.T) {
 	th := Setup(t).InitBasic()
 	defer th.TearDown()
--- a/api4/team_test.go
+++ b/api4/team_test.go
@@ -459,6 +459,73 @@ func TestPatchTeamSanitization(t *testing.T) {
 	})
 }

+func TestTeamUnicodeNames(t *testing.T) {
+	th := Setup(t).InitBasic()
+	defer th.TearDown()
+	Client := th.Client
+
+	t.Run("create team unicode", func(t *testing.T) {
+		team := &model.Team{
+			Name:        GenerateTestUsername(),
+			DisplayName: "Some\u206c Team",
+			Description: "A \ufffatest\ufffb channel.",
+			CompanyName: "\ufeffAcme Inc\ufffc",
+			Type:        model.TEAM_OPEN}
+		rteam, resp := Client.CreateTeam(team)
+		CheckNoError(t, resp)
+		CheckCreatedStatus(t, resp)
+
+		require.Equal(t, "Some Team", rteam.DisplayName, "bad unicode should be filtered from display name")
+		require.Equal(t, "A test channel.", rteam.Description, "bad unicode should be filtered from description")
+		require.Equal(t, "Acme Inc", rteam.CompanyName, "bad unicode should be filtered from company name")
+	})
+
+	t.Run("update team unicode", func(t *testing.T) {
+		team := &model.Team{
+			DisplayName: "Name",
+			Description: "Some description",
+			CompanyName: "Bad Company",
+			Name:        model.NewRandomTeamName(),
+			Email:       "success+" + model.NewId() + "@simulator.amazonses.com",
+			Type:        model.TEAM_OPEN}
+		team, _ = Client.CreateTeam(team)
+
+		team.DisplayName = "\u206eThe Team\u206f"
+		team.Description = "A \u17a3great\u17d3 team."
+		team.CompanyName = "\u206aAcme Inc"
+		uteam, resp := Client.UpdateTeam(team)
+		CheckNoError(t, resp)
+
+		require.Equal(t, "The Team", uteam.DisplayName, "bad unicode should be filtered from display name")
+		require.Equal(t, "A great team.", uteam.Description, "bad unicode should be filtered from description")
+		require.Equal(t, "Acme Inc", uteam.CompanyName, "bad unicode should be filtered from company name")
+	})
+
+	t.Run("patch team unicode", func(t *testing.T) {
+		team := &model.Team{
+			DisplayName: "Name",
+			Description: "Some description",
+			CompanyName: "Some company name",
+			Name:        model.NewRandomTeamName(),
+			Email:       "success+" + model.NewId() + "@simulator.amazonses.com",
+			Type:        model.TEAM_OPEN}
+		team, _ = Client.CreateTeam(team)
+
+		patch := &model.TeamPatch{}
+
+		patch.DisplayName = model.NewString("Goat\u206e Team")
+		patch.Description = model.NewString("\ufffaGreat team.")
+		patch.CompanyName = model.NewString("\u202bAcme Inc\u202c")
+
+		rteam, resp := Client.PatchTeam(team.Id, patch)
+		CheckNoError(t, resp)
+
+		require.Equal(t, "Goat Team", rteam.DisplayName, "bad unicode should be filtered from display name")
+		require.Equal(t, "Great team.", rteam.Description, "bad unicode should be filtered from description")
+		require.Equal(t, "Acme Inc", rteam.CompanyName, "bad unicode should be filtered from company name")
+	})
+}
+
 func TestRegenerateTeamInviteId(t *testing.T) {
 	th := Setup(t).InitBasic()
 	defer th.TearDown()
@@ -2366,7 +2433,7 @@ func TestInviteUsersToTeam(t *testing.T) {
 			"TeamDisplayName": th.BasicTeam.DisplayName,
 			"SiteName":        th.App.ClientConfig()["SiteName"]})

-	//Check if the email was send to the rigth email address
+	//Check if the email was send to the right email address
 	for _, email := range emailList {
 		var resultsMailbox mailservice.JSONMessageHeaderInbucket
 		err := mailservice.RetryInbucket(5, func() error {
@@ -2489,7 +2556,7 @@ func TestInviteGuestsToTeam(t *testing.T) {
 			"TeamDisplayName": th.BasicTeam.DisplayName,
 			"SiteName":        th.App.ClientConfig()["SiteName"]})

-	//Check if the email was send to the rigth email address
+	//Check if the email was send to the right email address
 	for _, email := range emailList {
 		var resultsMailbox mailservice.JSONMessageHeaderInbucket
 		err := mailservice.RetryInbucket(5, func() error {
--- a/api4/user_test.go
+++ b/api4/user_test.go
@@ -1567,6 +1567,69 @@ func TestPatchUser(t *testing.T) {
 	CheckNoError(t, resp)
 }

+func TestUserUnicodeNames(t *testing.T) {
+	th := Setup(t).InitBasic()
+	defer th.TearDown()
+	Client := th.Client
+
+	t.Run("create user unicode", func(t *testing.T) {
+		user := model.User{
+			Email:     th.GenerateTestEmail(),
+			FirstName: "Andrew\u202e",
+			LastName:  "\ufeffWiggin",
+			Nickname:  "Ender\u2028 Wiggin",
+			Password:  "hello1",
+			Username:  "\ufeffwiggin77",
+			Roles:     model.SYSTEM_ADMIN_ROLE_ID + " " + model.SYSTEM_USER_ROLE_ID}
+
+		ruser, resp := Client.CreateUser(&user)
+		CheckNoError(t, resp)
+		CheckCreatedStatus(t, resp)
+
+		_, _ = Client.Login(user.Email, user.Password)
+
+		require.Equal(t, "wiggin77", ruser.Username, "Bad Unicode not filtered from username")
+		require.Equal(t, "Andrew Wiggin", ruser.GetDisplayName(model.SHOW_FULLNAME), "Bad Unicode not filtered from displayname")
+		require.Equal(t, "Ender Wiggin", ruser.Nickname, "Bad Unicode not filtered from nickname")
+	})
+
+	t.Run("update user unicode", func(t *testing.T) {
+		user := th.CreateUser()
+		Client.Login(user.Email, user.Password)
+
+		user.Username = "wiggin\ufff9"
+		user.Nickname = "Ender\u0340 \ufffcWiggin"
+		user.FirstName = "Andrew\ufff9"
+		user.LastName = "Wig\u206fgin"
+
+		ruser, resp := Client.UpdateUser(user)
+		CheckNoError(t, resp)
+
+		require.Equal(t, "wiggin", ruser.Username, "bad unicode should be filtered from username")
+		require.Equal(t, "Ender Wiggin", ruser.Nickname, "bad unicode should be filtered from nickname")
+		require.Equal(t, "Andrew Wiggin", ruser.GetDisplayName(model.SHOW_FULLNAME), "bad unicode should be filtered from display name")
+	})
+
+	t.Run("patch user unicode", func(t *testing.T) {
+		user := th.CreateUser()
+		Client.Login(user.Email, user.Password)
+
+		patch := &model.UserPatch{}
+		patch.Nickname = model.NewString("\U000E0000Ender\u206d Wiggin\U000E007F")
+		patch.FirstName = model.NewString("\U0001d173Andrew\U0001d17a")
+		patch.LastName = model.NewString("\u2028Wiggin\u2029")
+
+		ruser, resp := Client.PatchUser(user.Id, patch)
+		CheckNoError(t, resp)
+		CheckUserSanitization(t, ruser)
+
+		require.Equal(t, "Ender Wiggin", ruser.Nickname, "Bad unicode should be filtered from nickname")
+		require.Equal(t, "Andrew", ruser.FirstName, "Bad unicode should be filtered from first name")
+		require.Equal(t, "Wiggin", ruser.LastName, "Bad unicode should be filtered from last name")
+		require.Equal(t, "Andrew Wiggin", ruser.GetDisplayName(model.SHOW_FULLNAME), "Bad unicode should be filtered from display name")
+	})
+}
+
 func TestUpdateUserAuth(t *testing.T) {
 	th := Setup(t).InitBasic()
 	defer th.TearDown()
--- a/model/channel.go
+++ b/model/channel.go
@@ -230,6 +230,9 @@ func (o *Channel) PreSave() {
 		o.Id = NewId()
 	}

+	o.Name = SanitizeUnicode(o.Name)
+	o.DisplayName = SanitizeUnicode(o.DisplayName)
+
 	o.CreateAt = GetMillis()
 	o.UpdateAt = o.CreateAt
 	o.ExtraUpdateAt = 0
@@ -237,6 +240,8 @@ func (o *Channel) PreSave() {

 func (o *Channel) PreUpdate() {
 	o.UpdateAt = GetMillis()
+	o.Name = SanitizeUnicode(o.Name)
+	o.DisplayName = SanitizeUnicode(o.DisplayName)
 }

 func (o *Channel) IsGroupOrDirect() bool {
--- a/model/team.go
+++ b/model/team.go
@@ -203,6 +203,11 @@ func (o *Team) PreSave() {
 	o.CreateAt = GetMillis()
 	o.UpdateAt = o.CreateAt

+	o.Name = SanitizeUnicode(o.Name)
+	o.DisplayName = SanitizeUnicode(o.DisplayName)
+	o.Description = SanitizeUnicode(o.Description)
+	o.CompanyName = SanitizeUnicode(o.CompanyName)
+
 	if len(o.InviteId) == 0 {
 		o.InviteId = NewId()
 	}
@@ -210,6 +215,10 @@ func (o *Team) PreSave() {

 func (o *Team) PreUpdate() {
 	o.UpdateAt = GetMillis()
+	o.Name = SanitizeUnicode(o.Name)
+	o.DisplayName = SanitizeUnicode(o.DisplayName)
+	o.Description = SanitizeUnicode(o.Description)
+	o.CompanyName = SanitizeUnicode(o.CompanyName)
 }

 func IsReservedTeamName(s string) bool {
--- a/model/user.go
+++ b/model/user.go
@@ -329,6 +329,11 @@ func (u *User) PreSave() {
 		u.AuthData = nil
 	}

+	u.Username = SanitizeUnicode(u.Username)
+	u.FirstName = SanitizeUnicode(u.FirstName)
+	u.LastName = SanitizeUnicode(u.LastName)
+	u.Nickname = SanitizeUnicode(u.Nickname)
+
 	u.Username = NormalizeUsername(u.Username)
 	u.Email = NormalizeEmail(u.Email)

@@ -362,10 +367,21 @@ func (u *User) PreSave() {

 // PreUpdate should be run before updating the user in the db.
 func (u *User) PreUpdate() {
+	u.Username = SanitizeUnicode(u.Username)
+	u.FirstName = SanitizeUnicode(u.FirstName)
+	u.LastName = SanitizeUnicode(u.LastName)
+	u.Nickname = SanitizeUnicode(u.Nickname)
+	u.BotDescription = SanitizeUnicode(u.BotDescription)
+
 	u.Username = NormalizeUsername(u.Username)
 	u.Email = NormalizeEmail(u.Email)
 	u.UpdateAt = GetMillis()

+	u.FirstName = SanitizeUnicode(u.FirstName)
+	u.LastName = SanitizeUnicode(u.LastName)
+	u.Nickname = SanitizeUnicode(u.Nickname)
+	u.BotDescription = SanitizeUnicode(u.BotDescription)
+
 	if u.AuthData != nil && *u.AuthData == "" {
 		u.AuthData = nil
 	}
--- a/model/utils.go
+++ b/model/utils.go
@@ -647,3 +647,48 @@ func AsStringBoolMap(list []string) map[string]bool {
 	}
 	return listMap
 }
+
+// SanitizeUnicode will remove undesirable Unicode characters from a string.
+func SanitizeUnicode(s string) string {
+	return strings.Map(filterBlacklist, s)
+}
+
+// filterBlacklist returns `r` if it is not in the blacklist, otherwise drop (-1).
+// Blacklist is taken from https://www.w3.org/TR/unicode-xml/#Charlist
+func filterBlacklist(r rune) rune {
+	const drop = -1
+	switch r {
+	case '\u0340', '\u0341': // clones of grave and acute; deprecated in Unicode
+		return drop
+	case '\u17A3', '\u17D3': // obsolete characters for Khmer; deprecated in Unicode
+		return drop
+	case '\u2028', '\u2029': // line and paragraph separator
+		return drop
+	case '\u202A', '\u202B', '\u202C', '\u202D', '\u202E': // BIDI embedding controls
+		return drop
+	case '\u206A', '\u206B': // activate/inhibit symmetric swapping; deprecated in Unicode
+		return drop
+	case '\u206C', '\u206D': // activate/inhibit Arabic form shaping; deprecated in Unicode
+		return drop
+	case '\u206E', '\u206F': // activate/inhibit national digit shapes; deprecated in Unicode
+		return drop
+	case '\uFFF9', '\uFFFA', '\uFFFB': // interlinear annotation characters
+		return drop
+	case '\uFEFF': // byte order mark
+		return drop
+	case '\uFFFC': // object replacement character
+		return drop
+	}
+
+	// Scoping for musical notation
+	if r >= 0x0001D173 && r <= 0x0001D17A {
+		return drop
+	}
+
+	// Language tag code points
+	if r >= 0x000E0000 && r <= 0x000E007F {
+		return drop
+	}
+
+	return r
+}
--- a/model/utils_test.go
+++ b/model/utils_test.go
@@ -4,6 +4,7 @@
 package model

 import (
+	"bytes"
 	"fmt"
 	"net/http"
 	"reflect"
@@ -736,3 +737,33 @@ func checkNowhereNil(t *testing.T, name string, value interface{}) bool {
 		return true
 	}
 }
+
+func TestSanitizeUnicode(t *testing.T) {
+	buf := bytes.Buffer{}
+	buf.WriteString("Hello")
+	buf.WriteRune(0x1d173)
+	buf.WriteRune(0x1d17a)
+	buf.WriteString(" there.")
+
+	musicArg := buf.String()
+	musicWant := "Hello there."
+
+	tests := []struct {
+		name string
+		arg  string
+		want string
+	}{
+		{name: "empty string", arg: "", want: ""},
+		{name: "ascii only", arg: "Hello There", want: "Hello There"},
+		{name: "allowed unicode", arg: "Ādam likes Iñtërnâtiônàližætiøn", want: "Ādam likes Iñtërnâtiônàližætiøn"},
+		{name: "allowed unicode escaped", arg: "\u00eaI like hats\u00e2", want: "êI like hatsâ"},
+		{name: "blacklist char, don't reverse string", arg: "\u202E2resu", want: "2resu"},
+		{name: "blacklist chars, scoping musical notation", arg: musicArg, want: musicWant},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := SanitizeUnicode(tt.arg)
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}