From 209dbdd49a89d6e3cebf61e860c779a1d8561dd9 Mon Sep 17 00:00:00 2001
From: James Moger <james.moger@gitblit.com>
Date: Sun, 07 Sep 2014 11:43:40 -0400
Subject: [PATCH] Implement a SafeTextModel and use that for fields vulnerable to XSS
---
src/main/java/com/gitblit/wicket/pages/EditTicketPage.java | 8 +-
src/main/java/com/gitblit/wicket/panels/CommentPanel.java | 5 +
src/main/java/com/gitblit/wicket/panels/MarkdownTextArea.java | 6 +-
src/main/java/com/gitblit/wicket/pages/NewTicketPage.java | 8 +-
src/main/java/com/gitblit/wicket/SafeTextModel.java | 96 ++++++++++++++++++++++++++++++++
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 0000000..aef7e97
--- /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 4a06e59..bd2ec63 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.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 @@
}
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 @@
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 961590a..7a68feb 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.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 @@
}
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 @@
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 1d49ff0..130e733 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.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 @@
}
}.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 f26f7fb..6e06e5b 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.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 @@
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 @@
setOutputMarkupId(true);
}
- protected void renderPreview(IModel<String> previewModel) {
+ protected void renderPreview(SafeTextModel previewModel) {
if (text == null) {
return;
}
--
Gitblit v1.9.1