Skip to content

Commit

Permalink
[SECURITY-906]
Browse files Browse the repository at this point in the history
  • Loading branch information
bakito authored and daniel-beck committed Jun 18, 2018
1 parent 0d6308f commit 63a7744
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 5 deletions.
5 changes: 5 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,11 @@
<version>1.39</version>
</dependency>

<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>antisamy-markup-formatter</artifactId>
<version>1.5</version>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins.workflow</groupId>
<artifactId>workflow-cps</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,21 @@
*/
package com.jenkinsci.plugins.badge.action;

import hudson.markup.RawHtmlMarkupFormatter;
import org.apache.commons.lang.StringEscapeUtils;
import org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists.Whitelisted;
import org.kohsuke.stapler.export.Exported;
import org.kohsuke.stapler.export.ExportedBean;

import java.io.IOException;
import java.util.logging.Level;
import java.util.logging.Logger;


@ExportedBean(defaultVisibility = 2)
public class BadgeSummaryAction extends AbstractAction {
private static final long serialVersionUID = 1L;
private static final Logger LOGGER = Logger.getLogger(BadgeSummaryAction.class.getName());

private final String iconPath;
private String summaryText = "";
Expand All @@ -57,9 +64,18 @@ public String getIconPath() {
return iconPath;
}

public String getRawText() {
return summaryText;
}

@Exported
public String getText() {
return summaryText;
try {
return new RawHtmlMarkupFormatter(false).translate(summaryText);
} catch (IOException e) {
LOGGER.log(Level.WARNING, "Error preparing summary text for ui", e);
return "<b><font color=\"red\">ERROR</font></b>";
}
}

@Whitelisted
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,19 @@
*/
package com.jenkinsci.plugins.badge.action;

import hudson.markup.RawHtmlMarkupFormatter;
import org.kohsuke.stapler.export.Exported;
import org.kohsuke.stapler.export.ExportedBean;

import java.io.IOException;
import java.util.logging.Level;
import java.util.logging.Logger;

@ExportedBean(defaultVisibility = 2)
public class HtmlBadgeAction extends AbstractBadgeAction {
private static final long serialVersionUID = 1L;
private static final Logger LOGGER = Logger.getLogger(BadgeSummaryAction.class.getName());

private final String html;

private HtmlBadgeAction(String html) {
Expand All @@ -52,9 +59,17 @@ public String getIconFileName() {
return null;
}

@Exported
public String getHtml() {
public String getRawHtml() {
return html;
}

@Exported
public String getHtml() {
try {
return new RawHtmlMarkupFormatter(false).translate(html);
} catch (IOException e) {
LOGGER.log(Level.WARNING, "Error preparing html content for ui", e);
return "<b><font color=\"red\">ERROR</font></b>";
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,18 @@ public class AddHtmlBadgeStepTest extends AbstractBadgeTest {
@Test
public void addHtmlBadge() throws Exception {
String html = UUID.randomUUID().toString();
testAddHtmlBadge(html, html);
}

@Test
public void addHtmlBadge_remove_script() throws Exception {
String uuid = UUID.randomUUID().toString();
String html = uuid + "<script>alert('exploit!');</script>";
testAddHtmlBadge(html, uuid);
}


private void testAddHtmlBadge(String html, String expected) throws Exception {
WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "p");

String script = "addHtmlBadge(\"" + html + "\")";
Expand All @@ -51,6 +63,7 @@ public void addHtmlBadge() throws Exception {
assertEquals(1, badgeActions.size());

HtmlBadgeAction action = (HtmlBadgeAction) badgeActions.get(0);
assertEquals(html, action.getHtml());
assertEquals(expected, action.getHtml());
assertEquals(html, action.getRawHtml());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,15 @@ public void createSummary_html_unescaped() throws Exception {
assertEquals("<li>" + text + "</li>", action.getText());
}

@Test
public void createSummary_html_unescaped_remove_script() throws Exception {
String text = randomUUID().toString();
String html = "<li>" + text + "</li><script>alert(\"exploit!\");</script>";
BadgeSummaryAction action = createSummary("summary.appendText('" + html + "', false);");
assertEquals("<li>" + text + "</li>", action.getText());
assertEquals(html, action.getRawText());
}

@Test
public void createSummary_html_escaped() throws Exception {
String text = randomUUID().toString();
Expand Down Expand Up @@ -85,7 +94,7 @@ public void createSummary_with_text() throws Exception {
String text = randomUUID().toString();

WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "p");
p.setDefinition(new CpsFlowDefinition("def summary = createSummary(icon:\"" + icon + "\", text:\""+text+"\")", true));
p.setDefinition(new CpsFlowDefinition("def summary = createSummary(icon:\"" + icon + "\", text:\"" + text + "\")", true));
WorkflowRun b = r.assertBuildStatusSuccess(p.scheduleBuild2(0));
List<BadgeSummaryAction> summaryActions = b.getActions(BadgeSummaryAction.class);
assertEquals(1, summaryActions.size());
Expand Down

0 comments on commit 63a7744

Please sign in to comment.