WW-5636 Harden redirect URL escaping in non-302 response body

[arunmanni-ai]

Summary

ServletRedirectResult.sendRedirect() writes the redirect target URL
directly to the response body for any statusCode != 302 (e.g. 301, 303,
307, 308, or misconfigured 200), without HTML encoding. Since the servlet
container defaults Content-Type to text/html, this is a reflected XSS
sink when user-controlled content reaches finalLocation via OGNL
expression evaluation (parse=true, the default).

PostbackResult.java (line 108) in the same package already uses
StringEscapeUtils.escapeHtml4() for the identical pattern — this aligns
ServletRedirectResult with the existing convention.

Changes

• HTML-escape finalLocation before writing to response body in sendRedirect()
• Location header is intentionally left unescaped (required for valid redirect)
• New test verifies <script> payload is escaped in body while Location header keeps raw URL

References

• CWE-79: Improper Neutralization of Input During Web Page Generation
• Comparable fix: PostbackResult.java line 108 (same codebase)

[arunmanni-ai]

JIRA ticket: https://issues.apache.org/jira/browse/WW-5636

[lukaszlenart]

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 security@struts.apache.org 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.

[arunmanni-ai]

Thanks for the thorough review. Updated the title to match the hardening framing. Noted on reporting suspected security issues to security@struts.apache.org first — will follow that process going forward.