From dfb3436f306468745a47a5f0e25fcc5c9a931c56 Mon Sep 17 00:00:00 2001 From: Umputun Date: Fri, 26 Mar 2021 16:42:56 -0500 Subject: [PATCH] backport url sanitizer to 1.6 --- backend/app/store/comment.go | 17 ++++++++- backend/app/store/comment_test.go | 56 ++++++++++++++++++++++++++++ backend/app/store/service/service.go | 1 + 3 files changed, 73 insertions(+), 1 deletion(-) diff --git a/backend/app/store/comment.go b/backend/app/store/comment.go index 85b2ae6d..9439996d 100644 --- a/backend/app/store/comment.go +++ b/backend/app/store/comment.go @@ -1,6 +1,7 @@ package store import ( + "fmt" "html/template" "regexp" "strings" @@ -121,7 +122,8 @@ func (c *Comment) Sanitize() { c.Orig = p.Sanitize(c.Orig) c.User.ID = template.HTMLEscapeString(c.User.ID) c.User.Name = c.escapeHTMLWithSome(c.User.Name) - c.User.Picture = p.Sanitize(c.User.Picture) + c.User.Picture = c.SanitizeAsURL(c.User.Picture) + c.Locator.URL = c.SanitizeAsURL(c.Locator.URL) } // Snippet from comment's text @@ -145,6 +147,19 @@ func (c *Comment) Snippet(limit int) string { return string(snippet) + " ..." } +var reHref = regexp.MustCompile(`]*?\s+)?href="([^"]*)"`) + +// SanitizeAsURL drops dangerous code from a url. +// It wraps input with href to trigger bluemonday sanitizer and cleans href after sanitizing done +func (c *Comment) SanitizeAsURL(inp string) string { + h := fmt.Sprintf(``, inp) + clean := bluemonday.UGCPolicy().Sanitize(h) + if match := reHref.FindStringSubmatch(clean); len(match) > 1 { + return match[1] + } + return "" // this shouldn't happen as we build the href +} + func (c *Comment) escapeHTMLWithSome(inp string) string { res := template.HTMLEscapeString(inp) res = strings.Replace(res, """, "\"", -1) diff --git a/backend/app/store/comment_test.go b/backend/app/store/comment_test.go index 29495d2c..cd102c80 100644 --- a/backend/app/store/comment_test.go +++ b/backend/app/store/comment_test.go @@ -47,6 +47,25 @@ func TestComment_Sanitize(t *testing.T) { inp: Comment{Text: "blah & & 123", User: User{Name: "name <> & ' ` \""}}, out: Comment{Text: `blah & & 123`, User: User{Name: "name <> & ' ` \""}}, }, + + { + inp: Comment{Text: "blah blah", Locator: Locator{URL: "javascript:alert('XSS1')"}}, + out: Comment{Text: "blah blah", Locator: Locator{URL: ""}}, + }, + { + inp: Comment{Text: "blah blah", Locator: Locator{URL: "javascript:alert(document.domain)//"}}, + out: Comment{Text: "blah blah", Locator: Locator{URL: ""}}, + }, + { + inp: Comment{Text: "blah blah", Locator: Locator{URL: ""}}, + out: Comment{Text: "blah blah", Locator: Locator{URL: "%3Cscript%3Ealert%28%29%3C/script%3E"}}, + }, + { + inp: Comment{Text: "blah blah", + Locator: Locator{URL: "/p/2021/03/23/prep-747/#remark42__comment-1b365913-7056-4920-b9ad-01304bdda085"}}, + out: Comment{Text: "blah blah", + Locator: Locator{URL: "/p/2021/03/23/prep-747/#remark42__comment-1b365913-7056-4920-b9ad-01304bdda085"}}, + }, } for n, tt := range tbl { @@ -156,3 +175,40 @@ func TestComment_Snippet(t *testing.T) { }) } } + +func TestComment_SanitizeAsURL(t *testing.T) { + + tbl := []struct { + inp, out string + }{ + { + "/p/2021/03/23/prep-747/#remark42__comment-1b365913-7056-4920-b9ad-01304bdda085", + "/p/2021/03/23/prep-747/#remark42__comment-1b365913-7056-4920-b9ad-01304bdda085", + }, + { + "https://radio-t.com/p/2021/03/23/prep-747/#remark42__comment-1b365913-7056-4920-b9ad-01304bdda085", + "https://radio-t.com/p/2021/03/23/prep-747/#remark42__comment-1b365913-7056-4920-b9ad-01304bdda085", + }, + { + "javascript:alert(document.domain)//", + "", + }, + { + "", + "%3Cscript%3Ealert%28%29%3C/script%3E", + }, + { + "xxx", + "", + }, + } + + for i, tt := range tbl { + tt := tt + c := Comment{} + t.Run(strconv.Itoa(i), func(t *testing.T) { + assert.Equal(t, tt.out, c.SanitizeAsURL(tt.inp)) + }) + } + +} diff --git a/backend/app/store/service/service.go b/backend/app/store/service/service.go index a3d65fd9..054eb5cd 100644 --- a/backend/app/store/service/service.go +++ b/backend/app/store/service/service.go @@ -902,6 +902,7 @@ func (s *DataStore) alterComment(c store.Comment, user store.User) (res store.Co } c = s.prepVotes(c, user) + c.Locator.URL = c.SanitizeAsURL(c.Locator.URL) // urls prior to #927 return c }