From 3e18681ca7b9718d5536debc738018ae76cfe964 Mon Sep 17 00:00:00 2001 From: Dmitry Verkhoturov Date: Tue, 30 Jun 2026 20:41:17 +0100 Subject: [PATCH] Sanitize comment text in email notifications (GHSA-74pc-3r2m-ppx3) Email notification templates rendered the comment HTML via text/template, so the store-level UGC sanitizer's permitted and tags reached the email body verbatim. An authenticated user could plant phishing links and remote tracking pixels in notification emails sent from the legitimate remark42 address. Switch notify to html/template (auto-escaping every non-HTML field) and add a stricter email-only bluemonday policy that drops and while keeping basic text formatting; the sanitized comment HTML is passed as template.HTML. Add regression tests asserting links and images are stripped while anchor text and formatting survive. --- backend/app/notify/email.go | 35 +++++++++++++++++++---- backend/app/notify/email_test.go | 48 ++++++++++++++++++++++++++++++-- 2 files changed, 76 insertions(+), 7 deletions(-) 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",