From 0bf060b69c57296e8bead71c5a9b17e7ae3ca0c1 Mon Sep 17 00:00:00 2001 From: Musachy Barroso Date: Wed, 19 Aug 2009 00:18:54 +0000 Subject: [PATCH] WW-3224 fix possible cross site scripting git-svn-id: https://svn.apache.org/repos/asf/struts/struts2/trunk@805635 13f79535-47bb-0310-9956-ffa450edef68 --- .../struts2/components/ActionError.java | 12 +++++-- .../struts2/components/ActionMessage.java | 11 +++++- .../apache/struts2/components/FieldError.java | 11 +++++- .../struts2/views/jsp/ui/ActionErrorTag.java | 12 +++++++ .../views/jsp/ui/ActionMessageTag.java | 14 ++++++++ .../struts2/views/jsp/ui/FieldErrorTag.java | 6 ++++ .../resources/template/simple/actionerror.ftl | 2 +- .../template/simple/actionmessage.ftl | 2 +- .../resources/template/simple/fielderror.ftl | 2 +- core/src/site/resources/tags/actionerror.html | 8 +++++ .../site/resources/tags/actionmessage.html | 8 +++++ core/src/site/resources/tags/fielderror.html | 8 +++++ .../views/jsp/ui/ActionErrorTagTest.java | 33 ++++++++++++++++++ .../views/jsp/ui/ActionMessageTagTest.java | 34 +++++++++++++++++++ .../views/jsp/ui/FieldErrorTagTest.java | 33 ++++++++++++++++++ 15 files changed, 189 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/components/ActionError.java b/core/src/main/java/org/apache/struts2/components/ActionError.java index 53e9ba7ed..8c855586f 100644 --- a/core/src/main/java/org/apache/struts2/components/ActionError.java +++ b/core/src/main/java/org/apache/struts2/components/ActionError.java @@ -25,6 +25,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.apache.struts2.views.annotations.StrutsTag; +import org.apache.struts2.views.annotations.StrutsTagAttribute; import org.apache.commons.lang.xwork.StringUtils; import com.opensymphony.xwork2.util.ValueStack; @@ -36,7 +37,8 @@ import java.util.List; * * * Render action errors if they exists the specific layout of the rendering depends on - * the theme itself. Empty (null or blank string) errors will not be printed. + * the theme itself. Empty (null or blank string) errors will not be printed. The action error + * strings will be html escaped by default. * * * @@ -58,7 +60,7 @@ import java.util.List; public class ActionError extends UIBean { public static final String TEMPLATE = "actionerror"; - + private boolean escape = true; public ActionError(ValueStack stack, HttpServletRequest request, HttpServletResponse response) { super(stack, request, response); @@ -81,5 +83,11 @@ public class ActionError extends UIBean { } addParameter("isEmptyList", isEmptyList); + addParameter("escape", escape); + } + + @StrutsTagAttribute(description=" Whether to escape HTML", type="Boolean", defaultValue="true") + public void setEscape(boolean escape) { + this.escape = escape; } } diff --git a/core/src/main/java/org/apache/struts2/components/ActionMessage.java b/core/src/main/java/org/apache/struts2/components/ActionMessage.java index 6d490b1fc..8165fe26b 100644 --- a/core/src/main/java/org/apache/struts2/components/ActionMessage.java +++ b/core/src/main/java/org/apache/struts2/components/ActionMessage.java @@ -25,6 +25,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.apache.struts2.views.annotations.StrutsTag; +import org.apache.struts2.views.annotations.StrutsTagAttribute; import org.apache.commons.lang.xwork.StringUtils; import com.opensymphony.xwork2.util.ValueStack; @@ -36,7 +37,8 @@ import java.util.Collection; * * * Render action messages if they exists, specific rendering layout depends on the - * theme itself. Empty (null or blank string) messages will not be printed. + * theme itself. Empty (null or blank string) messages will not be printed. The action message + * strings will be html escaped by default. * * * @@ -56,6 +58,7 @@ import java.util.Collection; public class ActionMessage extends UIBean { private static final String TEMPLATE = "actionmessage"; + protected boolean escape = true; public ActionMessage(ValueStack stack, HttpServletRequest request, HttpServletResponse response) { super(stack, request, response); @@ -78,5 +81,11 @@ public class ActionMessage extends UIBean { } addParameter("isEmptyList", isEmptyList); + addParameter("escape", escape); + } + + @StrutsTagAttribute(description=" Whether to escape HTML", type="Boolean", defaultValue="true") + public void setEscape(boolean escape) { + this.escape = escape; } } diff --git a/core/src/main/java/org/apache/struts2/components/FieldError.java b/core/src/main/java/org/apache/struts2/components/FieldError.java index 2145378bc..63fee5f3d 100644 --- a/core/src/main/java/org/apache/struts2/components/FieldError.java +++ b/core/src/main/java/org/apache/struts2/components/FieldError.java @@ -35,6 +35,7 @@ import java.util.List; * * * Render field errors if they exists. Specific layout depends on the particular theme. + * The field error strings will be html escaped by default. * * * @@ -91,6 +92,7 @@ import java.util.List; public class FieldError extends UIBean implements UnnamedParametric { private List errorFieldNames = new ArrayList(); + private boolean escape = true; public FieldError(ValueStack stack, HttpServletRequest request, HttpServletResponse response) { super(stack, request, response); @@ -107,7 +109,9 @@ public class FieldError extends UIBean implements UnnamedParametric { super.evaluateExtraParams(); if (errorFieldNames != null) - addParameter("errorFieldNames", errorFieldNames); + addParameter("errorFieldNames", errorFieldNames); + + addParameter("escape", escape); } public void addParameter(Object value) { @@ -124,5 +128,10 @@ public class FieldError extends UIBean implements UnnamedParametric { public void setFieldName(String fieldName) { addParameter(fieldName); } + + @StrutsTagAttribute(description=" Whether to escape HTML", type="Boolean", defaultValue="true") + public void setEscape(boolean escape) { + this.escape = escape; + } } diff --git a/core/src/main/java/org/apache/struts2/views/jsp/ui/ActionErrorTag.java b/core/src/main/java/org/apache/struts2/views/jsp/ui/ActionErrorTag.java index 7c6d3e58f..c09caaa6d 100644 --- a/core/src/main/java/org/apache/struts2/views/jsp/ui/ActionErrorTag.java +++ b/core/src/main/java/org/apache/struts2/views/jsp/ui/ActionErrorTag.java @@ -37,8 +37,20 @@ public class ActionErrorTag extends AbstractUITag { private static final long serialVersionUID = -3710234378022378639L; + private boolean escape = true; + public Component getBean(ValueStack stack, HttpServletRequest req, HttpServletResponse res) { return new ActionError(stack, req, res); } + protected void populateParams() { + super.populateParams(); + + ActionError error = (ActionError) component; + error.setEscape(escape); + } + + public void setEscape(boolean escape) { + this.escape = escape; + } } diff --git a/core/src/main/java/org/apache/struts2/views/jsp/ui/ActionMessageTag.java b/core/src/main/java/org/apache/struts2/views/jsp/ui/ActionMessageTag.java index 9cbbbe837..5e01c9c13 100644 --- a/core/src/main/java/org/apache/struts2/views/jsp/ui/ActionMessageTag.java +++ b/core/src/main/java/org/apache/struts2/views/jsp/ui/ActionMessageTag.java @@ -26,6 +26,7 @@ import javax.servlet.http.HttpServletResponse; import org.apache.struts2.components.ActionMessage; import org.apache.struts2.components.Component; +import org.apache.struts2.components.ActionError; import com.opensymphony.xwork2.util.ValueStack; @@ -37,7 +38,20 @@ public class ActionMessageTag extends AbstractUITag { private static final long serialVersionUID = 243396927554182506L; + private boolean escape = true; + public Component getBean(ValueStack stack, HttpServletRequest req, HttpServletResponse res) { return new ActionMessage(stack, req, res); } + + protected void populateParams() { + super.populateParams(); + + ActionMessage message = (ActionMessage) component; + message.setEscape(escape); + } + + public void setEscape(boolean escape) { + this.escape = escape; + } } diff --git a/core/src/main/java/org/apache/struts2/views/jsp/ui/FieldErrorTag.java b/core/src/main/java/org/apache/struts2/views/jsp/ui/FieldErrorTag.java index 32aa407ed..cf74c9d7d 100644 --- a/core/src/main/java/org/apache/struts2/views/jsp/ui/FieldErrorTag.java +++ b/core/src/main/java/org/apache/struts2/views/jsp/ui/FieldErrorTag.java @@ -37,6 +37,7 @@ public class FieldErrorTag extends AbstractUITag { private static final long serialVersionUID = -182532967507726323L; protected String fieldName; + protected boolean escape = true; public Component getBean(ValueStack stack, HttpServletRequest req, HttpServletResponse res) { @@ -48,10 +49,15 @@ public class FieldErrorTag extends AbstractUITag { FieldError fieldError = ((FieldError) component); fieldError.setFieldName(this.fieldName); + fieldError.setEscape(escape); } public void setFieldName(String fieldName) { this.fieldName = fieldName; } + + public void setEscape(boolean escape) { + this.escape = escape; + } } diff --git a/core/src/main/resources/template/simple/actionerror.ftl b/core/src/main/resources/template/simple/actionerror.ftl index 3ca9fa428..6563876d4 100644 --- a/core/src/main/resources/template/simple/actionerror.ftl +++ b/core/src/main/resources/template/simple/actionerror.ftl @@ -36,7 +36,7 @@ > <#list actionErrors as error> <#if error?if_exists != ""> -
  • ${error!}
    • +
  • <#if parameters.escape>${error!?html}<#else>${error!}<#rt/>
    • <#rt/> diff --git a/core/src/main/resources/template/simple/actionmessage.ftl b/core/src/main/resources/template/simple/actionmessage.ftl index c08308162..07174dd20 100644 --- a/core/src/main/resources/template/simple/actionmessage.ftl +++ b/core/src/main/resources/template/simple/actionmessage.ftl @@ -36,7 +36,7 @@ > <#list actionMessages as message> <#if message?if_exists != ""> -
  • ${message!}
    • +
  • <#if parameters.escape>${message!?html}<#else>${message!}
    • diff --git a/core/src/main/resources/template/simple/fielderror.ftl b/core/src/main/resources/template/simple/fielderror.ftl index 6760ec7e6..c036bb5c2 100644 --- a/core/src/main/resources/template/simple/fielderror.ftl +++ b/core/src/main/resources/template/simple/fielderror.ftl @@ -73,7 +73,7 @@ <#list eKeys as eKey><#t/> <#assign eValue = fieldErrors[eKey]><#t/> <#list eValue as eEachValue><#t/> -
  • ${eEachValue}
    • +
  • <#if parameters.escape>${eEachValue!?html}<#else>${eEachValue!}
    • <#t/> <#t/> diff --git a/core/src/site/resources/tags/actionerror.html b/core/src/site/resources/tags/actionerror.html index 04f82320b..76aba07d8 100644 --- a/core/src/site/resources/tags/actionerror.html +++ b/core/src/site/resources/tags/actionerror.html @@ -81,6 +81,14 @@ Please do not edit it directly. String Set the html disabled attribute on rendered html element + + escape + false + true + false + Boolean + Whether to escape HTML + id false diff --git a/core/src/site/resources/tags/actionmessage.html b/core/src/site/resources/tags/actionmessage.html index 00ea05042..d18d325e6 100644 --- a/core/src/site/resources/tags/actionmessage.html +++ b/core/src/site/resources/tags/actionmessage.html @@ -81,6 +81,14 @@ Please do not edit it directly. String Set the html disabled attribute on rendered html element + + escape + false + true + false + Boolean + Whether to escape HTML + id false diff --git a/core/src/site/resources/tags/fielderror.html b/core/src/site/resources/tags/fielderror.html index 9f7778c91..d5462d463 100644 --- a/core/src/site/resources/tags/fielderror.html +++ b/core/src/site/resources/tags/fielderror.html @@ -81,6 +81,14 @@ Please do not edit it directly. String Set the html disabled attribute on rendered html element + + escape + false + true + false + Boolean + Whether to escape HTML + fieldName false diff --git a/core/src/test/java/org/apache/struts2/views/jsp/ui/ActionErrorTagTest.java b/core/src/test/java/org/apache/struts2/views/jsp/ui/ActionErrorTagTest.java index 6ed56c674..b34848553 100644 --- a/core/src/test/java/org/apache/struts2/views/jsp/ui/ActionErrorTagTest.java +++ b/core/src/test/java/org/apache/struts2/views/jsp/ui/ActionErrorTagTest.java @@ -24,6 +24,7 @@ package org.apache.struts2.views.jsp.ui; import java.util.*; import org.apache.struts2.views.jsp.AbstractUITagTest; +import org.apache.struts2.TestAction; import org.apache.commons.lang.xwork.StringUtils; import com.opensymphony.xwork2.Action; @@ -48,6 +49,38 @@ public class ActionErrorTagTest extends AbstractUITagTest { verify(ActionErrorTagTest.class.getResource("actionerror-1.txt")); } + public void testActionErrorsEscape() throws Exception { + + ActionErrorTag tag = new ActionErrorTag(); + TestAction testAction = new TestAction(); + testAction.addActionError("

    hey

    "); + stack.pop(); + stack.push(testAction); + tag.setEscape(true); + tag.setPageContext(pageContext); + tag.doStartTag(); + tag.doEndTag(); + + assertEquals(normalize("
    • <p>hey</p>
    ", true), + normalize(writer.toString(), true)); + } + + public void testActionErrorsDontEscape() throws Exception { + + ActionErrorTag tag = new ActionErrorTag(); + TestAction testAction = new TestAction(); + testAction.addActionError("

    hey

    "); + stack.pop(); + stack.push(testAction); + tag.setEscape(false); + tag.setPageContext(pageContext); + tag.doStartTag(); + tag.doEndTag(); + + assertEquals(normalize("

    • hey

    ", true), + normalize(writer.toString(), true)); + } + public void testHaveActionErrors() throws Exception { ActionErrorTag tag = new ActionErrorTag(); diff --git a/core/src/test/java/org/apache/struts2/views/jsp/ui/ActionMessageTagTest.java b/core/src/test/java/org/apache/struts2/views/jsp/ui/ActionMessageTagTest.java index 9cfbeaa71..0c1d87bb5 100644 --- a/core/src/test/java/org/apache/struts2/views/jsp/ui/ActionMessageTagTest.java +++ b/core/src/test/java/org/apache/struts2/views/jsp/ui/ActionMessageTagTest.java @@ -24,6 +24,7 @@ package org.apache.struts2.views.jsp.ui; import java.util.*; import org.apache.struts2.views.jsp.AbstractUITagTest; +import org.apache.struts2.TestAction; import org.apache.commons.lang.xwork.StringUtils; import com.opensymphony.xwork2.Action; @@ -46,6 +47,39 @@ public class ActionMessageTagTest extends AbstractUITagTest { verify(ActionMessageTagTest.class.getResource("actionmessage-1.txt")); } + public void testActionMessageEscape() throws Exception { + + ActionMessageTag tag = new ActionMessageTag(); + TestAction testAction = new TestAction(); + testAction.addActionMessage("

    hey

    "); + stack.pop(); + stack.push(testAction); + tag.setEscape(true); + tag.setPageContext(pageContext); + tag.doStartTag(); + tag.doEndTag(); + + assertEquals(normalize("
    • <p>hey</p>
    ", true), + normalize(writer.toString(), true)); + } + + public void testActionErrorsDontEscape() throws Exception { + + ActionMessageTag tag = new ActionMessageTag(); + TestAction testAction = new TestAction(); + testAction.addActionMessage("

    hey

    "); + stack.pop(); + stack.push(testAction); + tag.setEscape(false); + tag.setPageContext(pageContext); + tag.doStartTag(); + tag.doEndTag(); + + assertEquals(normalize("

    • hey

    ", true), + normalize(writer.toString(), true)); + } + + public void testYesActionMessages() throws Exception { ActionMessageTag tag = new ActionMessageTag(); diff --git a/core/src/test/java/org/apache/struts2/views/jsp/ui/FieldErrorTagTest.java b/core/src/test/java/org/apache/struts2/views/jsp/ui/FieldErrorTagTest.java index 0740ec64b..50f5cb72c 100644 --- a/core/src/test/java/org/apache/struts2/views/jsp/ui/FieldErrorTagTest.java +++ b/core/src/test/java/org/apache/struts2/views/jsp/ui/FieldErrorTagTest.java @@ -29,6 +29,7 @@ import java.util.Map; import org.apache.struts2.views.jsp.AbstractUITagTest; import org.apache.struts2.views.jsp.ParamTag; +import org.apache.struts2.TestAction; import com.opensymphony.xwork2.Action; import com.opensymphony.xwork2.ActionSupport; @@ -60,6 +61,38 @@ public class FieldErrorTagTest extends AbstractUITagTest { verify(FieldErrorTagTest.class.getResource("fielderror-2.txt")); } + public void testFieldErrorsEscape() throws Exception { + + FieldErrorTag tag = new FieldErrorTag(); + TestAction testAction = new TestAction(); + testAction.addFieldError("f", "

    hey

    "); + stack.pop(); + stack.push(testAction); + tag.setEscape(true); + tag.setPageContext(pageContext); + tag.doStartTag(); + tag.doEndTag(); + + assertEquals(normalize("
    • <p>hey</p>
    ", true), + normalize(writer.toString(), true)); + } + + public void testFieldErrorsDontEscape() throws Exception { + + FieldErrorTag tag = new FieldErrorTag(); + TestAction testAction = new TestAction(); + testAction.addFieldError("f", "

    hey

    "); + stack.pop(); + stack.push(testAction); + tag.setEscape(false); + tag.setPageContext(pageContext); + tag.doStartTag(); + tag.doEndTag(); + + assertEquals(normalize("

    • hey

    ", true), + normalize(writer.toString(), true)); + } + public void testWithParamsWithFieldErrors1() throws Exception { FieldErrorTag tag = new FieldErrorTag(); tag.setId("someid");