From 209dbdd49a89d6e3cebf61e860c779a1d8561dd9 Mon Sep 17 00:00:00 2001 From: James Moger Date: Sat, 6 Sep 2014 13:14:38 -0400 Subject: [PATCH] Implement a SafeTextModel and use that for fields vulnerable to XSS --- .../com/gitblit/wicket/SafeTextModel.java | 96 +++++++++++++++++++ .../gitblit/wicket/pages/EditTicketPage.java | 8 +- .../gitblit/wicket/pages/NewTicketPage.java | 8 +- .../gitblit/wicket/panels/CommentPanel.java | 5 +- .../wicket/panels/MarkdownTextArea.java | 6 +- 5 files changed, 112 insertions(+), 11 deletions(-) create mode 100644 src/main/java/com/gitblit/wicket/SafeTextModel.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 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 { + + 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("title", titleModel)); form.add(new TextField("topic", topicModel)); - final IModel markdownPreviewModel = new Model(); + 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("title", titleModel)); form.add(new TextField("topic", topicModel)); - final IModel markdownPreviewModel = new Model(); + 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 markdownPreviewModel = new Model(); + 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 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 previewModel) { + protected void renderPreview(SafeTextModel previewModel) { if (text == null) { return; } -- 2.39.5