This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/struts.git
The following commit(s) were added to refs/heads/main by this push:
new 8d6f13904 fix(core): WW-5623 HTML-encode form action in PostbackResult
to prevent XSS (#1653)
8d6f13904 is described below
commit 8d6f13904f9b1cd59fb96dcb62a85fa36f8d79c6
Author: quactv <[email protected]>
AuthorDate: Fri May 1 15:42:25 2026 +0700
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 <form action="">
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 <[email protected]>
---
.../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 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("<!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 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=<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();
+ }
}
