Implement a SafeTextModel and use that for fields vulnerable to XSS

James Moger

2014-09-06 209dbdd49a89d6e3cebf61e860c779a1d8561dd9

Implement a SafeTextModel and use that for fields vulnerable to XSS

 src/main/java/com/gitblit/wicket/SafeTextModel.java

New file
@@ -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());
    }
}

 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);


 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);


 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);

 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;
        }