This is an automated email from the ASF dual-hosted git repository. lukaszlenart pushed a commit to branch WW-5623-postback-result-xss-6x in repository https://gitbox.apache.org/repos/asf/struts.git
commit 90751ff9a8a08acd046c6cf538034f58f2896ddf Author: Lukasz Lenart <[email protected]> AuthorDate: Wed May 20 08:52:34 2026 +0200 WW-5623 fix(core): HTML-encode form action in PostbackResult to prevent XSS PostbackResult.doExecute() embeds finalLocation into a <form action=""> attribute via raw string concatenation. A double quote in the location breaks out of the attribute, enabling reflected XSS. The response Content-Type is text/html. Form field names and values elsewhere in the same class are properly URL-encoded via URLEncoder.encode(); the action attribute was not encoded at all. Wrap finalLocation with StringEscapeUtils.escapeHtml4() before embedding it in the form tag, consistent with the encoding approach used in DefaultActionProxy, Property, and TextProviderHelper. Adds three regression tests in PostbackResultTest: - testFormActionHtmlEscaping: XSS payload with attribute breakout - testFormActionEscapesAllHtmlSpecialChars: covers ", &, <, > - testFormActionCleanLocationUnchanged: regression for clean URLs --- .../org/apache/struts2/result/PostbackResult.java | 3 +- .../apache/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 96015e8fd..8c4056ca9 100644 --- a/core/src/main/java/org/apache/struts2/result/PostbackResult.java +++ b/core/src/main/java/org/apache/struts2/result/PostbackResult.java @@ -21,6 +21,7 @@ package org.apache.struts2.result; import com.opensymphony.xwork2.ActionContext; import com.opensymphony.xwork2.ActionInvocation; import com.opensymphony.xwork2.inject.Inject; +import org.apache.commons.text.StringEscapeUtils; import org.apache.struts2.dispatcher.mapper.ActionMapper; import org.apache.struts2.dispatcher.mapper.ActionMapping; @@ -101,7 +102,7 @@ public class PostbackResult extends StrutsResultSupport { // Render PrintWriter pw = new PrintWriter(response.getOutputStream()); - pw.write("<!DOCTYPE html><html><body><form action=\"" + finalLocation + "\" method=\"POST\">"); + pw.write("<!DOCTYPE html><html><body><form action=\"" + StringEscapeUtils.escapeHtml4(finalLocation) + "\" method=\"POST\">"); writeFormElements(request, pw); writePrologueScript(pw); pw.write("</html>"); 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 f019436f7..3ec70637f 100644 --- a/core/src/test/java/org/apache/struts2/result/PostbackResultTest.java +++ b/core/src/test/java/org/apache/struts2/result/PostbackResultTest.java @@ -146,5 +146,108 @@ public class PostbackResultTest extends StrutsInternalTestCase { } } + /** + * WW-5623: Verify that HTML special characters in finalLocation are properly + * escaped in the rendered form action attribute. + */ + 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=<script>", mockInvocation); + + String output = res.getContentAsString(); + + // The action attribute must contain escaped HTML entities + assertTrue("Double quote should be escaped to "", + output.contains("action=\"/test"onmouseover="alert(1)"&param=<script>\"")); + // Must not contain unescaped double-quote that breaks out of the attribute + assertFalse("Raw double-quote must not appear in action value", + output.contains("action=\"/test\"")); + + control.verify(); + } + + /** + * WW-5623: Verify that each individual HTML special character is properly escaped. + */ + public void testFormActionEscapesAllHtmlSpecialChars() throws Exception { + ActionContext context = ActionContext.getContext(); + MockHttpServletRequest req = new MockHttpServletRequest(); + MockHttpServletResponse res = new MockHttpServletResponse(); + context.put(ServletActionContext.HTTP_REQUEST, req); + context.put(ServletActionContext.HTTP_RESPONSE, res); + + IMocksControl control = createControl(); + ActionInvocation mockInvocation = control.createMock(ActionInvocation.class); + expect(mockInvocation.getInvocationContext()).andReturn(context).anyTimes(); + + control.replay(); + + PostbackResult result = new PostbackResult(); + result.setActionMapper(container.getInstance(ActionMapper.class)); + result.doExecute("/path?a=1&b=2\"<>", mockInvocation); + + String output = res.getContentAsString(); + + assertTrue("Ampersand should be escaped", output.contains("&")); + assertTrue("Double-quote should be escaped", output.contains(""")); + assertTrue("Less-than should be escaped", output.contains("<")); + assertTrue("Greater-than should be escaped", output.contains(">")); + + control.verify(); + } + + /** + * WW-5623: Verify that a clean location (no special chars) renders unchanged. + */ + public void testFormActionCleanLocationUnchanged() throws Exception { + ActionContext context = ActionContext.getContext(); + MockHttpServletRequest req = new MockHttpServletRequest(); + MockHttpServletResponse res = new MockHttpServletResponse(); + context.put(ServletActionContext.HTTP_REQUEST, req); + context.put(ServletActionContext.HTTP_RESPONSE, res); + + IMocksControl control = createControl(); + ActionInvocation mockInvocation = control.createMock(ActionInvocation.class); + expect(mockInvocation.getInvocationContext()).andReturn(context).anyTimes(); + + control.replay(); + + PostbackResult result = new PostbackResult(); + result.setActionMapper(container.getInstance(ActionMapper.class)); + result.doExecute("/clean/path/action.do", mockInvocation); + + String output = res.getContentAsString(); + + assertTrue("Clean location should render as-is in action attribute", + output.contains("action=\"/clean/path/action.do\"")); + + control.verify(); + } }
