From 1b2242487179f38009d118707b8e70ab396a3d27 Mon Sep 17 00:00:00 2001 From: tranquac Date: Mon, 6 Apr 2026 22:17:29 +0700 Subject: [PATCH 1/2] 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. --- .../apache/struts2/result/PostbackResult.java | 16 +++++++++++++++- 1 file changed, 15 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 f46f7c52c3..da40110cc3 100644 --- a/core/src/main/java/org/apache/struts2/result/PostbackResult.java +++ b/core/src/main/java/org/apache/struts2/result/PostbackResult.java @@ -104,7 +104,8 @@ protected void doExecute(String finalLocation, ActionInvocation invocation) thro // Render PrintWriter pw = new PrintWriter(response.getOutputStream()); - pw.write(""); + String safeLocation = encodeHtml(finalLocation); + pw.write(""); writeFormElements(request, pw); writePrologueScript(pw); pw.write(""); @@ -213,6 +214,19 @@ public final void setPrependServletContext(boolean prependServletContext) { this.prependServletContext = prependServletContext; } + /** + * Encodes special HTML characters to prevent injection in HTML attribute context. + */ + private static String encodeHtml(String value) { + if (value == null) { + return ""; + } + return value.replace("&", "&") + .replace("\"", """) + .replace("<", "<") + .replace(">", ">"); + } + protected void writeFormElement(PrintWriter pw, String name, String[] values) throws UnsupportedEncodingException { for (String value : values) { String encName = URLEncoder.encode(name, StandardCharsets.UTF_8); From 721ef1c6c05db290af0ea28762251cb7ed9ac0b0 Mon Sep 17 00:00:00 2001 From: tranquac Date: Thu, 9 Apr 2026 07:16:30 +0700 Subject: [PATCH 2/2] 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 --- .../apache/struts2/result/PostbackResult.java | 17 +-- .../struts2/result/PostbackResultTest.java | 103 ++++++++++++++++++ 2 files changed, 105 insertions(+), 15 deletions(-) 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 da40110cc3..519feee9db 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.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,8 +105,7 @@ protected void doExecute(String finalLocation, ActionInvocation invocation) thro // Render PrintWriter pw = new PrintWriter(response.getOutputStream()); - String safeLocation = encodeHtml(finalLocation); - pw.write(""); + pw.write(""); writeFormElements(request, pw); writePrologueScript(pw); pw.write(""); @@ -214,19 +214,6 @@ public final void setPrependServletContext(boolean prependServletContext) { this.prependServletContext = prependServletContext; } - /** - * Encodes special HTML characters to prevent injection in HTML attribute context. - */ - private static String encodeHtml(String value) { - if (value == null) { - return ""; - } - return value.replace("&", "&") - .replace("\"", """) - .replace("<", "<") - .replace(">", ">"); - } - protected void writeFormElement(PrintWriter pw, String name, String[] values) throws UnsupportedEncodingException { for (String value : values) { String encName = URLEncoder.encode(name, StandardCharsets.UTF_8); 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 2bd310985e..c17bfdbd3b 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 void testPassingNullInvocation() throws Exception{ } } + /** + * 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=