diff --git a/pkg/notifications/mail_render.go b/pkg/notifications/mail_render.go index 292927c80..7749e1d9c 100644 --- a/pkg/notifications/mail_render.go +++ b/pkg/notifications/mail_render.go @@ -20,6 +20,7 @@ import ( "bytes" "embed" templatehtml "html/template" + "net/url" "regexp" "strings" templatetext "text/template" @@ -187,16 +188,26 @@ const mailTemplateConversationalHTML = ` //go:embed logo.png var logo embed.FS -func convertLinesToHTML(lines []*mailLine) (linesHTML []templatehtml.HTML, err error) { +// newNotificationSanitizer builds the bluemonday policy for all HTML in notification +// emails. Only inline data-URI images (avatars) are allowed: RewriteSrc blanks any +// remote image src so a user-controlled task title, comment or description can't +// smuggle a tracking pixel into a recipient's inbox. +func newNotificationSanitizer() *bluemonday.Policy { p := bluemonday.UGCPolicy() - // Allow data URI images for inline avatars in mentions p.AllowDataURIImages() - // Allow style attribute on img and div elements for avatar and layout styling p.AllowAttrs("style").OnElements("img", "div") - // Allow specific CSS properties for avatar styling p.AllowStyles("border-radius", "vertical-align", "margin-right").OnElements("img") - // Allow padding styles on div elements for content spacing p.AllowStyles("padding-top", "margin-bottom").OnElements("div") + p.RewriteSrc(func(u *url.URL) { + if u.Scheme != "data" { + *u = url.URL{} + } + }) + return p +} + +func convertLinesToHTML(lines []*mailLine) (linesHTML []templatehtml.HTML, err error) { + p := newNotificationSanitizer() for _, line := range lines { if line.isHTML { @@ -225,11 +236,7 @@ func convertLinesToHTML(lines []*mailLine) (linesHTML []templatehtml.HTML, err e // sanitizeLinesToHTML sanitizes lines without wrapping in
tags or adding margins.
// Used for footer lines and other content that should not have paragraph styling.
func sanitizeLinesToHTML(lines []*mailLine) (linesHTML []templatehtml.HTML, err error) {
- p := bluemonday.UGCPolicy()
- p.AllowDataURIImages()
- p.AllowAttrs("style").OnElements("img", "div")
- p.AllowStyles("border-radius", "vertical-align", "margin-right").OnElements("img")
- p.AllowStyles("padding-top", "margin-bottom").OnElements("div")
+ p := newNotificationSanitizer()
for _, line := range lines {
if line.isHTML {
@@ -366,12 +373,8 @@ func RenderMail(m *Mail, lang string) (mailOpts *mail.Opts, err error) {
data["CopyURLText"] = i18n.T(lang, "notifications.common.copy_url")
if m.headerLine != nil {
- p := bluemonday.UGCPolicy()
- p.AllowDataURIImages()
- p.AllowAttrs("style").OnElements("img", "div")
- p.AllowStyles("border-radius", "vertical-align", "margin-right").OnElements("img")
// #nosec G203 -- the html is sanitized
- data["HeaderLineHTML"] = templatehtml.HTML(p.Sanitize(m.headerLine.Text))
+ data["HeaderLineHTML"] = templatehtml.HTML(newNotificationSanitizer().Sanitize(m.headerLine.Text))
}
data["IntroLinesHTML"], err = convertLinesToHTML(m.introLines)
diff --git a/pkg/notifications/mail_test.go b/pkg/notifications/mail_test.go
index fca5c6447..d8f5db2e4 100644
--- a/pkg/notifications/mail_test.go
+++ b/pkg/notifications/mail_test.go
@@ -711,3 +711,55 @@ func TestConversationalMail(t *testing.T) {
assert.Contains(t, headerLine1, "(Project > Task) #1")
})
}
+
+// Regression test for GHSA-2vr2-r3qw-rjvq: a user-controlled task title (or comment
+// or description) must not be able to smuggle a remote image into a notification
+// email, where it would act as a tracking pixel. Inline data-URI avatars and normal
+// links must keep working.
+func TestNotificationEmailStripsRemoteImages(t *testing.T) {
+ const remoteSrc = "https://attacker.example/track.png?u=victim"
+
+ t.Run("remote image injected via task title in header is stripped", func(t *testing.T) {
+ payloadTitle := ` hinormal title`
+ header := CreateConversationalHeader("", "attacker left a comment", "https://example.com/task/1", "Project", "#1", payloadTitle)
+
+ mailOpts, err := RenderMail(NewMail().
+ Conversational().
+ Subject("Test").
+ HeaderLine(header).
+ Action("View Task", "https://example.com/task/1"), "en")
+ require.NoError(t, err)
+
+ assert.NotContains(t, mailOpts.HTMLMessage, remoteSrc)
+ assert.NotContains(t, mailOpts.HTMLMessage, "attacker.example")
+ // The benign text is still delivered, and the legitimate task link survives.
+ assert.Contains(t, mailOpts.HTMLMessage, "normal title")
+ assert.Contains(t, mailOpts.HTMLMessage, `href="https://example.com/task/1"`)
+ })
+
+ t.Run("remote image in body content is stripped", func(t *testing.T) {
+ mailOpts, err := RenderMail(NewMail().
+ Conversational().
+ Subject("Test").
+ HTML(`
`).
+ Action("View Task", "https://example.com/task/1"), "en")
+ require.NoError(t, err)
+
+ assert.NotContains(t, mailOpts.HTMLMessage, remoteSrc)
+ assert.Contains(t, mailOpts.HTMLMessage, "hi")
+ })
+
+ t.Run("inline data-URI avatar is preserved", func(t *testing.T) {
+ const avatar = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNk+M9QDwADhgGAWjR9awAAAABJRU5ErkJggg=="
+ header := CreateConversationalHeader(avatar, "alice left a comment", "https://example.com/task/1", "Project", "#1", "Task")
+
+ mailOpts, err := RenderMail(NewMail().
+ Conversational().
+ Subject("Test").
+ HeaderLine(header).
+ Action("View Task", "https://example.com/task/1"), "en")
+ require.NoError(t, err)
+
+ assert.Contains(t, mailOpts.HTMLMessage, "data:image/png;base64,")
+ })
+}