From a5962734a421184fef072d805424e15518633973 Mon Sep 17 00:00:00 2001 From: James Moger Date: Wed, 17 Sep 2014 13:58:38 -0400 Subject: [PATCH] Sanitize ticket text at presentation time to avoid unintended html encoding --- src/main/java/com/gitblit/utils/JSoupXssFilter.java | 10 ++++++---- .../com/gitblit/wicket/pages/EditTicketPage.java | 8 +++----- .../com/gitblit/wicket/pages/NewTicketPage.java | 8 +++----- .../java/com/gitblit/wicket/pages/TicketPage.java | 13 +++++++++---- .../com/gitblit/wicket/panels/CommentPanel.java | 5 ++--- .../com/gitblit/wicket/panels/MarkdownTextArea.java | 9 +++++---- .../com/gitblit/wicket/panels/TicketListPanel.java | 3 ++- 7 files changed, 30 insertions(+), 26 deletions(-) diff --git a/src/main/java/com/gitblit/utils/JSoupXssFilter.java b/src/main/java/com/gitblit/utils/JSoupXssFilter.java index b07bcb9d..7fa7b2a1 100644 --- a/src/main/java/com/gitblit/utils/JSoupXssFilter.java +++ b/src/main/java/com/gitblit/utils/JSoupXssFilter.java @@ -68,16 +68,18 @@ public class JSoupXssFilter implements XssFilter { "sub", "sup", "table", "tbody", "td", "tfoot", "th", "thead", "tr", "tt", "u", "ul", "var") - .addAttributes("a", "href", "title") + .addAttributes("a", "class", "href", "style", "title") .addAttributes("blockquote", "cite") .addAttributes("col", "span", "width") .addAttributes("colgroup", "span", "width") + .addAttributes("div", "class", "style") .addAttributes("img", "align", "alt", "height", "src", "title", "width") .addAttributes("ol", "start", "type") .addAttributes("q", "cite") - .addAttributes("table", "summary", "width") - .addAttributes("td", "abbr", "axis", "colspan", "rowspan", "width") - .addAttributes("th", "abbr", "axis", "colspan", "rowspan", "scope", "width") + .addAttributes("span", "class", "style") + .addAttributes("table", "class", "style", "summary", "width") + .addAttributes("td", "abbr", "axis", "class", "colspan", "rowspan", "style", "width") + .addAttributes("th", "abbr", "axis", "class", "colspan", "rowspan", "scope", "style", "width") .addAttributes("ul", "type") .addEnforcedAttribute("a", "rel", "nofollow") diff --git a/src/main/java/com/gitblit/wicket/pages/EditTicketPage.java b/src/main/java/com/gitblit/wicket/pages/EditTicketPage.java index bd2ec63c..1adc7136 100644 --- a/src/main/java/com/gitblit/wicket/pages/EditTicketPage.java +++ b/src/main/java/com/gitblit/wicket/pages/EditTicketPage.java @@ -50,8 +50,6 @@ 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; @@ -112,8 +110,8 @@ public class EditTicketPage extends RepositoryPage { } typeModel = Model.of(ticket.type); - titleModel = SafeTextModel.none(ticket.title); - topicModel = SafeTextModel.none(ticket.topic == null ? "" : ticket.topic); + titleModel = Model.of(ticket.title); + topicModel = Model.of(ticket.topic == null ? "" : ticket.topic); responsibleModel = Model.of(); milestoneModel = Model.of(); mergeToModel = Model.of(ticket.mergeTo == null ? getRepositoryModel().mergeTo : ticket.mergeTo); @@ -136,7 +134,7 @@ public class EditTicketPage extends RepositoryPage { form.add(new TextField("title", titleModel)); form.add(new TextField("topic", topicModel)); - final SafeTextModel markdownPreviewModel = new SafeTextModel(Mode.none); + final IModel markdownPreviewModel = Model.of(ticket.body == null ? "" : ticket.body); 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 21a2af10..e6d9cb1e 100644 --- a/src/main/java/com/gitblit/wicket/pages/NewTicketPage.java +++ b/src/main/java/com/gitblit/wicket/pages/NewTicketPage.java @@ -49,8 +49,6 @@ 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; @@ -92,8 +90,8 @@ public class NewTicketPage extends RepositoryPage { } typeModel = Model.of(TicketModel.Type.defaultType); - titleModel = SafeTextModel.none(); - topicModel = SafeTextModel.none(); + titleModel = Model.of(); + topicModel = Model.of(); mergeToModel = Model.of(Repository.shortenRefName(getRepositoryModel().mergeTo)); responsibleModel = Model.of(); milestoneModel = Model.of(); @@ -108,7 +106,7 @@ public class NewTicketPage extends RepositoryPage { form.add(new TextField("title", titleModel)); form.add(new TextField("topic", topicModel)); - final SafeTextModel markdownPreviewModel = new SafeTextModel(Mode.none); + final IModel markdownPreviewModel = Model.of(); descriptionPreview = new Label("descriptionPreview", markdownPreviewModel); descriptionPreview.setEscapeModelStrings(false); descriptionPreview.setOutputMarkupId(true); diff --git a/src/main/java/com/gitblit/wicket/pages/TicketPage.java b/src/main/java/com/gitblit/wicket/pages/TicketPage.java index b1f94a5b..0bad8beb 100644 --- a/src/main/java/com/gitblit/wicket/pages/TicketPage.java +++ b/src/main/java/com/gitblit/wicket/pages/TicketPage.java @@ -287,7 +287,9 @@ public class TicketPage extends RepositoryPage { desc = getString("gb.noDescriptionGiven"); } else { String bugtraq = bugtraqProcessor().processText(getRepository(), repositoryName, ticket.body); - desc = MarkdownUtils.transformGFM(app().settings(), bugtraq, ticket.repository); + String html = MarkdownUtils.transformGFM(app().settings(), bugtraq, ticket.repository); + String safeHtml = app().xssFilter().relaxed(html); + desc = safeHtml; } add(new Label("ticketDescription", desc).setEscapeModelStrings(false)); @@ -523,7 +525,8 @@ public class TicketPage extends RepositoryPage { } else { // process the topic using the bugtraq config to link things String topic = bugtraqProcessor().processText(getRepository(), repositoryName, ticket.topic); - add(new Label("ticketTopic", topic).setEscapeModelStrings(false)); + String safeTopic = app().xssFilter().relaxed(topic); + add(new Label("ticketTopic", safeTopic).setEscapeModelStrings(false)); } @@ -703,6 +706,7 @@ public class TicketPage extends RepositoryPage { */ String bugtraq = bugtraqProcessor().processText(getRepository(), repositoryName, entry.comment.text); String comment = MarkdownUtils.transformGFM(app().settings(), bugtraq, repositoryName); + String safeComment = app().xssFilter().relaxed(comment); Fragment frag = new Fragment("entry", "commentFragment", this); Label commentIcon = new Label("commentIcon"); if (entry.comment.src == CommentSource.Email) { @@ -711,7 +715,7 @@ public class TicketPage extends RepositoryPage { WicketUtils.setCssClass(commentIcon, "iconic-comment-alt2-stroke"); } frag.add(commentIcon); - frag.add(new Label("comment", comment).setEscapeModelStrings(false)); + frag.add(new Label("comment", safeComment).setEscapeModelStrings(false)); addUserAttributions(frag, entry, avatarWidth); addDateAttributions(frag, entry); item.add(frag); @@ -972,7 +976,8 @@ public class TicketPage extends RepositoryPage { sb.append(""); } sb.append(""); - item.add(new Label("fields", sb.toString()).setEscapeModelStrings(false)); + String safeHtml = app().xssFilter().relaxed(sb.toString()); + item.add(new Label("fields", safeHtml).setEscapeModelStrings(false)); } else { item.add(new Label("fields").setVisible(false)); } diff --git a/src/main/java/com/gitblit/wicket/panels/CommentPanel.java b/src/main/java/com/gitblit/wicket/panels/CommentPanel.java index 130e7336..acf4dd9a 100644 --- a/src/main/java/com/gitblit/wicket/panels/CommentPanel.java +++ b/src/main/java/com/gitblit/wicket/panels/CommentPanel.java @@ -19,14 +19,13 @@ 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; @@ -90,7 +89,7 @@ public class CommentPanel extends BasePanel { } }.setVisible(ticket != null && ticket.number > 0)); - final SafeTextModel markdownPreviewModel = new SafeTextModel(Mode.none); + final IModel markdownPreviewModel = Model.of(); 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 6e06e5bb..ade92c09 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 SafeTextModel previewModel, final Label previewLabel) { + public MarkdownTextArea(String id, final IModel previewModel, final Label previewLabel) { super(id); setModel(new PropertyModel(this, "text")); add(new AjaxFormComponentUpdatingBehavior("onblur") { @@ -65,12 +65,13 @@ public class MarkdownTextArea extends TextArea { setOutputMarkupId(true); } - protected void renderPreview(SafeTextModel previewModel) { + protected void renderPreview(IModel previewModel) { if (text == null) { return; } String html = MarkdownUtils.transformGFM(GitBlitWebApp.get().settings(), text, repositoryName); - previewModel.setObject(html); + String safeHtml = GitBlitWebApp.get().xssFilter().relaxed(html); + previewModel.setObject(safeHtml); } public String getText() { diff --git a/src/main/java/com/gitblit/wicket/panels/TicketListPanel.java b/src/main/java/com/gitblit/wicket/panels/TicketListPanel.java index c7079c85..cc0b57a8 100644 --- a/src/main/java/com/gitblit/wicket/panels/TicketListPanel.java +++ b/src/main/java/com/gitblit/wicket/panels/TicketListPanel.java @@ -130,9 +130,10 @@ public class TicketListPanel extends BasePanel { Repository db = app().repositories().getRepository(repository.name); BugtraqProcessor btp = new BugtraqProcessor(app().settings()); String content = btp.processText(db, repository.name, labelItem.getModelObject()); + String safeContent = app().xssFilter().relaxed(content); db.close(); - label = new Label("label", content); + label = new Label("label", safeContent); label.setEscapeModelStrings(false); tLabel = app().tickets().getLabel(repository, labelItem.getModelObject()); -- 2.39.5