From 19c7c6fa7afe2ac89195d57c833cb95aef8cdefd Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Thu, 23 Mar 2023 13:03:59 +0530 Subject: [PATCH] MM-51486: Implementing UTS 46 to check for preview blocklist (#22601) There is a standard IDNA2008 which defines how unicode characters in domain names can be compared using UTS (Unicode Technical Standard) 46. We use that to convert any links in messages to ASCII and then compare with the domain list in config. https://mattermost.atlassian.net/browse/MM-51486 ```release-note NONE ``` --- server/channels/app/post_metadata.go | 33 +++++++++++++++++++++-- server/channels/app/post_metadata_test.go | 15 +++++++++++ server/channels/app/team.go | 6 ----- 3 files changed, 46 insertions(+), 8 deletions(-) diff --git a/server/channels/app/post_metadata.go b/server/channels/app/post_metadata.go index ad4b0e1336..fc87374fe3 100644 --- a/server/channels/app/post_metadata.go +++ b/server/channels/app/post_metadata.go @@ -16,6 +16,7 @@ import ( "time" "github.com/dyatlov/go-opengraph/opengraph" + "golang.org/x/net/idna" "github.com/mattermost/mattermost-server/v6/model" "github.com/mattermost/mattermost-server/v6/server/channels/app/platform" @@ -443,9 +444,23 @@ func (a *App) getCustomEmojisForPost(c request.CTX, post *model.Post, reactions } func (a *App) isLinkAllowedForPreview(link string) bool { - domains := a.normalizeDomains(*a.Config().ServiceSettings.RestrictLinkPreviews) + domains := normalizeDomains(*a.Config().ServiceSettings.RestrictLinkPreviews) for _, d := range domains { - if strings.Contains(link, d) { + parsed, err := url.Parse(link) + if err != nil { + a.Log().Warn("Unable to parse the link", mlog.String("link", link), mlog.Err(err)) + // We disable link preview if link is badly formed + // to remain on the safe side + return false + } + // Conforming to IDNA2008 using the UTS-46 standard. + cleaned, err := idna.Lookup.ToASCII(parsed.Hostname()) + if err != nil { + a.Log().Warn("Unable to lookup hostname to ASCII", mlog.String("hostname", parsed.Hostname()), mlog.Err(err)) + // Same applies if compatibility processing fails. + return false + } + if strings.Contains(cleaned, d) { return false } } @@ -453,6 +468,20 @@ func (a *App) isLinkAllowedForPreview(link string) bool { return true } +func normalizeDomains(domains string) []string { + // commas and @ signs are optional + // can be in the form of "@corp.mattermost.com, mattermost.com mattermost.org" -> corp.mattermost.com mattermost.com mattermost.org + return strings.Fields( + strings.TrimSpace( + strings.ToLower( + strings.ReplaceAll( + strings.ReplaceAll(domains, "@", " "), + ",", " "), + ), + ), + ) +} + // Given a string, returns the first autolinked URL in the string as well as an array of all Markdown // images of the form ![alt text](image url). Note that this does not return Markdown links of the // form [text](url). diff --git a/server/channels/app/post_metadata_test.go b/server/channels/app/post_metadata_test.go index 3c62f097b1..9171b9c657 100644 --- a/server/channels/app/post_metadata_test.go +++ b/server/channels/app/post_metadata_test.go @@ -1664,6 +1664,21 @@ func TestGetFirstLinkAndImages(t *testing.T) { ExpectedFirstLink: "", ExpectedImages: []string{}, }, + "test with denied domain as username, password": { + Input: "link as http://example.com:example.com@example1.com/link1", + ExpectedFirstLink: "http://example.com:example.com@example1.com/link1", + ExpectedImages: []string{}, + }, + "test with URL encoded": { + Input: "link as https://example%E3%80%82com/link1", + ExpectedFirstLink: "", + ExpectedImages: []string{}, + }, + "test with unicode": { + Input: "link as https://example。com/link1", + ExpectedFirstLink: "", + ExpectedImages: []string{}, + }, } { th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.RestrictLinkPreviews = "example.com, test.com" diff --git a/server/channels/app/team.go b/server/channels/app/team.go index cd33057b52..4a08fcd9ab 100644 --- a/server/channels/app/team.go +++ b/server/channels/app/team.go @@ -356,12 +356,6 @@ func (a *App) CreateTeamWithUser(c *request.Context, team *model.Team, userID st return rteam, nil } -func (a *App) normalizeDomains(domains string) []string { - // commas and @ signs are optional - // can be in the form of "@corp.mattermost.com, mattermost.com mattermost.org" -> corp.mattermost.com mattermost.com mattermost.org - return strings.Fields(strings.TrimSpace(strings.ToLower(strings.Replace(strings.Replace(domains, "@", " ", -1), ",", " ", -1)))) -} - func (a *App) UpdateTeam(team *model.Team) (*model.Team, *model.AppError) { oldTeam, err := a.ch.srv.teamService.UpdateTeam(team, teams.UpdateOptions{Sanitized: true}) if err != nil {