From 270ff856cbecad214e38321b8438499a7bf272c0 Mon Sep 17 00:00:00 2001 From: Dmitry Verkhoturov Date: Mon, 9 Jan 2023 21:19:34 +0100 Subject: [PATCH] don't allow relative links in comments (url) is a text inserted by default and never an intended URL. That additional validation will ensure that users won't post relative links because they are rarely intended. --- backend/app/rest/api/rest_private_test.go | 19 +++++++++++++++ backend/app/store/comment.go | 4 ++-- backend/app/store/formatter.go | 28 ++++++++++++++--------- backend/app/store/service/service.go | 21 +++++++++++++++-- backend/app/store/service/service_test.go | 17 ++++++++------ 5 files changed, 67 insertions(+), 22 deletions(-) diff --git a/backend/app/rest/api/rest_private_test.go b/backend/app/rest/api/rest_private_test.go index 0e412863f9..6537519b9f 100644 --- a/backend/app/rest/api/rest_private_test.go +++ b/backend/app/rest/api/rest_private_test.go @@ -166,6 +166,25 @@ func TestRest_CreateWithRestrictedWord(t *testing.T) { assert.Equal(t, "invalid comment", c["details"]) } +func TestRest_CreateRelativeURL(t *testing.T) { + ts, _, teardown := startupT(t) + defer teardown() + + // check that it's not possible to click insert URL button and not alter the URL in it (which is `url` by default) + relativeURLText := `{"text": "here is a link with relative URL: [google.com](url)", "locator":{"url": "https://radio-t.com/blah1", "site": "remark42"}}` + resp, err := post(t, ts.URL+"/api/v1/comment", relativeURLText) + assert.NoError(t, err) + assert.Equal(t, http.StatusBadRequest, resp.StatusCode) + b, err := io.ReadAll(resp.Body) + assert.NoError(t, err) + assert.NoError(t, resp.Body.Close()) + c := R.JSON{} + err = json.Unmarshal(b, &c) + assert.NoError(t, err) + assert.Equal(t, "links should start with mailto:, http:// or https://", c["error"]) + assert.Equal(t, "invalid comment", c["details"]) +} + func TestRest_CreateRejected(t *testing.T) { ts, _, teardown := startupT(t) defer teardown() diff --git a/backend/app/store/comment.go b/backend/app/store/comment.go index 6c94065eb2..bd7c09f6d9 100644 --- a/backend/app/store/comment.go +++ b/backend/app/store/comment.go @@ -121,8 +121,8 @@ func (c *Comment) Sanitize() { // special case for embedding the quotes from Twitter p.AllowAttrs("class").Matching(regexp.MustCompile("^twitter-tweet$")).OnElements("blockquote") // this is list of tag classes which could be produced by chroma code renderer - // source: https://github.com/alecthomas/chroma/blob/cc2dd5b/types.go#L211-L307 - const codeSpanClassRegex = "^(bg|chroma|line|ln|lnt|hl|lntable|lntd|cl|w|err|x|k|kc" + + // source: https://github.com/alecthomas/chroma/blob/c263f6f/types.go#L209-L306 + const codeSpanClassRegex = "^(bg|chroma|line|ln|lnt|hl|lntable|lntd|lnlinks|cl|w|err|x|k|kc" + "|kd|kn|kp|kr|kt|n|na|nb|bp|nc|no|nd|ni|ne|nf|fm|py|nl|nn|nx|nt|nv|vc|vg" + "|vi|vm|l|ld|s|sa|sb|sc|dl|sd|s2|se|sh|si|sx|sr|s1|ss|m|mb|mf|mh|mi|il" + "|mo|o|ow|p|c|ch|cm|cp|cpf|c1|cs|g|gd|ge|gr|gh|gi|go|gp|gs|gu|gt|gl)$" diff --git a/backend/app/store/formatter.go b/backend/app/store/formatter.go index 26189c85f6..864a9b9fe5 100644 --- a/backend/app/store/formatter.go +++ b/backend/app/store/formatter.go @@ -42,17 +42,8 @@ func (f *CommentFormatter) Format(c Comment) Comment { // FormatText converts text with markdown processor, applies external converters and shortens links func (f *CommentFormatter) FormatText(txt string) (res string) { - mdExt := bf.NoIntraEmphasis | bf.Tables | bf.FencedCode | - bf.Strikethrough | bf.SpaceHeadings | bf.HardLineBreak | - bf.BackslashLineBreak | bf.Autolink - - rend := bf.NewHTMLRenderer(bf.HTMLRendererParameters{ - Flags: bf.Smartypants | bf.SmartypantsFractions | bf.SmartypantsDashes | bf.SmartypantsAngledQuotes, - }) - - extRend := bfchroma.NewRenderer(bfchroma.Extend(rend), bfchroma.ChromaOptions(html.WithClasses(true))) - - res = string(bf.Run([]byte(txt), bf.WithExtensions(mdExt), bf.WithRenderer(extRend))) + mdExt, rend := GetMdExtensionsAndRenderer() + res = string(bf.Run([]byte(txt), bf.WithExtensions(mdExt), bf.WithRenderer(rend))) res = f.unEscape(res) for _, conv := range f.converters { @@ -126,3 +117,18 @@ func (f *CommentFormatter) lazyImage(commentHTML string) (resHTML string) { } return resHTML } + +// GetMdExtensionsAndRenderer returns blackfriday extensions and renderer used for rendering markdown +// within store module. +func GetMdExtensionsAndRenderer() (bf.Extensions, *bfchroma.Renderer) { + mdExt := bf.NoIntraEmphasis | bf.Tables | bf.FencedCode | + bf.Strikethrough | bf.SpaceHeadings | bf.HardLineBreak | + bf.BackslashLineBreak | bf.Autolink + + rend := bf.NewHTMLRenderer(bf.HTMLRendererParameters{ + Flags: bf.Smartypants | bf.SmartypantsFractions | bf.SmartypantsDashes | bf.SmartypantsAngledQuotes, + }) + + extRend := bfchroma.NewRenderer(bfchroma.Extend(rend), bfchroma.ChromaOptions(html.WithClasses(true))) + return mdExt, extRend +} diff --git a/backend/app/store/service/service.go b/backend/app/store/service/service.go index 102cdca9a5..e03ccc2ad7 100644 --- a/backend/app/store/service/service.go +++ b/backend/app/store/service/service.go @@ -14,6 +14,7 @@ import ( log "github.com/go-pkgz/lgr" "github.com/google/uuid" "github.com/hashicorp/go-multierror" + bf "github.com/russross/blackfriday/v2" "github.com/umputun/remark42/backend/app/store" "github.com/umputun/remark42/backend/app/store/admin" @@ -627,7 +628,9 @@ func (s *DataStore) Counts(siteID string, postIDs []string) ([]store.PostInfo, e return res, nil } -// ValidateComment checks if comment size below max and user fields set +// ValidateComment checks if comment size below max and user fields set. +// It also validates the absence of relative links as they are almost never the intention of the commenter, +// usually added by mistakes and only create confusion. func (s *DataStore) ValidateComment(c *store.Comment) error { maxSize := s.MaxCommentSize if s.MaxCommentSize <= 0 { @@ -642,7 +645,21 @@ func (s *DataStore) ValidateComment(c *store.Comment) error { if c.User.ID == "" || c.User.Name == "" { return fmt.Errorf("empty user info") } - return nil + + mdExt, rend := store.GetMdExtensionsAndRenderer() + parser := bf.New(bf.WithRenderer(rend), bf.WithExtensions(bf.CommonExtensions), bf.WithExtensions(mdExt)) + var wrongLinkError error + parser.Parse([]byte(c.Orig)).Walk(func(node *bf.Node, entering bool) bf.WalkStatus { + if len(node.LinkData.Destination) != 0 && + !(strings.HasPrefix(string(node.LinkData.Destination), "http://") || + strings.HasPrefix(string(node.LinkData.Destination), "https://") || + strings.HasPrefix(string(node.LinkData.Destination), "mailto:")) { + wrongLinkError = fmt.Errorf("links should start with mailto:, http:// or https://") + return bf.Terminate + } + return bf.GoToNext + }) + return wrongLinkError } // IsAdmin checks if usesID in the list of admins diff --git a/backend/app/store/service/service_test.go b/backend/app/store/service/service_test.go index e4ff08f7b6..13638fc522 100644 --- a/backend/app/store/service/service_test.go +++ b/backend/app/store/service/service_test.go @@ -780,22 +780,25 @@ func TestService_ValidateComment(t *testing.T) { tbl := []struct { inp store.Comment - err error + err string }{ - {inp: store.Comment{}, err: fmt.Errorf("empty comment text")}, - {inp: store.Comment{Orig: "something blah", User: store.User{ID: "myid", Name: "name"}}, err: nil}, - {inp: store.Comment{Orig: "something blah", User: store.User{ID: "myid"}}, err: fmt.Errorf("empty user info")}, - {inp: store.Comment{Orig: longText, User: store.User{ID: "myid", Name: "name"}}, err: fmt.Errorf("comment text exceeded max allowed size 2000 (4000)")}, + {inp: store.Comment{}, err: "empty comment text"}, + {inp: store.Comment{Orig: "something blah", User: store.User{ID: "myid", Name: "name"}}, err: ""}, + {inp: store.Comment{Orig: "something blah", User: store.User{ID: "myid"}}, err: "empty user info"}, + {inp: store.Comment{Orig: longText, User: store.User{ID: "myid", Name: "name"}}, err: "comment text exceeded max allowed size 2000 (4000)"}, + {inp: store.Comment{Orig: "here is a link with relative URL: [google.com](url)", User: store.User{ID: "myid", Name: "name"}}, err: "links should start with mailto:, http:// or https://"}, + {inp: store.Comment{Orig: "here is a link with relative URL: [google.com](url)", User: store.User{ID: "myid", Name: "name"}}, err: "links should start with mailto:, http:// or https://"}, + {inp: store.Comment{Orig: "multiple links, one is bad: [test](http://test) [test2](bad_url) [test3](https://test3)", User: store.User{ID: "myid", Name: "name"}}, err: "links should start with mailto:, http:// or https://"}, } for n, tt := range tbl { err := b.ValidateComment(&tt.inp) - if tt.err == nil { + if tt.err == "" { assert.NoError(t, err, "check #%d", n) continue } require.Error(t, err) - assert.EqualError(t, tt.err, err.Error(), "check #%d", n) + assert.EqualError(t, err, tt.err, "check #%d", n) } }