[
https://issues.apache.org/jira/browse/WW-5639?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18097495#comment-18097495
]
Greg Huber commented on WW-5639:
--------------------------------
I will do a bit more testing, then do a PR if that's OK.
I looked at OWASP and then Jsoup, they remove the disallowed tag rather than
escape it, so could possibly loose data. Having one escapeHtml for <property>
for the whole app might seem a bit limiting so will test for this.
Also about the naming, and looking at other tags which use the same logic, the
escape for property would possibly be different to textarea (less strict?), so
will name the adapter per tag name, ie PropertyEscapeAdapter.java?
Thanks
> <s:property> escape options should be mutually exclusive
> --------------------------------------------------------
>
> Key: WW-5639
> URL: https://issues.apache.org/jira/browse/WW-5639
> Project: Struts 2
> Issue Type: Improvement
> Components: Core Tags
> Reporter: Lukasz Lenart
> Priority: Major
> Fix For: 7.3.0
>
>
> The {{<s:property>}} tag exposes four boolean escape attributes —
> {{escapeHtml}}
> (default {{true}}), {{escapeJavaScript}}, {{escapeXml}} and {{escapeCsv}}
> (default
> {{false}}). {{Property.prepare()}} applies them *cumulatively*, chaining one
> escaper after another. Chaining escapers that target different output contexts
> produces corrupt output:
> * {{escapeHtml=false escapeJavaScript=true}} on {{<b></b>}} yields
> {{<b><\/b>}},
> because {{escapeEcmaScript}} escapes {{/}} as {{\/}}. The closing tag is
> broken
> and the rendered layout is damaged. Allowing HTML markup ({{<b>}},
> {{<strong>}},
> {{<em>}}) while also JavaScript-escaping is impossible — the two contexts
> conflict by nature.
> * Leaving {{escapeHtml=true}} (the default) while also setting
> {{escapeXml=true}}
> or {{escapeCsv=true}} double-escapes entities ({{escapeHtml4}} turns {{&}}
> into
> {{&}}, then {{escapeXml10}} escapes the {{&}} again).
> h3. Note on security
> {{escapeJavaScript}} ({{escapeEcmaScript}}) escapes only {{' " \ /}} and
> control
> characters; it does *not* escape {{<}}, {{>}} or {{&}}, so it is not an
> HTML-safety mechanism. The real exposure is that {{escapeHtml=false}} emits
> raw,
> unescaped HTML — an XSS risk unless the value is fully trusted.
> h3. Proposed fix
> Make the four escape options mutually exclusive. Apply exactly one escaper,
> selected by first-match precedence:
> {noformat}
> escapeHtml -> escapeJavaScript -> escapeXml -> escapeCsv
> {noformat}
> HTML has the highest precedence. Since {{escapeHtml}} defaults to {{true}},
> the
> default behavior (HTML escaping only) is unchanged, and the established
> pattern of
> "set {{escapeHtml=false}} plus the one escaper you want" is preserved. The
> only
> outputs that change are configurations that were already producing corrupt
> double-escaped results.
> h3. Additionally
> * When {{devMode}} is enabled and more than one escape flag is set, log a
> warning
> naming which escaper was applied and noting the options are mutually
> exclusive.
> * Update the {{escapeHtml}} / {{escapeJavaScript}} {{@StrutsTagAttribute}}
> descriptions and the class JavaDoc: {{escapeHtml=false}} emits raw HTML (XSS
> risk unless the value is trusted); {{escapeJavaScript}} targets a JS
> string-literal context, not HTML, and escapes {{/}} as {{\/}} so it cannot
> coexist with surviving HTML markup.
> This is a behavior change and should ship in *7.3.x* (minor). The public docs
> page
> at [https://struts.apache.org/tag-developers/property-tag] is updated
> separately
> in the {{struts-site}} repo.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
