This is an automated email from the ASF dual-hosted git repository. rzo1 pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/storm.git
commit 5aa8698eae70f449a7eb1035145750ba43a9ff5e Author: Richard Zowalla <[email protected]> AuthorDate: Tue Apr 14 13:35:33 2026 +0200 Hardening: validate JSONP callback and add nosniff on UI JSON responses --- .../java/org/apache/storm/daemon/ui/UIHelpers.java | 23 ++++++- .../org/apache/storm/daemon/ui/UIHelpersTest.java | 73 ++++++++++++++++++++++ 2 files changed, 93 insertions(+), 3 deletions(-) diff --git a/storm-webapp/src/main/java/org/apache/storm/daemon/ui/UIHelpers.java b/storm-webapp/src/main/java/org/apache/storm/daemon/ui/UIHelpers.java index 274d5a0bd..7ebb39cc3 100644 --- a/storm-webapp/src/main/java/org/apache/storm/daemon/ui/UIHelpers.java +++ b/storm-webapp/src/main/java/org/apache/storm/daemon/ui/UIHelpers.java @@ -444,9 +444,23 @@ public class UIHelpers { stormRunJetty(port, null, null, headerBufferSize, configurator); } + private static final Pattern JSONP_CALLBACK_PATTERN = + Pattern.compile("^[A-Za-z_$][A-Za-z0-9_$]*(?:\\.[A-Za-z_$][A-Za-z0-9_$]*)*$"); + + private static String sanitizeJsonpCallback(String callback) { + if (callback == null) { + return null; + } + if (callback.length() > 128 || !JSONP_CALLBACK_PATTERN.matcher(callback).matches()) { + LOG.warn("Ignoring invalid JSONP callback parameter"); + return null; + } + return callback; + } + /** * wrapJsonInCallback. - * @param callback callbackParameterName + * @param callback callbackParameterName (must already be validated) * @param response response * @return wrapJsonInCallback */ @@ -461,6 +475,7 @@ public class UIHelpers { * @return getJsonResponseHeaders */ public static Map getJsonResponseHeaders(String callback, Map headers) { + final String safeCallback = sanitizeJsonpCallback(callback); Map<String, String> headersResult = new HashMap<>(); headersResult.put("Cache-Control", "no-cache, no-store"); headersResult.put("Access-Control-Allow-Origin", "*"); @@ -469,7 +484,8 @@ public class UIHelpers { + "Access-Controler-Allow-Origin, " + "X-Requested-By, X-Csrf-Token, " + "Authorization, X-Requested-With"); - if (callback != null) { + headersResult.put("X-Content-Type-Options", "nosniff"); + if (safeCallback != null) { headersResult.put("Content-Type", "application/javascript;charset=utf-8"); } else { headersResult.put("Content-Type", "application/json;charset=utf-8"); @@ -481,8 +497,9 @@ public class UIHelpers { } public static String getJsonResponseBody(Object data, String callback, boolean needSerialize) { + String safeCallback = sanitizeJsonpCallback(callback); String serializedData = needSerialize ? JSONValue.toJSONString(data) : (String) data; - return callback != null ? wrapJsonInCallback(callback, serializedData) : serializedData; + return safeCallback != null ? wrapJsonInCallback(safeCallback, serializedData) : serializedData; } /** diff --git a/storm-webapp/src/test/java/org/apache/storm/daemon/ui/UIHelpersTest.java b/storm-webapp/src/test/java/org/apache/storm/daemon/ui/UIHelpersTest.java index 30e8e0e53..852af5cb9 100644 --- a/storm-webapp/src/test/java/org/apache/storm/daemon/ui/UIHelpersTest.java +++ b/storm-webapp/src/test/java/org/apache/storm/daemon/ui/UIHelpersTest.java @@ -592,4 +592,77 @@ class UIHelpersTest { .findFirst() .orElseThrow(() -> new IllegalArgumentException("Unable to find entry for spoutId '" + spoutId + "'")); } + + @Test + public void testGetJsonResponseBodyNoCallbackReturnsJson() { + Map<String, Object> data = new HashMap<>(); + data.put("a", 1); + String body = UIHelpers.getJsonResponseBody(data, null, true); + assertEquals("{\"a\":1}", body); + } + + @Test + public void testGetJsonResponseBodyValidCallbackIsWrapped() { + Map<String, Object> data = new HashMap<>(); + data.put("a", 1); + String body = UIHelpers.getJsonResponseBody(data, "myCb", true); + assertEquals("myCb({\"a\":1});", body); + } + + @Test + public void testGetJsonResponseBodyValidDottedCallbackIsWrapped() { + String body = UIHelpers.getJsonResponseBody("{\"x\":1}", "foo.bar.$baz_0", false); + assertEquals("foo.bar.$baz_0({\"x\":1});", body); + } + + @Test + public void testGetJsonResponseBodyInvalidCallbackFallsBackToJson() { + Map<String, Object> data = new HashMap<>(); + data.put("a", 1); + String body = UIHelpers.getJsonResponseBody(data, "alert(document.cookie)//", true); + assertEquals("{\"a\":1}", body); + } + + @Test + public void testGetJsonResponseBodyEmptyCallbackFallsBackToJson() { + String body = UIHelpers.getJsonResponseBody("{\"x\":1}", "", false); + assertEquals("{\"x\":1}", body); + } + + @Test + public void testGetJsonResponseBodyTooLongCallbackFallsBackToJson() { + StringBuilder sb = new StringBuilder("cb"); + for (int i = 0; i < 200; i++) { + sb.append('x'); + } + String body = UIHelpers.getJsonResponseBody("{\"x\":1}", sb.toString(), false); + assertEquals("{\"x\":1}", body); + } + + @Test + public void testGetJsonResponseBodyRejectsCallbacksStartingWithDigit() { + String body = UIHelpers.getJsonResponseBody("{\"x\":1}", "1cb", false); + assertEquals("{\"x\":1}", body); + } + + @Test + public void testGetJsonResponseHeadersNoCallbackUsesJsonContentType() { + Map headers = UIHelpers.getJsonResponseHeaders(null, null); + assertEquals("application/json;charset=utf-8", headers.get("Content-Type")); + assertEquals("nosniff", headers.get("X-Content-Type-Options")); + } + + @Test + public void testGetJsonResponseHeadersValidCallbackUsesJavaScriptContentType() { + Map headers = UIHelpers.getJsonResponseHeaders("myCb", null); + assertEquals("application/javascript;charset=utf-8", headers.get("Content-Type")); + assertEquals("nosniff", headers.get("X-Content-Type-Options")); + } + + @Test + public void testGetJsonResponseHeadersInvalidCallbackFallsBackToJsonContentType() { + Map headers = UIHelpers.getJsonResponseHeaders("alert(1)//", null); + assertEquals("application/json;charset=utf-8", headers.get("Content-Type")); + assertEquals("nosniff", headers.get("X-Content-Type-Options")); + } } \ No newline at end of file
