diff --git a/dotCMS/src/main/java/com/dotmarketing/util/UtilMethods.java b/dotCMS/src/main/java/com/dotmarketing/util/UtilMethods.java index bea0da111438..e28504f28641 100644 --- a/dotCMS/src/main/java/com/dotmarketing/util/UtilMethods.java +++ b/dotCMS/src/main/java/com/dotmarketing/util/UtilMethods.java @@ -1244,29 +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); - - 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; - myConn.setRequestMethod("POST"); - if(myConn.getResponseCode() != HttpServletResponse.SC_OK){ - return null; + 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 { + 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; } - BufferedReader in = new BufferedReader(new InputStreamReader(conn.getInputStream())); - String inputLine; - - while ((inputLine = in.readLine()) != null) { - html.append(inputLine + "\n"); + // 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); } - - 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); @@ -1275,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 new file mode 100644 index 000000000000..85dfadecf4a9 --- /dev/null +++ b/dotCMS/src/test/java/com/dotcms/security/SstiGetUrlReproTest.java @@ -0,0 +1,96 @@ +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); + } + + // ---- (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) ===="); + } +}