Implement a SafeTextModel and use that for fields vulnerable to XSS

5 files changed, 112 insertions, 11 deletions

diff --git a/src/main/java/com/gitblit/wicket/SafeTextModel.java b/src/main/java/com/gitblit/wicket/SafeTextModel.java
new file mode 100644
index 00000000..aef7e97a
--- /dev/null
+++ b/src/main/java/com/gitblit/wicket/SafeTextModel.java
@@ -0,0 +1,96 @@
+package com.gitblit.wicket;
+
+import org.apache.wicket.model.IModel;
+import org.apache.wicket.model.Model;
+import org.apache.wicket.util.lang.Objects;
+import org.parboiled.common.StringUtils;
+import org.slf4j.LoggerFactory;
+
+public class SafeTextModel implements IModel<String> {
+
+	private static final long serialVersionUID = 1L;
+
+	public enum Mode {
+		relaxed, none
+	}
+
+	private final Mode mode;
+
+	private String value;
+
+	public static SafeTextModel none() {
+		return new SafeTextModel(Mode.none);
+	}
+
+	public static SafeTextModel none(String value) {
+		return new SafeTextModel(Mode.none);
+	}
+
+	public static SafeTextModel relaxed() {
+		return new SafeTextModel(Mode.relaxed);
+	}
+
+	public static SafeTextModel relaxed(String value) {
+		return new SafeTextModel(Mode.relaxed);
+	}
+
+	public SafeTextModel(Mode mode) {
+		this.mode = mode;
+	}
+
+	public SafeTextModel(String value, Mode mode) {
+		this.value = value;
+		this.mode = mode;
+	}
+
+	@Override
+	public void detach() {
+	}
+
+	@Override
+	public String getObject() {
+		if (StringUtils.isEmpty(value)) {
+			return value;
+		}
+		String safeValue;
+		switch (mode) {
+		case none:
+			safeValue = GitBlitWebApp.get().xssFilter().none(value);
+			break;
+		default:
+			safeValue = GitBlitWebApp.get().xssFilter().relaxed(value);
+			break;
+		}
+		if (!value.equals(safeValue)) {
+			LoggerFactory.getLogger(getClass()).warn("XSS filter trigggered on suspicious form field value {}",
+					value);
+		}
+		return safeValue;
+	}
+
+	@Override
+	public void setObject(String input) {
+		this.value = input;
+	}
+
+	@Override
+	public int hashCode()
+	{
+		return Objects.hashCode(value);
+	}
+
+	@Override
+	public boolean equals(Object obj)
+	{
+		if (this == obj)
+		{
+			return true;
+		}
+		if (!(obj instanceof Model<?>))
+		{
+			return false;
+		}
+		Model<?> that = (Model<?>)obj;
+		return Objects.equal(value, that.getObject());
+	}
+}
diff --git a/src/main/java/com/gitblit/wicket/pages/EditTicketPage.java b/src/main/java/com/gitblit/wicket/pages/EditTicketPage.java
index 4a06e59e..bd2ec63c 100644
--- a/src/main/java/com/gitblit/wicket/pages/EditTicketPage.java
+++ b/src/main/java/com/gitblit/wicket/pages/EditTicketPage.java
@@ -50,6 +50,8 @@ import com.gitblit.tickets.TicketNotifier;
 import com.gitblit.tickets.TicketResponsible;

 import com.gitblit.utils.StringUtils;

 import com.gitblit.wicket.GitBlitWebSession;

+import com.gitblit.wicket.SafeTextModel;

+import com.gitblit.wicket.SafeTextModel.Mode;

 import com.gitblit.wicket.WicketUtils;

 import com.gitblit.wicket.panels.MarkdownTextArea;

 

@@ -110,8 +112,8 @@ public class EditTicketPage extends RepositoryPage {
 		}

 

 		typeModel = Model.of(ticket.type);

-		titleModel = Model.of(ticket.title);

-		topicModel = Model.of(ticket.topic == null ? "" : ticket.topic);

+		titleModel = SafeTextModel.none(ticket.title);

+		topicModel = SafeTextModel.none(ticket.topic == null ? "" : ticket.topic);

 		responsibleModel = Model.of();

 		milestoneModel = Model.of();

 		mergeToModel = Model.of(ticket.mergeTo == null ? getRepositoryModel().mergeTo : ticket.mergeTo);

@@ -134,7 +136,7 @@ public class EditTicketPage extends RepositoryPage {
 		form.add(new TextField<String>("title", titleModel));

 		form.add(new TextField<String>("topic", topicModel));

 

-		final IModel<String> markdownPreviewModel = new Model<String>();

+		final SafeTextModel markdownPreviewModel = new SafeTextModel(Mode.none);

 		descriptionPreview = new Label("descriptionPreview", markdownPreviewModel);

 		descriptionPreview.setEscapeModelStrings(false);

 		descriptionPreview.setOutputMarkupId(true);

diff --git a/src/main/java/com/gitblit/wicket/pages/NewTicketPage.java b/src/main/java/com/gitblit/wicket/pages/NewTicketPage.java
index 961590a2..7a68febb 100644
--- a/src/main/java/com/gitblit/wicket/pages/NewTicketPage.java
+++ b/src/main/java/com/gitblit/wicket/pages/NewTicketPage.java
@@ -46,6 +46,8 @@ import com.gitblit.tickets.TicketNotifier;
 import com.gitblit.tickets.TicketResponsible;

 import com.gitblit.utils.StringUtils;

 import com.gitblit.wicket.GitBlitWebSession;

+import com.gitblit.wicket.SafeTextModel;

+import com.gitblit.wicket.SafeTextModel.Mode;

 import com.gitblit.wicket.WicketUtils;

 import com.gitblit.wicket.panels.MarkdownTextArea;

 

@@ -87,8 +89,8 @@ public class NewTicketPage extends RepositoryPage {
 		}

 

 		typeModel = Model.of(TicketModel.Type.defaultType);

-		titleModel = Model.of();

-		topicModel = Model.of();

+		titleModel = SafeTextModel.none();

+		topicModel = SafeTextModel.none();

 		mergeToModel = Model.of(Repository.shortenRefName(getRepositoryModel().mergeTo));

 		responsibleModel = Model.of();

 		milestoneModel = Model.of();

@@ -103,7 +105,7 @@ public class NewTicketPage extends RepositoryPage {
 		form.add(new TextField<String>("title", titleModel));

 		form.add(new TextField<String>("topic", topicModel));

 

-		final IModel<String> markdownPreviewModel = new Model<String>();

+		final SafeTextModel markdownPreviewModel = new SafeTextModel(Mode.none);

 		descriptionPreview = new Label("descriptionPreview", markdownPreviewModel);

 		descriptionPreview.setEscapeModelStrings(false);

 		descriptionPreview.setOutputMarkupId(true);

diff --git a/src/main/java/com/gitblit/wicket/panels/CommentPanel.java b/src/main/java/com/gitblit/wicket/panels/CommentPanel.java
index 1d49ff0f..130e7336 100644
--- a/src/main/java/com/gitblit/wicket/panels/CommentPanel.java
+++ b/src/main/java/com/gitblit/wicket/panels/CommentPanel.java
@@ -19,13 +19,14 @@ import org.apache.wicket.ajax.AjaxRequestTarget;
 import org.apache.wicket.ajax.markup.html.form.AjaxButton;
 import org.apache.wicket.markup.html.basic.Label;
 import org.apache.wicket.markup.html.form.Form;
-import org.apache.wicket.model.IModel;
 import org.apache.wicket.model.Model;
 
 import com.gitblit.models.RepositoryModel;
 import com.gitblit.models.TicketModel;
 import com.gitblit.models.TicketModel.Change;
 import com.gitblit.models.UserModel;
+import com.gitblit.wicket.SafeTextModel;
+import com.gitblit.wicket.SafeTextModel.Mode;
 import com.gitblit.wicket.WicketUtils;
 import com.gitblit.wicket.pages.BasePage;
 
@@ -89,7 +90,7 @@ public class CommentPanel extends BasePanel {
 			}
 		}.setVisible(ticket != null && ticket.number > 0));
 
-		final IModel<String> markdownPreviewModel = new Model<String>();
+		final SafeTextModel markdownPreviewModel = new SafeTextModel(Mode.none);
 		markdownPreview = new Label("markdownPreview", markdownPreviewModel);
 		markdownPreview.setEscapeModelStrings(false);
 		markdownPreview.setOutputMarkupId(true);
diff --git a/src/main/java/com/gitblit/wicket/panels/MarkdownTextArea.java b/src/main/java/com/gitblit/wicket/panels/MarkdownTextArea.java
index f26f7fbe..6e06e5bb 100644
--- a/src/main/java/com/gitblit/wicket/panels/MarkdownTextArea.java
+++ b/src/main/java/com/gitblit/wicket/panels/MarkdownTextArea.java
@@ -20,12 +20,12 @@ import org.apache.wicket.ajax.AjaxRequestTarget;
 import org.apache.wicket.ajax.form.AjaxFormComponentUpdatingBehavior;
 import org.apache.wicket.markup.html.basic.Label;
 import org.apache.wicket.markup.html.form.TextArea;
-import org.apache.wicket.model.IModel;
 import org.apache.wicket.model.PropertyModel;
 import org.apache.wicket.util.time.Duration;
 
 import com.gitblit.utils.MarkdownUtils;
 import com.gitblit.wicket.GitBlitWebApp;
+import com.gitblit.wicket.SafeTextModel;
 
 public class MarkdownTextArea extends TextArea {
 
@@ -35,7 +35,7 @@ public class MarkdownTextArea extends TextArea {
 
 	protected String text = "";
 
-	public MarkdownTextArea(String id, final IModel<String> previewModel, final Label previewLabel) {
+	public MarkdownTextArea(String id, final SafeTextModel previewModel, final Label previewLabel) {
 		super(id);
 		setModel(new PropertyModel(this, "text"));
 		add(new AjaxFormComponentUpdatingBehavior("onblur") {
@@ -65,7 +65,7 @@ public class MarkdownTextArea extends TextArea {
 		setOutputMarkupId(true);
 	}
 
-	protected void renderPreview(IModel<String> previewModel) {
+	protected void renderPreview(SafeTextModel previewModel) {
 		if (text == null) {
 			return;
 		}