Fix SSF-241

diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/issue/notification/IssueChangesEmailTemplate.java b/server/sonar-server-common/src/main/java/org/sonar/server/issue/notification/IssueChangesEmailTemplate.java
index 14beb1e818949a3517a7dbba882d491488f22e9c..c8f193e32f03465fc64ab1a7afb5ac56dd168c52 100644 (file)
--- a/server/sonar-server-common/src/main/java/org/sonar/server/issue/notification/IssueChangesEmailTemplate.java
+++ b/server/sonar-server-common/src/main/java/org/sonar/server/issue/notification/IssueChangesEmailTemplate.java
@@ -46,6 +46,7 @@ import org.sonar.server.issue.notification.IssuesChangesNotificationBuilder.Rule
 import static java.net.URLEncoder.encode;
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static java.util.function.Function.identity;
+import static org.apache.commons.lang.StringEscapeUtils.escapeHtml;
 import static org.sonar.core.util.stream.MoreCollectors.index;
 import static org.sonar.server.issue.notification.RuleGroup.ISSUES;
 import static org.sonar.server.issue.notification.RuleGroup.SECURITY_HOTSPOTS;
@@ -85,11 +86,11 @@ public abstract class IssueChangesEmailTemplate implements EmailTemplate {
    */
   protected static void toString(StringBuilder sb, Project project) {
     Optional<String> branchName = project.getBranchName();
+    String value = project.getProjectName();
     if (branchName.isPresent()) {
-      sb.append(project.getProjectName()).append(", ").append(branchName.get());
-    } else {
-      sb.append(project.getProjectName());
+      value += ", " + branchName.get();
     }
+    sb.append(escapeHtml(value));
   }
 
   static String toUrlParams(Project project) {
@@ -155,7 +156,8 @@ public abstract class IssueChangesEmailTemplate implements EmailTemplate {
       Rule rule = rules.next();
       Collection<ChangedIssue> issues = issuesByRule.get(rule);
 
-      sb.append("<li>").append("Rule ").append(" <em>").append(rule.getName()).append("</em> - ");
+      String ruleName = escapeHtml(rule.getName());
+      sb.append("<li>").append("Rule ").append(" <em>").append(ruleName).append("</em> - ");
       appendIssueLinks(sb, issuePageHref, issues, rule.getRuleType());
       sb.append("</li>");
     }

diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/issue/notification/ChangesOnMyIssuesEmailTemplateTest.java b/server/sonar-server-common/src/test/java/org/sonar/server/issue/notification/ChangesOnMyIssuesEmailTemplateTest.java
index 66250436ce2aef09db2631d2917d51daa8117d49..3b5e20f49c85ebeb22c634421182c243fe001218 100644 (file)
--- a/server/sonar-server-common/src/test/java/org/sonar/server/issue/notification/ChangesOnMyIssuesEmailTemplateTest.java
+++ b/server/sonar-server-common/src/test/java/org/sonar/server/issue/notification/ChangesOnMyIssuesEmailTemplateTest.java
@@ -53,6 +53,7 @@ import static java.util.stream.Collectors.joining;
 import static java.util.stream.Collectors.toList;
 import static java.util.stream.Collectors.toSet;
 import static org.apache.commons.lang.RandomStringUtils.randomAlphabetic;
+import static org.apache.commons.lang.StringEscapeUtils.escapeHtml;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.mockito.Mockito.mock;
@@ -409,6 +410,38 @@ public class ChangesOnMyIssuesEmailTemplateTest {
       .noMoreBlock();
   }
 
+  @Test
+  public void user_input_content_should_be_html_escape() {
+    Project project = new Project.Builder("uuid").setProjectName("</projectName>").setKey("project_key").build();
+    String ruleName = "</RandomRule>";
+    String host = randomAlphabetic(15);
+    Rule rule = newRule(ruleName, randomRuleTypeHotspotExcluded());
+    List<ChangedIssue> changedIssues = IntStream.range(0, 2 + new Random().nextInt(5))
+      .mapToObj(i -> newChangedIssue("issue_" + i, randomValidStatus(), project, rule))
+      .collect(toList());
+    UserChange userChange = newUserChange();
+    when(emailSettings.getServerBaseURL()).thenReturn(host);
+
+    EmailMessage emailMessage = underTest.format(new ChangesOnMyIssuesNotification(userChange, ImmutableSet.copyOf(changedIssues)));
+
+    assertThat(emailMessage.getMessage())
+      .doesNotContain(project.getProjectName())
+      .contains(escapeHtml(project.getProjectName()))
+      .doesNotContain(ruleName)
+      .contains(escapeHtml(ruleName));
+
+    String expectedHref = host + "/project/issues?id=" + project.getKey()
+      + "&issues=" + changedIssues.stream().map(ChangedIssue::getKey).collect(joining("%2C"));
+    String expectedLinkText = "See all " + changedIssues.size() + " issues";
+    HtmlFragmentAssert.assertThat(emailMessage.getMessage())
+      .hasParagraph().hasParagraph() // skip header
+      .hasParagraph(project.getProjectName())
+      .hasList("Rule " + ruleName + " - " + expectedLinkText)
+      .withLink(expectedLinkText, expectedHref)
+      .hasParagraph().hasParagraph() // skip footer
+      .noMoreBlock();
+  }
+
   @Test
   public void formats_returns_html_message_for_single_issue_on_master_when_user_change() {
     Project project = newProject("1");