[
https://issues.apache.org/jira/browse/WW-5636?focusedWorklogId=1025118&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1025118
]
ASF GitHub Bot logged work on WW-5636:
--------------------------------------
Author: ASF GitHub Bot
Created on: 14/Jun/26 16:21
Start Date: 14/Jun/26 16:21
Worklog Time Spent: 10m
Work Description: lukaszlenart commented on PR #1737:
URL: https://github.com/apache/struts/pull/1737#issuecomment-4702340405
Thanks for the patch and for filing WW-5636. Reviewed and verified against
current main.
The change is correct. The non-302 branch of `sendRedirect()` does write
`finalLocation` straight to the response body unescaped
(`ServletRedirectResult.java:251`), and HTML-escaping it brings this in line
with the existing convention in the same package - `PostbackResult` already
wraps its finalLocation in `StringEscapeUtils.escapeHtml4(...)`. Leaving the
`Location` header raw is the right call; only the body is escaped, so redirects
keep working. Full `ServletRedirectResultTest` suite passes (16/16) including
the new case.
A couple of notes for the record:
- Scope is defense-in-depth, not a default-config vulnerability. The default
`statusCode` is 302, which takes the `response.sendRedirect()` path and writes
no body - the escaped sink is only reachable when an operator explicitly
configures a non-302 status code and the location is authored (via OGNL
`parse=true`) to reflect user-controlled input. That combination is the kind of
forced-OGNL pattern our security guidance already warns against, so this is a
hardening/consistency improvement rather than a fix for an exploitable default.
I'd suggest softening the "reflected XSS / CWE-79" framing in the PR title
toward "harden / escape redirect URL in non-302 body" so the changelog doesn't
imply a default-config flaw was patched — WW-5636's neutral title is already a
good model.
- Minor accuracy point: WW-5636 notes the container "defaults to text/html."
Unlike `PostbackResult` (which calls `setContentType("text/html")`), this
branch never sets a content type, so that's container-dependent. The escaping
is correct hardening regardless.
Neither note is blocking. Happy to merge once the title is adjusted.
And a general reminder for the future: suspected security issues are best
sent to **[email protected]** first rather than opened as a public PR,
so we can triage severity before anything lands in the open. In this case the
impact is operator-owned, so no harm done.
Issue Time Tracking
-------------------
Worklog Id: (was: 1025118)
Remaining Estimate: 0h
Time Spent: 10m
> ServletRedirectResult writes unescaped URL to response body for non-302
> status codes
> ------------------------------------------------------------------------------------
>
> Key: WW-5636
> URL: https://issues.apache.org/jira/browse/WW-5636
> Project: Struts 2
> Issue Type: Bug
> Reporter: Arun Manni
> Priority: Minor
> Fix For: 7.2.0
>
> Time Spent: 10m
> Remaining Estimate: 0h
>
> When statusCode is not 302, ServletRedirectResult.sendRedirect() writes the
> redirect URL directly to the response body without HTML encoding. The servlet
> container defaults Content-Type to text/html, which means any HTML characters
> in the URL are rendered unescaped.
> PostbackResult.java (line 108) in the same package already uses
> StringEscapeUtils.escapeHtml4() for the identical pattern, confirming this
> was an oversight.
> This change applies escapeHtml4() to the response body output, matching the
> existing convention in PostbackResult.
> PR: https://github.com/apache/struts/pull/1737
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
