From 991e7ddc908b879355db80aa0e93099ac082bbfb Mon Sep 17 00:00:00 2001 From: mbiuki Date: Fri, 7 Aug 2026 18:38:17 -0400 Subject: [PATCH 1/3] =?UTF-8?q?sec:=20harden=20UtilMethods.getURL=20?= =?UTF-8?q?=E2=80=94=20restrict=20to=20http(s),=20block=20non-routable=20h?= =?UTF-8?q?osts?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit UtilMethods.getURL(String) fetched any URI a caller passed and returned the body, with no scheme allowlist or host restriction. It is exposed to the Velocity template context as $UtilMethods.getURL (VelocityUtil), so any design-layer (template/container) user could use it for local file read (file://) and full-read SSRF to loopback / link-local (cloud metadata) / private hosts. The introspector denylist blocks reflection/Runtime/etc but not this method. Restrict getURL to http/https and reject loopback/any-local/link-local/site-local/ multicast targets, returning empty + a SecurityLogger entry otherwise. getURL has no callers in Java or shipped .vtl, so no functional impact; $UtilMethods and its other helpers (isSet, date/HTML utils used by ~24 templates) are untouched. Adds SstiGetUrlReproTest as a regression guard (file:// read blocked, loopback SSRF blocked). Verified: ./mvnw test -pl :dotcms-core -Dtest=SstiGetUrlReproTest -> BUILD SUCCESS. Details: dotCMS/private-issues#668 Co-Authored-By: Claude Fable 5 --- .../com/dotmarketing/util/UtilMethods.java | 24 ++++++ .../dotcms/security/SstiGetUrlReproTest.java | 83 +++++++++++++++++++ 2 files changed, 107 insertions(+) create mode 100644 dotCMS/src/test/java/com/dotcms/security/SstiGetUrlReproTest.java diff --git a/dotCMS/src/main/java/com/dotmarketing/util/UtilMethods.java b/dotCMS/src/main/java/com/dotmarketing/util/UtilMethods.java index bea0da111438..d94b1ecc0f1a 100644 --- a/dotCMS/src/main/java/com/dotmarketing/util/UtilMethods.java +++ b/dotCMS/src/main/java/com/dotmarketing/util/UtilMethods.java @@ -1248,6 +1248,30 @@ public static StringBuffer getURL(String URI) throws java.net.ConnectException{ System.setProperty("sun.net.client.defaultConnectTimeout","10000"); java.net.URL pointer = new java.net.URL(URI); + // Security: this method is reachable from the Velocity template context as + // $UtilMethods.getURL by any design-layer (template/container) user. Restrict it to + // http(s) and refuse non-routable targets so it cannot be abused for local file read + // (file://, jar://, …) or SSRF to loopback / link-local (cloud metadata) / private + // hosts. See dotCMS/private-issues#668. + final String scheme = pointer.getProtocol() == null ? "" : pointer.getProtocol().toLowerCase(); + if (!"http".equals(scheme) && !"https".equals(scheme)) { + SecurityLogger.logInfo(UtilMethods.class, + "Blocked getURL: disallowed scheme '" + scheme + "' for " + URI); + return html; + } + try { + final InetAddress target = InetAddress.getByName(pointer.getHost()); + if (target.isLoopbackAddress() || target.isAnyLocalAddress() + || target.isLinkLocalAddress() || target.isSiteLocalAddress() + || target.isMulticastAddress()) { + SecurityLogger.logInfo(UtilMethods.class, + "Blocked getURL: internal/non-routable host '" + pointer.getHost() + "' for " + URI); + return html; + } + } catch (java.net.UnknownHostException uhe) { + return html; + } + java.net.URLConnection conn = pointer.openConnection(); conn.setUseCaches(false); conn.setConnectTimeout(10000); diff --git a/dotCMS/src/test/java/com/dotcms/security/SstiGetUrlReproTest.java b/dotCMS/src/test/java/com/dotcms/security/SstiGetUrlReproTest.java new file mode 100644 index 000000000000..17c4b51aa6fd --- /dev/null +++ b/dotCMS/src/test/java/com/dotcms/security/SstiGetUrlReproTest.java @@ -0,0 +1,83 @@ +package com.dotcms.security; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +import com.dotmarketing.util.UtilMethods; +import com.sun.net.httpserver.HttpServer; +import java.io.File; +import java.io.FileWriter; +import java.net.InetSocketAddress; +import java.nio.charset.StandardCharsets; +import java.util.concurrent.atomic.AtomicReference; +import org.apache.velocity.util.introspection.SecureIntrospectorImpl; +import org.junit.Test; + +/** + * Regression guard for the SSTI → arbitrary-file-read + SSRF finding via + * $UtilMethods.getURL() (pentest F1 / dotCMS/private-issues#668). + * + * $UtilMethods stays in the Velocity context and the SecureIntrospector denylist still permits + * the getURL *method* (used by ~24 templates for other helpers, and the introspector is + * class-based), so the defense lives inside getURL: it must refuse non-http(s) schemes and + * non-routable hosts. This test fails if that guard is removed and the primitive returns. + */ +public class SstiGetUrlReproTest { + + private static final String[] BAD_CLASSES = { + "java.lang.Class", "java.lang.ClassLoader", "java.lang.Runtime", "java.lang.Process", + "java.lang.System", "java.lang.Thread", "java.net.Socket", + "org.apache.velocity.app.VelocityEngine" + }; + private static final String[] BAD_PACKAGES = { "java.lang.reflect" }; + + @Test + public void getUrl_is_hardened_against_file_read_and_ssrf() throws Exception { + + // ---- The sandbox is real: it blocks dangerous classes but is class-based, so it still + // permits the getURL method. That is exactly why getURL itself must self-defend. ---- + final SecureIntrospectorImpl sandbox = new SecureIntrospectorImpl(BAD_CLASSES, BAD_PACKAGES); + assertTrue("sandbox should block Runtime.exec", + !sandbox.checkObjectExecutePermission(Runtime.class, "exec")); + assertTrue("sandbox is class-based and still exposes getURL — the guard must be in getURL", + sandbox.checkObjectExecutePermission(UtilMethods.class, "getURL")); + + // ---- (1) file:// arbitrary read must now be blocked (returns empty, no file contents) ---- + final File secret = File.createTempFile("dotcms-ssti-secret", ".txt"); + secret.deleteOnExit(); + final String marker = "DOT_INITIAL_ADMIN_PASSWORD=CANARY-" + System.nanoTime(); + try (FileWriter fw = new FileWriter(secret)) { fw.write(marker); } + final String fileRead = String.valueOf(UtilMethods.getURL(secret.toURI().toString())).trim(); + System.out.println("[LFI] file:// read now returns: '" + fileRead + "'"); + assertFalse("REGRESSION: file:// read leaked file contents", fileRead.contains(marker)); + assertTrue("REGRESSION: file:// read returned non-empty content", fileRead.isEmpty()); + + // ---- (2) SSRF to a loopback/internal host must now be blocked (no request, empty body) ---- + final HttpServer server = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0); + final AtomicReference hitMethod = new AtomicReference<>(null); + final String canary = "SSRF-CANARY-" + System.nanoTime(); + server.createContext("/oob", ex -> { + hitMethod.set(ex.getRequestMethod()); + final byte[] body = canary.getBytes(StandardCharsets.UTF_8); + ex.sendResponseHeaders(200, body.length); + ex.getResponseBody().write(body); + ex.close(); + }); + server.start(); + try { + final int port = server.getAddress().getPort(); + final String ssrf = String.valueOf( + UtilMethods.getURL("http://127.0.0.1:" + port + "/oob")).trim(); + System.out.println("[SSRF] loopback request hit method: " + hitMethod.get() + + " | body returned: '" + ssrf + "'"); + assertNull("REGRESSION: SSRF request reached the loopback listener", hitMethod.get()); + assertFalse("REGRESSION: SSRF returned the internal response body", ssrf.contains(canary)); + assertTrue("REGRESSION: SSRF returned non-empty content", ssrf.isEmpty()); + } finally { + server.stop(0); + } + + System.out.println("\n==== getURL hardened: file:// blocked, loopback SSRF blocked (F1 fixed) ===="); + } +} From 66fa643aad198b893fdf61365a3716a8f8e815b8 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 22:51:44 +0000 Subject: [PATCH 2/3] sec: disable redirect following in UtilMethods.getURL Set setInstanceFollowRedirects(false) on the HttpURLConnection so a 3xx response cannot redirect the request to an internal host after the loopback/link-local host check has passed. Closes the redirect-to-internal residual noted in the PR description. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01HMzJB7ujVZN6N2nV1xgJng --- dotCMS/src/main/java/com/dotmarketing/util/UtilMethods.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dotCMS/src/main/java/com/dotmarketing/util/UtilMethods.java b/dotCMS/src/main/java/com/dotmarketing/util/UtilMethods.java index d94b1ecc0f1a..1a4c84860f1c 100644 --- a/dotCMS/src/main/java/com/dotmarketing/util/UtilMethods.java +++ b/dotCMS/src/main/java/com/dotmarketing/util/UtilMethods.java @@ -1277,6 +1277,9 @@ public static StringBuffer getURL(String URI) throws java.net.ConnectException{ conn.setConnectTimeout(10000); if(conn instanceof java.net.HttpURLConnection){ java.net.HttpURLConnection myConn = (java.net.HttpURLConnection)conn; + // Disable redirect following so a 3xx response cannot redirect to an internal host, + // bypassing the loopback/link-local host check performed above. + myConn.setInstanceFollowRedirects(false); myConn.setRequestMethod("POST"); if(myConn.getResponseCode() != HttpServletResponse.SC_OK){ return null; From 7a4db6e622bc6bf8f88c6ae218349dc5d161462d Mon Sep 17 00:00:00 2001 From: mbiuki Date: Sat, 8 Aug 2026 08:59:19 -0400 Subject: [PATCH 3/3] sec(review): route getURL through CircuitBreakerUrl; strengthen SSRF host guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses review on #36969 (wezell, rsh1k, automated review): - Fetch now uses the shared CircuitBreakerUrl client (same path as $import): circuit breaker, timeout, IPUtils private-subnet gate, redirects disabled. Being HTTP-only it also removes the file:// read path. Replaces the hand-rolled URLConnection. - Kept a strong pre-connect host guard because IPUtils' default blacklist is weaker (misses 127/8, 0.0.0.0, full 169.254/16 incl. 169.254.170.2, IPv6). Now resolves via getAllByName and rejects if ANY address is non-routable (multi-record bypass), via a new isNonRoutable() helper that also covers IPv6 ULA (fd00::/7) and IPv4 CGNAT (100.64/10). - Fixed cosmetic 'jar://' -> 'jar:' comment. Regression test extended: file:// + loopback + IPv6 ::1 + CGNAT + ULA + 0.0.0.0 + 169.254.170.2 all return empty. ./mvnw test -pl :dotcms-core -Dtest=SstiGetUrlReproTest -> BUILD SUCCESS. Deferred to dotCMS/private-issues#668: TOCTOU/DNS-rebinding (pin connection to the validated IP) — needs client-level IP pinning CircuitBreakerUrl doesn't expose. Co-Authored-By: Claude Fable 5 --- .../com/dotmarketing/util/UtilMethods.java | 83 ++++++++++--------- .../dotcms/security/SstiGetUrlReproTest.java | 15 +++- 2 files changed, 60 insertions(+), 38 deletions(-) diff --git a/dotCMS/src/main/java/com/dotmarketing/util/UtilMethods.java b/dotCMS/src/main/java/com/dotmarketing/util/UtilMethods.java index 1a4c84860f1c..e28504f28641 100644 --- a/dotCMS/src/main/java/com/dotmarketing/util/UtilMethods.java +++ b/dotCMS/src/main/java/com/dotmarketing/util/UtilMethods.java @@ -1244,56 +1244,47 @@ public static StringBuffer getURL(String URI) throws java.net.ConnectException{ html.append(""); try { - System.setProperty("sun.net.client.defaultReadTimeout","20000"); - System.setProperty("sun.net.client.defaultConnectTimeout","10000"); - java.net.URL pointer = new java.net.URL(URI); - - // Security: this method is reachable from the Velocity template context as - // $UtilMethods.getURL by any design-layer (template/container) user. Restrict it to - // http(s) and refuse non-routable targets so it cannot be abused for local file read - // (file://, jar://, …) or SSRF to loopback / link-local (cloud metadata) / private - // hosts. See dotCMS/private-issues#668. + final java.net.URL pointer = new java.net.URL(URI); + + // Security: $UtilMethods.getURL is reachable from the Velocity template context by any + // design-layer (template/container) user. Restrict to http(s) so it cannot read local + // files (file:, jar:, …), and refuse non-routable targets so it cannot SSRF to + // loopback / link-local (cloud metadata) / private hosts. See dotCMS/private-issues#668. final String scheme = pointer.getProtocol() == null ? "" : pointer.getProtocol().toLowerCase(); if (!"http".equals(scheme) && !"https".equals(scheme)) { SecurityLogger.logInfo(UtilMethods.class, "Blocked getURL: disallowed scheme '" + scheme + "' for " + URI); return html; } + // Reject if ANY resolved address is non-routable — checking every A/AAAA record (not + // just the first) closes the multi-record bypass. isNonRoutable covers what + // CircuitBreakerUrl's IPUtils default blacklist misses: all of 127/8 loopback, 0.0.0.0, + // the whole 169.254/16 link-local range (incl. the ECS creds endpoint 169.254.170.2), + // IPv6 (::1, fe80::/10, fd00::/7 ULA), and IPv4 CGNAT 100.64/10. See private-issues#668. try { - final InetAddress target = InetAddress.getByName(pointer.getHost()); - if (target.isLoopbackAddress() || target.isAnyLocalAddress() - || target.isLinkLocalAddress() || target.isSiteLocalAddress() - || target.isMulticastAddress()) { - SecurityLogger.logInfo(UtilMethods.class, - "Blocked getURL: internal/non-routable host '" + pointer.getHost() + "' for " + URI); - return html; + for (final InetAddress target : InetAddress.getAllByName(pointer.getHost())) { + if (isNonRoutable(target)) { + SecurityLogger.logInfo(UtilMethods.class, + "Blocked getURL: internal/non-routable host '" + pointer.getHost() + "' for " + URI); + return html; + } } } catch (java.net.UnknownHostException uhe) { return html; } - java.net.URLConnection conn = pointer.openConnection(); - conn.setUseCaches(false); - conn.setConnectTimeout(10000); - if(conn instanceof java.net.HttpURLConnection){ - java.net.HttpURLConnection myConn = (java.net.HttpURLConnection)conn; - // Disable redirect following so a 3xx response cannot redirect to an internal host, - // bypassing the loopback/link-local host check performed above. - myConn.setInstanceFollowRedirects(false); - myConn.setRequestMethod("POST"); - if(myConn.getResponseCode() != HttpServletResponse.SC_OK){ - return null; - } + // Reuse the shared, hardened HTTP client ($import uses the same path): circuit breaker, + // timeout, IPUtils private-subnet gate, and redirects disabled so a 3xx cannot bounce to + // an internal host after the check above. + final String body = com.dotcms.http.CircuitBreakerUrl.builder() + .setUrl(URI) + .setTimeout(Config.getIntProperty("URL_CONNECTION_TIMEOUT", 10000)) + .setAllowRedirects(false) + .build() + .doString(); + if (UtilMethods.isSet(body)) { + html.append(body); } - - BufferedReader in = new BufferedReader(new InputStreamReader(conn.getInputStream())); - String inputLine; - - while ((inputLine = in.readLine()) != null) { - html.append(inputLine + "\n"); - } - - in.close(); } catch (Exception e) { Logger.debug(UtilMethods.class, "Browser class failed to get page: " + URI + " - " + e, e); Logger.warn(UtilMethods.class, "Browser class failed to get page: " + URI + " - " + e); @@ -1302,6 +1293,24 @@ public static StringBuffer getURL(String URI) throws java.net.ConnectException{ return html; } + /** + * SSRF guard for user-supplied URLs: an address is non-routable (must never be reachable + * via getURL) if it is loopback / any-local / link-local / site-local / multicast per the + * JDK predicates, or — cases those predicates miss — an IPv6 unique-local address + * (fd00::/7) or an IPv4 carrier-grade-NAT address (100.64.0.0/10, RFC 6598). + */ + private static boolean isNonRoutable(final InetAddress addr) { + if (addr.isLoopbackAddress() || addr.isAnyLocalAddress() || addr.isLinkLocalAddress() + || addr.isSiteLocalAddress() || addr.isMulticastAddress()) { + return true; + } + final byte[] b = addr.getAddress(); + if (b.length == 16 && (b[0] & 0xFE) == 0xFC) { + return true; // IPv6 unique-local fc00::/7 (fd00::/8) + } + return b.length == 4 && (b[0] & 0xFF) == 100 && (b[1] & 0xC0) == 0x40; // IPv4 CGNAT 100.64/10 + } + public static String capitalize(String s) { if (s == null) { return ""; diff --git a/dotCMS/src/test/java/com/dotcms/security/SstiGetUrlReproTest.java b/dotCMS/src/test/java/com/dotcms/security/SstiGetUrlReproTest.java index 17c4b51aa6fd..85dfadecf4a9 100644 --- a/dotCMS/src/test/java/com/dotcms/security/SstiGetUrlReproTest.java +++ b/dotCMS/src/test/java/com/dotcms/security/SstiGetUrlReproTest.java @@ -78,6 +78,19 @@ public void getUrl_is_hardened_against_file_read_and_ssrf() throws Exception { server.stop(0); } - System.out.println("\n==== getURL hardened: file:// blocked, loopback SSRF blocked (F1 fixed) ===="); + // ---- (3) other non-routable targets must also be refused (empty), per review ---- + for (final String url : new String[]{ + "http://[::1]:9999/x", // IPv6 loopback + "http://100.64.0.1/x", // IPv4 CGNAT (RFC 6598) + "http://[fd00::1]/x", // IPv6 unique-local (ULA) + "http://0.0.0.0/x", // any-local + "http://169.254.170.2/x"}) { // ECS credentials endpoint (link-local) + final String out = String.valueOf(UtilMethods.getURL(url)).trim(); + System.out.println("[BLOCK] " + url + " -> '" + out + "'"); + assertTrue("REGRESSION: " + url + " was not blocked", out.isEmpty()); + } + + System.out.println("\n==== getURL hardened: file:// blocked; loopback / IPv6 / CGNAT / ULA / " + + "link-local SSRF all blocked (F1 fixed) ===="); } }