From a5962734a421184fef072d805424e15518633973 Mon Sep 17 00:00:00 2001
From: James Moger <james.moger@gitblit.com>
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/wicket/pages/EditTicketPage.java | 8 +++-----
src/main/java/com/gitblit/wicket/panels/CommentPanel.java | 5 ++---
src/main/java/com/gitblit/wicket/panels/MarkdownTextArea.java | 9 +++++----
src/main/java/com/gitblit/wicket/pages/NewTicketPage.java | 8 +++-----
src/main/java/com/gitblit/wicket/pages/TicketPage.java | 13 +++++++++----
src/main/java/com/gitblit/utils/JSoupXssFilter.java | 10 ++++++----
src/main/java/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 b07bcb9..7fa7b2a 100644
--- a/src/main/java/com/gitblit/utils/JSoupXssFilter.java
+++ b/src/main/java/com/gitblit/utils/JSoupXssFilter.java
@@ -68,16 +68,18 @@
"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 bd2ec63..1adc713 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.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 @@
}
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 @@
form.add(new TextField<String>("title", titleModel));
form.add(new TextField<String>("topic", topicModel));
- final SafeTextModel markdownPreviewModel = new SafeTextModel(Mode.none);
+ final IModel<String> 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 21a2af1..e6d9cb1 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.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 @@
}
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 @@
form.add(new TextField<String>("title", titleModel));
form.add(new TextField<String>("topic", topicModel));
- final SafeTextModel markdownPreviewModel = new SafeTextModel(Mode.none);
+ final IModel<String> 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 b1f94a5..0bad8be 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 @@
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 @@
} 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 @@
*/
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 @@
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 @@
sb.append("</td></tr>");
}
sb.append("</tbody></table>");
- 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 130e733..acf4dd9 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.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 @@
}
}.setVisible(ticket != null && ticket.number > 0));
- final SafeTextModel markdownPreviewModel = new SafeTextModel(Mode.none);
+ final IModel<String> 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 6e06e5b..ade92c0 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 SafeTextModel previewModel, final Label previewLabel) {
+ public MarkdownTextArea(String id, final IModel<String> previewModel, final Label previewLabel) {
super(id);
setModel(new PropertyModel(this, "text"));
add(new AjaxFormComponentUpdatingBehavior("onblur") {
@@ -65,12 +65,13 @@
setOutputMarkupId(true);
}
- protected void renderPreview(SafeTextModel previewModel) {
+ protected void renderPreview(IModel<String> 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 c7079c8..cc0b57a 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 @@
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());
--
Gitblit v1.9.1