Apache/struts
1
0
Fork 0
mirror of https://github.com/apache/struts.git synced 2026-08-05 14:47:09 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(core): WW-5623 HTML-encode form action in PostbackResult to prevent XSS (#1653)

Browse Source 
* 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 <tranquac@users.noreply.github.com>
This commit is contained in:
quactv
2026-05-01 15:42:25 +07:00
committed by GitHub
parent f4c6349283
commit 8d6f13904f
2 changed files with 105 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files
core/src/main/java/org/apache/struts2/result/PostbackResult.java
+2 -1
View File
@@ -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>");
core/src/test/java/org/apache/struts2/result/PostbackResultTest.java
+103
View File
@@ -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)\"&param=<script>", mockInvocation);
String output = res.getContentAsString();
// The action attribute must contain escaped HTML entities
assertTrue("Double quote should be escaped to &quot;",
output.contains("action=\"/test&quot;onmouseover=&quot;alert(1)&quot;&amp;param=&lt;script&gt;\""));
// 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("&amp;"));
assertTrue("Double-quote should be escaped", output.contains("&quot;"));
assertTrue("Less-than should be escaped", output.contains("&lt;"));
assertTrue("Greater-than should be escaped", output.contains("&gt;"));
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();
}
}