From ddbceb53756f8e2ef8cfb5ee43eccc0ead5a59ae Mon Sep 17 00:00:00 2001 From: Umputun Date: Fri, 26 Mar 2021 16:00:50 -0500 Subject: [PATCH] post-sanitize location's url in case if stored already #927 this is done via service.alterComment the same way as we handle leftovers from prev issues with results stored already --- backend/app/store/comment.go | 6 +++--- backend/app/store/comment_test.go | 2 +- backend/app/store/service/service.go | 4 +++- backend/app/store/service/service_test.go | 9 ++++++--- 4 files changed, 13 insertions(+), 8 deletions(-) diff --git a/backend/app/store/comment.go b/backend/app/store/comment.go index b0787784..0c0e0fc6 100644 --- a/backend/app/store/comment.go +++ b/backend/app/store/comment.go @@ -126,8 +126,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 = c.sanitizeAsURL(c.User.Picture) - c.Locator.URL = c.sanitizeAsURL(c.Locator.URL) + c.User.Picture = c.SanitizeAsURL(c.User.Picture) + c.Locator.URL = c.SanitizeAsURL(c.Locator.URL) } // Snippet from comment's text @@ -155,7 +155,7 @@ var reHref = regexp.MustCompile(`]*?\s+)?href="([^"]*)"`) // wrap with href to trigger bluemonday sanitizer // clean href after sanitizing done -func (c *Comment) sanitizeAsURL(inp string) string { +func (c *Comment) SanitizeAsURL(inp string) string { h := fmt.Sprintf(``, inp) clean := bluemonday.UGCPolicy().Sanitize(h) if match := reHref.FindStringSubmatch(clean); len(match) > 1 { diff --git a/backend/app/store/comment_test.go b/backend/app/store/comment_test.go index 2ffe2ac9..ad1a9942 100644 --- a/backend/app/store/comment_test.go +++ b/backend/app/store/comment_test.go @@ -211,7 +211,7 @@ func TestComment_sanitizeAsURL(t *testing.T) { tt := tt c := Comment{} t.Run(strconv.Itoa(i), func(t *testing.T) { - assert.Equal(t, tt.out, c.sanitizeAsURL(tt.inp)) + 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 70bacd40..788d587b 100644 --- a/backend/app/store/service/service.go +++ b/backend/app/store/service/service.go @@ -125,7 +125,8 @@ func (s *DataStore) FindSince(locator store.Locator, sortMethod string, user sto } changedSort := false - // set votes controversy for comments added prior to #274 + // sets votes controversy for comments added prior to #274 + // also sanitizes locator.URL for comments added prior to #927 for i, c := range comments { if c.Controversy == 0 && len(c.Votes) > 0 { c.Controversy = s.controversy(s.upsAndDowns(c)) @@ -924,6 +925,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 } diff --git a/backend/app/store/service/service_test.go b/backend/app/store/service/service_test.go index 86abd968..cc2f48b9 100644 --- a/backend/app/store/service/service_test.go +++ b/backend/app/store/service/service_test.go @@ -1457,9 +1457,11 @@ func TestService_alterComment(t *testing.T) { engineMock.On("Flag", engine.FlagRequest{Flag: engine.Verified, UserID: "devid"}).Return(false, nil) svc := DataStore{Engine: &engineMock} - r := svc.alterComment(store.Comment{ID: "123", User: store.User{IP: "127.0.0.1", ID: "devid"}}, + r := svc.alterComment(store.Comment{ID: "123", User: store.User{IP: "127.0.0.1", ID: "devid"}, + Locator: store.Locator{URL: "http://example.com?foo=bar"}}, store.User{Name: "dev", ID: "devid", Admin: false}) - assert.Equal(t, store.Comment{ID: "123", User: store.User{IP: "", ID: "devid"}}, r, "ip cleaned") + assert.Equal(t, store.Comment{ID: "123", User: store.User{IP: "", ID: "devid"}, + Locator: store.Locator{URL: "http://example.com?foo=bar"}}, r, "ip cleaned") r = svc.alterComment(store.Comment{ID: "123", User: store.User{IP: "127.0.0.1", ID: "devid"}}, store.User{Name: "dev", ID: "devid", Admin: true}) assert.Equal(t, store.Comment{ID: "123", User: store.User{IP: "127.0.0.1", ID: "devid"}}, r, "ip not cleaned") @@ -1476,7 +1478,8 @@ func TestService_alterComment(t *testing.T) { engineMock.On("Flag", engine.FlagRequest{Flag: engine.Blocked, UserID: "devid"}).Return(true, nil) engineMock.On("Flag", engine.FlagRequest{Flag: engine.Verified, UserID: "devid"}).Return(false, nil) svc = DataStore{Engine: &engineMock} - r = svc.alterComment(store.Comment{ID: "123", User: store.User{IP: "127.0.0.1", ID: "devid", Verified: true}}, + r = svc.alterComment(store.Comment{ID: "123", User: store.User{IP: "127.0.0.1", ID: "devid", Verified: true}, + Locator: store.Locator{URL: "javascript:alert('XSS1')"}}, store.User{Name: "dev", ID: "devid", Admin: false}) assert.Equal(t, store.Comment{ID: "123", User: store.User{IP: "", Verified: true, Blocked: true, ID: "devid"}, Deleted: false}, r, "blocked")