diff --git a/backend/app/notify/email.go b/backend/app/notify/email.go index ce7b1bfb..1199fd1a 100644 --- a/backend/app/notify/email.go +++ b/backend/app/notify/email.go @@ -4,14 +4,15 @@ import ( "bytes" "context" "fmt" + "html/template" "net/url" - "text/template" "time" log "github.com/go-pkgz/lgr" ntf "github.com/go-pkgz/notify" "github.com/go-pkgz/repeater/v2" "github.com/hashicorp/go-multierror" + "github.com/microcosm-cc/bluemonday" "github.com/umputun/remark42/backend/app/templates" ) @@ -42,12 +43,12 @@ type Email struct { type msgTmplData struct { UserName string UserPicture string - CommentText string + CommentText template.HTML CommentLink string CommentDate time.Time ParentUserName string ParentUserPicture string - ParentCommentText string + ParentCommentText template.HTML ParentCommentLink string ParentCommentDate time.Time PostTitle string @@ -56,6 +57,30 @@ type msgTmplData struct { ForAdmin bool } +// emailCommentPolicy sanitizes comment HTML for inclusion in notification emails. +// It is intentionally stricter than the store-level UGC policy used for web rendering: +// links () and images () are dropped so a comment can't smuggle phishing links +// or remote tracking pixels into an email sent from the legitimate remark42 address, +// while basic inline and block text formatting is preserved. +var emailCommentPolicy = func() *bluemonday.Policy { + p := bluemonday.NewPolicy() + p.AllowElements( + "p", "br", "hr", "div", "span", + "b", "strong", "i", "em", "u", "s", "strike", "del", "ins", "sub", "sup", "mark", "small", + "blockquote", "q", "cite", + "code", "pre", "kbd", "samp", "var", + "ul", "ol", "li", "dl", "dt", "dd", + "h1", "h2", "h3", "h4", "h5", "h6", + ) + return p +}() + +// emailSafeHTML strips links and images from pre-rendered comment HTML and returns +// it as template.HTML so html/template renders the remaining safe formatting as-is. +func emailSafeHTML(commentHTML string) template.HTML { + return template.HTML(emailCommentPolicy.Sanitize(commentHTML)) //nolint:gosec // sanitized above: / dropped, only formatting tags survive +} + // verifyTmplData store data for verification message template execution type verifyTmplData struct { User string @@ -257,7 +282,7 @@ func (e *Email) buildMessageFromRequest(req Request, email string, forAdmin bool tmplData := msgTmplData{ UserName: req.Comment.User.Name, UserPicture: req.Comment.User.Picture, - CommentText: req.Comment.Text, + CommentText: emailSafeHTML(req.Comment.Text), CommentLink: commentURLPrefix + req.Comment.ID, CommentDate: req.Comment.Timestamp, PostTitle: req.Comment.PostTitle, @@ -269,7 +294,7 @@ func (e *Email) buildMessageFromRequest(req Request, email string, forAdmin bool if req.Comment.ParentID != "" { tmplData.ParentUserName = req.parent.User.Name tmplData.ParentUserPicture = req.parent.User.Picture - tmplData.ParentCommentText = req.parent.Text + tmplData.ParentCommentText = emailSafeHTML(req.parent.Text) tmplData.ParentCommentLink = commentURLPrefix + req.parent.ID tmplData.ParentCommentDate = req.parent.Timestamp } diff --git a/backend/app/notify/email_test.go b/backend/app/notify/email_test.go index 6589856a..87117d38 100644 --- a/backend/app/notify/email_test.go +++ b/backend/app/notify/email_test.go @@ -3,8 +3,8 @@ package notify import ( "context" "fmt" + "html/template" "testing" - "text/template" ntf "github.com/go-pkgz/notify" "github.com/stretchr/testify/assert" @@ -164,7 +164,7 @@ User: test_user 01.01.0001 at 00:00 Comment: test@example.org for parent_user -Unsubscribe link: https://remark42.com/api/v1/email/unsubscribe?site=&tkn=token +Unsubscribe link: https://remark42.com/api/v1/email/unsubscribe?site=&tkn=token `, msg.body) assert.Equal(t, "https://remark42.com/api/v1/email/unsubscribe?site=&tkn=token", msg.unsubscribeLink) assert.Equal(t, `New reply to your comment for "test_title"`, msg.subject) @@ -190,6 +190,50 @@ admin@example.org assert.Empty(t, msg.unsubscribeLink) } +func TestEmail_CommentTextSanitizedForEmail(t *testing.T) { + // comment HTML reaching the email path is sanitized by the store-level UGC policy, + // which permits and . The email must drop both so a comment can't inject + // phishing links or remote tracking pixels into a notification (GHSA-74pc-3r2m-ppx3). + email, err := NewEmail(EmailParams{ + From: "from@example.org", + MsgTemplatePath: "testdata/msg.html.tmpl", + }, ntf.SMTPParams{}) + require.NoError(t, err) + email.TokenGenFn = TokenGenFn + + malicious := `hello click to verify` + + ` kept` + req := Request{ + Comment: store.Comment{ID: "999", User: store.User{ID: "1", Name: "test_user"}, PostTitle: "test_title", Text: malicious}, + Emails: []string{"test@example.org"}, + } + msg, err := email.buildMessageFromRequest(req, req.Emails[0], false) + require.NoError(t, err) + + assert.NotContains(t, msg.body, "phishing.example", "phishing link must be stripped") + assert.NotContains(t, msg.body, "attacker.example", "tracking pixel must be stripped") + assert.NotContains(t, msg.body, "kept", "basic formatting is preserved") +} + +// emailSafeHTML drops links/images while keeping inline/block formatting and escaping nothing extra. +func TestEmailSafeHTML(t *testing.T) { + tbl := []struct{ name, in, want string }{ + {"strips anchor keeps text", `x`, "x"}, + {"strips image entirely", `ab`, "ab"}, + {"keeps bold/italic/code", `bic`, `bic`}, + {"keeps blockquote and lists", `
q
`, `
q
`}, + {"drops onclick handlers", `s`, `s`}, + } + for _, tt := range tbl { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, string(emailSafeHTML(tt.in))) + }) + } +} + func TestEmail_SendVerification(t *testing.T) { email, err := NewEmail(EmailParams{ From: "from@example.org",