Skip to content

Commit e7b7a29

Browse files
authored
fix: escape task and language names in notification emails (#3842)
## Context Triaging an external report claiming stored HTML injection in the signup verification email. That specific claim does not hold — the verification email contains no user-supplied name, and every template variable already goes through `#strings.escapeXml` (covered by `EmailServiceTest.it is not vulnerable to injection`). Reviewing the surrounding code did turn up a genuine unescaped sink, which this PR fixes. ## The issue `TaskEmailComposer` builds the notification email body as raw HTML and interpolates two user-supplied values without escaping: - `task.name` - `task.language.name` That string is passed to `EmailParams.text`, which lands in the `default` template's `content` variable — the one variable deliberately injected as raw HTML (`dangerouslyInjectValueAsHtmlWithoutSanitization`). So a project member who names a task `<h1><a href="https://evil.example">…</a></h1>` gets that markup rendered in the task notification email received by everyone else on the project. Severity is low: it requires project access, reaches only project members, and email clients do not execute script. It is the same class as the invitation-name bug fixed in #1898 — cosmetic markup injection into a transactional email, useful only for making phishing content look native. ## The fix Escape both values with `HtmlUtils.htmlEscape` at the interpolation site, matching what `InvitationEmailSender` already does. `taskName()` stays plain text — it is also called from the billing repo, which escapes at its own interpolation site (see tolgee/billing PR). The two changes are independent; either can merge first. ## Tests New `TaskEmailComposerTest` asserts both values come out escaped. Verified it fails against the unpatched composer and passes with the fix. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved task-assignment email security by safely escaping task and language names in email links. * Prevented special characters in names from being interpreted as HTML. * **Tests** * Added coverage to verify correct escaping in task notification emails. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent 10d26c8 commit e7b7a29

2 files changed

Lines changed: 51 additions & 1 deletion

File tree

backend/data/src/main/kotlin/io/tolgee/service/notification/TaskEmailComposer.kt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import io.tolgee.model.notifications.Notification
55
import io.tolgee.model.task.Task
66
import io.tolgee.util.I18n
77
import org.springframework.stereotype.Component
8+
import org.springframework.web.util.HtmlUtils
89

910
@Component
1011
class TaskEmailComposer(
@@ -33,7 +34,7 @@ class TaskEmailComposer(
3334
private fun taskLink(task: Task): String {
3435
return """
3536
|<a href="${taskUrl(task)}">
36-
| ${taskName(task.name)} #${task.number} (${task.language.name})
37+
| ${HtmlUtils.htmlEscape(taskName(task.name))} #${task.number} (${HtmlUtils.htmlEscape(task.language.name)})
3738
|</a>
3839
""".trimMargin()
3940
}
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
package io.tolgee.unit
2+
3+
import io.tolgee.component.FrontendUrlProvider
4+
import io.tolgee.development.testDataBuilder.data.TaskTestData
5+
import io.tolgee.model.notifications.Notification
6+
import io.tolgee.service.notification.TaskEmailComposer
7+
import io.tolgee.testing.assert
8+
import io.tolgee.util.I18n
9+
import org.junit.jupiter.api.Test
10+
import org.mockito.kotlin.any
11+
import org.mockito.kotlin.doReturn
12+
import org.mockito.kotlin.mock
13+
14+
class TaskEmailComposerTest {
15+
private val frontendUrlProvider =
16+
mock<FrontendUrlProvider> {
17+
on { getTaskUrl(any(), any()) } doReturn "https://app.tolgee.io/task"
18+
on { getMyTasksUrl() } doReturn "https://app.tolgee.io/my-tasks"
19+
}
20+
21+
private val composer = TaskEmailComposer(frontendUrlProvider, I18n())
22+
23+
@Test
24+
fun `escapes task name`() {
25+
val email = composer.composeEmail(notification(taskName = "<h1>pwned</h1>"))
26+
email.assert.doesNotContain("<h1>pwned</h1>")
27+
email.assert.contains("&lt;h1&gt;pwned&lt;/h1&gt;")
28+
}
29+
30+
@Test
31+
fun `escapes language name`() {
32+
val email = composer.composeEmail(notification(languageName = """<a href="https://evil.example">English</a>"""))
33+
email.assert.doesNotContain("""<a href="https://evil.example">""")
34+
email.assert.contains("&lt;a href=&quot;https://evil.example&quot;&gt;")
35+
}
36+
37+
private fun notification(
38+
taskName: String = "Translate",
39+
languageName: String = "English",
40+
): Notification {
41+
val testData = TaskTestData()
42+
testData.addNotifications()
43+
testData.translateTask.self.apply {
44+
name = taskName
45+
language.name = languageName
46+
}
47+
return testData.taskNotification.self
48+
}
49+
}

0 commit comments

Comments
 (0)