From 8d6f13904f9b1cd59fb96dcb62a85fa36f8d79c6 Mon Sep 17 00:00:00 2001 From: quactv <51528368+tranquac@users.noreply.github.com> Date: Fri, 1 May 2026 15:42:25 +0700 Subject: [PATCH] fix(core): WW-5623 HTML-encode form action in PostbackResult to prevent XSS (#1653) * fix(core): HTML-encode form action in PostbackResult to prevent XSS PostbackResult.doExecute() embeds finalLocation into a
attribute via raw string concatenation without HTML encoding. A double quote in the location breaks out of the attribute, enabling reflected XSS. The response Content-Type is text/html (line 103). This is an encoding inconsistency: form field names and values at lines 218-219 ARE properly URL-encoded via URLEncoder.encode(), but the form action attribute was not encoded at all. Add encodeHtml() to escape &, ", <, > in finalLocation before embedding it in the HTML form tag, consistent with the existing encoding approach for form field values in the same class. * fix(core): WW-5623 use StringEscapeUtils and add regression tests Address review feedback from @lukaszlenart: - Replace custom encodeHtml() with StringEscapeUtils.escapeHtml4() for consistency with the rest of Struts core (DefaultActionProxy, Property, TextProviderHelper all use StringEscapeUtils) - Add 3 focused unit tests in PostbackResultTest: - testFormActionHtmlEscaping: XSS payload with attribute breakout - testFormActionEscapesAllHtmlSpecialChars: covers ", &, <, > - testFormActionCleanLocationUnchanged: regression for clean URLs --------- Co-authored-by: tranquac --- .../apache/struts2/result/PostbackResult.java | 3 +- .../struts2/result/PostbackResultTest.java | 103 ++++++++++++++++++ 2 files changed, 105 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/apache/struts2/result/PostbackResult.java b/core/src/main/java/org/apache/struts2/result/PostbackResult.java index f46f7c52c..519feee9d 100644 --- a/core/src/main/java/org/apache/struts2/result/PostbackResult.java +++ b/core/src/main/java/org/apache/struts2/result/PostbackResult.java @@ -23,6 +23,7 @@ import org.apache.struts2.ActionInvocation; import org.apache.struts2.inject.Inject; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; +import org.apache.commons.text.StringEscapeUtils; import org.apache.struts2.dispatcher.mapper.ActionMapper; import org.apache.struts2.dispatcher.mapper.ActionMapping; @@ -104,7 +105,7 @@ public class PostbackResult extends StrutsResultSupport { // Render PrintWriter pw = new PrintWriter(response.getOutputStream()); - pw.write(""); + pw.write(""); writeFormElements(request, pw); writePrologueScript(pw); pw.write(""); diff --git a/core/src/test/java/org/apache/struts2/result/PostbackResultTest.java b/core/src/test/java/org/apache/struts2/result/PostbackResultTest.java index 2bd310985..c17bfdbd3 100644 --- a/core/src/test/java/org/apache/struts2/result/PostbackResultTest.java +++ b/core/src/test/java/org/apache/struts2/result/PostbackResultTest.java @@ -145,5 +145,108 @@ public class PostbackResultTest extends StrutsInternalTestCase { } } + /** + * WW-5623: Verify that HTML special characters in finalLocation are properly + * escaped in the rendered form action attribute to prevent XSS. + */ + public void testFormActionHtmlEscaping() throws Exception { + ActionContext context = ActionContext.getContext(); + ValueStack stack = context.getValueStack(); + MockHttpServletRequest req = new MockHttpServletRequest(); + MockHttpServletResponse res = new MockHttpServletResponse(); + context.put(ServletActionContext.HTTP_REQUEST, req); + context.put(ServletActionContext.HTTP_RESPONSE, res); + + // Push an object with a malicious property onto the value stack + stack.push(new Object() { + public String getTargetUrl() { + return "/test\"onmouseover=\"alert(1)"; + } + }); + + PostbackResult result = new PostbackResult(); + result.setLocation("/redirect?url=${targetUrl}"); + result.setPrependServletContext(false); + + IMocksControl control = createControl(); + ActionInvocation mockInvocation = control.createMock(ActionInvocation.class); + expect(mockInvocation.getInvocationContext()).andReturn(context).anyTimes(); + expect(mockInvocation.getStack()).andReturn(stack).anyTimes(); + + control.replay(); + result.setActionMapper(container.getInstance(ActionMapper.class)); + + // Call doExecute directly with a malicious location containing all critical chars + result.doExecute("/test\"onmouseover=\"alert(1)\"¶m=