]> source.dussan.org Git - gitblit.git/commitdiff
Implement a SafeTextModel and use that for fields vulnerable to XSS
authorJames Moger <james.moger@gitblit.com>
Sat, 6 Sep 2014 17:14:38 +0000 (13:14 -0400)
committerJames Moger <james.moger@gitblit.com>
Sun, 7 Sep 2014 15:43:40 +0000 (11:43 -0400)
src/main/java/com/gitblit/wicket/SafeTextModel.java [new file with mode: 0644]
src/main/java/com/gitblit/wicket/pages/EditTicketPage.java
src/main/java/com/gitblit/wicket/pages/NewTicketPage.java
src/main/java/com/gitblit/wicket/panels/CommentPanel.java
src/main/java/com/gitblit/wicket/panels/MarkdownTextArea.java

diff --git a/src/main/java/com/gitblit/wicket/SafeTextModel.java b/src/main/java/com/gitblit/wicket/SafeTextModel.java
new file mode 100644 (file)
index 0000000..aef7e97
--- /dev/null
@@ -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());
+       }
+}
index 4a06e59e9db1e71d5b21760cd046934efeaf8ecc..bd2ec63c740ac99504390dda326e8f7a3231224f 100644 (file)
@@ -50,6 +50,8 @@ import com.gitblit.tickets.TicketNotifier;
 import com.gitblit.tickets.TicketResponsible;\r
 import com.gitblit.utils.StringUtils;\r
 import com.gitblit.wicket.GitBlitWebSession;\r
+import com.gitblit.wicket.SafeTextModel;\r
+import com.gitblit.wicket.SafeTextModel.Mode;\r
 import com.gitblit.wicket.WicketUtils;\r
 import com.gitblit.wicket.panels.MarkdownTextArea;\r
 \r
@@ -110,8 +112,8 @@ public class EditTicketPage extends RepositoryPage {
                }\r
 \r
                typeModel = Model.of(ticket.type);\r
-               titleModel = Model.of(ticket.title);\r
-               topicModel = Model.of(ticket.topic == null ? "" : ticket.topic);\r
+               titleModel = SafeTextModel.none(ticket.title);\r
+               topicModel = SafeTextModel.none(ticket.topic == null ? "" : ticket.topic);\r
                responsibleModel = Model.of();\r
                milestoneModel = Model.of();\r
                mergeToModel = Model.of(ticket.mergeTo == null ? getRepositoryModel().mergeTo : ticket.mergeTo);\r
@@ -134,7 +136,7 @@ public class EditTicketPage extends RepositoryPage {
                form.add(new TextField<String>("title", titleModel));\r
                form.add(new TextField<String>("topic", topicModel));\r
 \r
-               final IModel<String> markdownPreviewModel = new Model<String>();\r
+               final SafeTextModel markdownPreviewModel = new SafeTextModel(Mode.none);\r
                descriptionPreview = new Label("descriptionPreview", markdownPreviewModel);\r
                descriptionPreview.setEscapeModelStrings(false);\r
                descriptionPreview.setOutputMarkupId(true);\r
index 961590a298a14e8730e1ffb6c3fdfb15ec890f9b..7a68febb5d78ad7cec2e851804fe4263bc1e4756 100644 (file)
@@ -46,6 +46,8 @@ import com.gitblit.tickets.TicketNotifier;
 import com.gitblit.tickets.TicketResponsible;\r
 import com.gitblit.utils.StringUtils;\r
 import com.gitblit.wicket.GitBlitWebSession;\r
+import com.gitblit.wicket.SafeTextModel;\r
+import com.gitblit.wicket.SafeTextModel.Mode;\r
 import com.gitblit.wicket.WicketUtils;\r
 import com.gitblit.wicket.panels.MarkdownTextArea;\r
 \r
@@ -87,8 +89,8 @@ public class NewTicketPage extends RepositoryPage {
                }\r
 \r
                typeModel = Model.of(TicketModel.Type.defaultType);\r
-               titleModel = Model.of();\r
-               topicModel = Model.of();\r
+               titleModel = SafeTextModel.none();\r
+               topicModel = SafeTextModel.none();\r
                mergeToModel = Model.of(Repository.shortenRefName(getRepositoryModel().mergeTo));\r
                responsibleModel = Model.of();\r
                milestoneModel = Model.of();\r
@@ -103,7 +105,7 @@ public class NewTicketPage extends RepositoryPage {
                form.add(new TextField<String>("title", titleModel));\r
                form.add(new TextField<String>("topic", topicModel));\r
 \r
-               final IModel<String> markdownPreviewModel = new Model<String>();\r
+               final SafeTextModel markdownPreviewModel = new SafeTextModel(Mode.none);\r
                descriptionPreview = new Label("descriptionPreview", markdownPreviewModel);\r
                descriptionPreview.setEscapeModelStrings(false);\r
                descriptionPreview.setOutputMarkupId(true);\r
index 1d49ff0fc3c944d9e7945e08d829c46c2c5fbf04..130e7336127d1c369b6982df122caf5b01fc0066 100644 (file)
@@ -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);
index f26f7fbe6616f76df4cdc4a0e15c8ccbc4c241ad..6e06e5bbcd9f6ec196ab958c5313712eea1c5b50 100644 (file)
@@ -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;
                }