Merge pull request #18153 from owen-mc/java/resttemplate-getforobject

Java: add SSRF sink model for the third parameter of `RestTemplate.getForObject`

java/ql/lib/change-notes/2024-11-28-model-resttemplate-getforobject-third-parameter.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Added a sink for "Server-side request forgery" (`java/ssrf`) for the third parameter to org.springframework.web.client.RestTemplate.getForObject, when we cannot statically determine that it does not affect the host in the URL.

java/ql/lib/semmle/code/java/frameworks/spring/SpringWebClient.qll

@@ -4,6 +4,7 @@
 
 import java
 import SpringHttp
+private import semmle.code.java.security.RequestForgery
 
 /** The class `org.springframework.web.client.RestTemplate`. */
 class SpringRestTemplate extends Class {
@@ -27,3 +28,76 @@ class SpringWebClient extends Interface {
     this.hasQualifiedName("org.springframework.web.reactive.function.client", "WebClient")
   }
 }
+
+/** The method `getForObject` on `org.springframework.web.reactive.function.client.RestTemplate`. */
+class SpringRestTemplateGetForObjectMethod extends Method {
+  SpringRestTemplateGetForObjectMethod() {
+    this.getDeclaringType() instanceof SpringRestTemplate and
+    this.hasName("getForObject")
+  }
+}
+
+/** A call to the method `getForObject` on `org.springframework.web.reactive.function.client.RestTemplate`. */
+class SpringRestTemplateGetForObjectMethodCall extends MethodCall {
+  SpringRestTemplateGetForObjectMethodCall() {
+    this.getMethod() instanceof SpringRestTemplateGetForObjectMethod
+  }
+
+  /** Gets the first argument, if it is a compile time constant. */
+  CompileTimeConstantExpr getConstantUrl() { result = this.getArgument(0) }
+
+  /**
+   * Holds if the first argument is a compile time constant and it has a
+   * placeholder at offset `offset`, and there are `idx` placeholders that
+   * appear before it.
+   */
+  predicate urlHasPlaceholderAtOffset(int idx, int offset) {
+    exists(
+      this.getConstantUrl()
+          .getStringValue()
+          .replaceAll("\\{", "  ")
+          .replaceAll("\\}", "  ")
+          .regexpFind("\\{[^}]*\\}", idx, offset)
+    )
+  }
+}
+
+private class SpringWebClientRestTemplateGetForObject extends RequestForgerySink {
+  SpringWebClientRestTemplateGetForObject() {
+    exists(SpringRestTemplateGetForObjectMethodCall mc, int i |
+      // Note that the first argument is modeled as a request forgery sink
+      // separately. This model is for arguments beyond the first two. There
+      // are two relevant overloads, one with third parameter type `Object...`
+      // and one with third parameter type `Map<String, ?>`. For the latter we
+      // cannot deal with MapValue content easily but there is a default
+      // implicit taint read at sinks that will catch it.
+      i >= 0 and
+      this.asExpr() = mc.getArgument(i + 2)
+    |
+      // If we can determine that part of mc.getArgument(0) is a hostname
+      // sanitizing prefix, then we count how many placeholders occur before it
+      // and only consider that many arguments beyond the first two as sinks.
+      // For the `Map<String, ?>` overload this has the effect of only
+      // considering the map values as sinks if there is at least one
+      // placeholder in the URL before the hostname sanitizing prefix.
+      exists(int offset |
+        mc.urlHasPlaceholderAtOffset(i, offset) and
+        offset < mc.getConstantUrl().(HostnameSanitizingPrefix).getOffset()
+      )
+      or
+      // If we cannot determine that part of mc.getArgument(0) is a hostname
+      // sanitizing prefix, but it is a compile time constant and we can get
+      // its string value, then we count how many placeholders occur in it
+      // and only consider that many arguments beyond the first two as sinks.
+      // For the `Map<String, ?>` overload this has the effect of only
+      // considering the map values as sinks if there is at least one
+      // placeholder in the URL.
+      not mc.getConstantUrl() instanceof HostnameSanitizingPrefix and
+      mc.urlHasPlaceholderAtOffset(i, _)
+      or
+      // If we cannot determine the string value of mc.getArgument(0), then we
+      // conservatively consider all arguments as sinks.
+      not exists(mc.getConstantUrl().getStringValue())
+    )
+  }
+}

java/ql/lib/semmle/code/java/security/RequestForgery.qll

@@ -63,14 +63,17 @@ abstract class RequestForgerySanitizer extends DataFlow::Node { }
 
 private class PrimitiveSanitizer extends RequestForgerySanitizer instanceof SimpleTypeSanitizer { }
 
-private class HostnameSanitizingPrefix extends InterestingPrefix {
+/**
+ * A string constant that contains a prefix which looks like when it is prepended to untrusted
+ * input, it will restrict the host or entity addressed.
+ *
+ * For example, anything containing `?` or `#`, or a slash that doesn't appear to be a protocol
+ * specifier (e.g. `http://` is not sanitizing), or specifically the string "/".
+ */
+class HostnameSanitizingPrefix extends InterestingPrefix {
   int offset;
 
   HostnameSanitizingPrefix() {
-    // Matches strings that look like when prepended to untrusted input, they will restrict
-    // the host or entity addressed: for example, anything containing `?` or `#`, or a slash that
-    // doesn't appear to be a protocol specifier (e.g. `http://` is not sanitizing), or specifically
-    // the string "/".
     exists(this.getStringValue().regexpFind("([?#]|[^?#:/\\\\][/\\\\])|^/$", 0, offset))
   }
 
@@ -81,8 +84,10 @@ private class HostnameSanitizingPrefix extends InterestingPrefix {
  * A value that is the result of prepending a string that prevents any value from controlling the
  * host of a URL.
  */
-private class HostnameSantizer extends RequestForgerySanitizer {
-  HostnameSantizer() { this.asExpr() = any(HostnameSanitizingPrefix hsp).getAnAppendedExpression() }
+private class HostnameSanitizer extends RequestForgerySanitizer {
+  HostnameSanitizer() {
+    this.asExpr() = any(HostnameSanitizingPrefix hsp).getAnAppendedExpression()
+  }
 }
 
 /**

java/ql/test/query-tests/security/CWE-918/SpringSSRF.java

@@ -13,6 +13,7 @@ import java.net.http.HttpClient;
 import java.net.http.HttpRequest;
 import java.net.Proxy.Type;
 import java.io.InputStream;
+import java.util.Map;
 
 import org.apache.http.client.methods.HttpGet;
 import javax.servlet.ServletException;
@@ -32,6 +33,14 @@ public class SpringSSRF extends HttpServlet {
             restTemplate.exchange(fooResourceUrl, HttpMethod.POST, request, String.class); // $ SSRF
             restTemplate.execute(fooResourceUrl, HttpMethod.POST, null, null, "test"); // $ SSRF
             restTemplate.getForObject(fooResourceUrl, String.class, "test"); // $ SSRF
+            restTemplate.getForObject("http://{foo}", String.class, fooResourceUrl); // $ SSRF
+            restTemplate.getForObject("http://{foo}/a/b", String.class, fooResourceUrl); // $ SSRF
+            restTemplate.getForObject("http://safe.com/{foo}", String.class, fooResourceUrl); // not bad - the tainted value does not affect the host
+            restTemplate.getForObject("http://{foo}", String.class, "safe.com", fooResourceUrl); // not bad - the tainted value is unused
+            restTemplate.getForObject("http://{foo}", String.class, Map.of("foo", fooResourceUrl)); // $ SSRF
+            restTemplate.getForObject("http://safe.com/{foo}", String.class, Map.of("foo", fooResourceUrl)); // not bad - the tainted value does not affect the host
+            restTemplate.getForObject("http://{foo}", String.class, Map.of("foo", "safe.com", "unused", fooResourceUrl)); // $ SPURIOUS: SSRF // not bad - the key for the tainted value is unused
+            restTemplate.getForObject("http://{foo}", String.class, Map.of("foo", "safe.com", fooResourceUrl, "unused")); // not bad - the tainted value is in a map key
             restTemplate.patchForObject(fooResourceUrl, new String("object"), String.class, "hi"); // $ SSRF
             restTemplate.postForEntity(new URI(fooResourceUrl), new String("object"), String.class); // $ SSRF
             restTemplate.postForLocation(fooResourceUrl, new String("object")); // $ SSRF