Java: refactor QL, move code to libraries

github · jcogs33 · Mar 11, 2025 · Feb 16, 2025 · Feb 16, 2025 · Feb 16, 2025
commit 8dfb920e051c3573fe0ca52c429408985bbaa6a3
@@ -0,0 +1,24 @@
+/**
+ * Provides classes for working with Spring classes and interfaces from
+ * `org.springframework.boot.*`.
+ */
+
+import java
+
+/**
+ * The class `org.springframework.boot.actuate.autoconfigure.security.servlet.EndpointRequest`.
+ */
+class TypeEndpointRequest extends Class {
+  TypeEndpointRequest() {
+    this.hasQualifiedName("org.springframework.boot.actuate.autoconfigure.security.servlet",
+      "EndpointRequest")
+  }
+}
+
+/** A call to `EndpointRequest.toAnyEndpoint` method. */
+class ToAnyEndpointCall extends MethodCall {
+  ToAnyEndpointCall() {
+    this.getMethod().hasName("toAnyEndpoint") and
+    this.getMethod().getDeclaringType() instanceof TypeEndpointRequest
+  }
+}
@@ -0,0 +1,59 @@
+/**
+ * Provides classes for working with Spring classes and interfaces from
+ * `org.springframework.security.*`.
+ */
+
+import java
+
+/** The class `org.springframework.security.config.annotation.web.builders.HttpSecurity`. */
+class TypeHttpSecurity extends Class {
+  TypeHttpSecurity() {
+    this.hasQualifiedName("org.springframework.security.config.annotation.web.builders",
+      "HttpSecurity")
+  }
+}
+
+/**
+ * The class
+ * `org.springframework.security.config.annotation.web.configurers.ExpressionUrlAuthorizationConfigurer`.
+ */
+class TypeAuthorizedUrl extends Class {
+  TypeAuthorizedUrl() {
+    this.hasQualifiedName("org.springframework.security.config.annotation.web.configurers",
+      "ExpressionUrlAuthorizationConfigurer<HttpSecurity>$AuthorizedUrl<>")
+  }
+}
+
+/**
+ * The class `org.springframework.security.config.annotation.web.AbstractRequestMatcherRegistry`.
+ */
+class TypeAbstractRequestMatcherRegistry extends Class {
+  TypeAbstractRequestMatcherRegistry() {
+    this.hasQualifiedName("org.springframework.security.config.annotation.web",
+      "AbstractRequestMatcherRegistry<AuthorizedUrl<>>")
+  }
+}
+
+/** A call to `HttpSecurity.authorizeRequests` method. */
+class AuthorizeRequestsCall extends MethodCall {
+  AuthorizeRequestsCall() {
+    this.getMethod().hasName("authorizeRequests") and
+    this.getMethod().getDeclaringType() instanceof TypeHttpSecurity
+  }
+}
+
+/** A call to `AuthorizedUrl.permitAll` method. */
+class PermitAllCall extends MethodCall {
+  PermitAllCall() {
+    this.getMethod().hasName("permitAll") and
+    this.getMethod().getDeclaringType() instanceof TypeAuthorizedUrl
+  }
+}
+
+/** A call to `AbstractRequestMatcherRegistry.anyRequest` method. */
+class AnyRequestCall extends MethodCall {
+  AnyRequestCall() {
+    this.getMethod().hasName("anyRequest") and
+    this.getMethod().getDeclaringType() instanceof TypeAbstractRequestMatcherRegistry
+  }
+}
@@ -1,58 +1,14 @@
 /** Provides classes and predicates to reason about exposed actuators in Spring Boot. */
 
 import java
-
-/** The class `org.springframework.security.config.annotation.web.builders.HttpSecurity`. */
-class TypeHttpSecurity extends Class {
-  TypeHttpSecurity() {
-    this.hasQualifiedName("org.springframework.security.config.annotation.web.builders",
-      "HttpSecurity")
-  }
-}
-
-/**
- * The class
- * `org.springframework.security.config.annotation.web.configurers.ExpressionUrlAuthorizationConfigurer`.
- */
-class TypeAuthorizedUrl extends Class {
-  TypeAuthorizedUrl() {
-    this.hasQualifiedName("org.springframework.security.config.annotation.web.configurers",
-      "ExpressionUrlAuthorizationConfigurer<HttpSecurity>$AuthorizedUrl<>")
-  }
-}
+private import semmle.code.java.frameworks.spring.SpringSecurity
+private import semmle.code.java.frameworks.spring.SpringBoot
 
 /**
- * The class `org.springframework.security.config.annotation.web.AbstractRequestMatcherRegistry`.
- */
-class TypeAbstractRequestMatcherRegistry extends Class {
-  TypeAbstractRequestMatcherRegistry() {
-    this.hasQualifiedName("org.springframework.security.config.annotation.web",
-      "AbstractRequestMatcherRegistry<AuthorizedUrl<>>")
-  }
-}
-
-/**
- * The class `org.springframework.boot.actuate.autoconfigure.security.servlet.EndpointRequest`.
- */
-class TypeEndpointRequest extends Class {
-  TypeEndpointRequest() {
-    this.hasQualifiedName("org.springframework.boot.actuate.autoconfigure.security.servlet",
-      "EndpointRequest")
-  }
-}
-
-/** A call to `EndpointRequest.toAnyEndpoint` method. */
-class ToAnyEndpointCall extends MethodCall {
-  ToAnyEndpointCall() {
-    this.getMethod().hasName("toAnyEndpoint") and
-    this.getMethod().getDeclaringType() instanceof TypeEndpointRequest
-  }
-}
-
-/**
- * A call to `HttpSecurity.requestMatcher` method with argument `RequestMatcher.toAnyEndpoint()`.
+ * A call to `HttpSecurity.requestMatcher` method with argument
+ * `RequestMatcher.toAnyEndpoint()`.
  */
-class RequestMatcherCall extends MethodCall {
+private class RequestMatcherCall extends MethodCall {
   RequestMatcherCall() {
     this.getMethod().hasName("requestMatcher") and
     this.getMethod().getDeclaringType() instanceof TypeHttpSecurity and
@@ -64,94 +20,70 @@ class RequestMatcherCall extends MethodCall {
  * A call to `HttpSecurity.requestMatchers` method with lambda argument
  * `RequestMatcher.toAnyEndpoint()`.
  */
-class RequestMatchersCall extends MethodCall {
+private class RequestMatchersCall extends MethodCall {
   RequestMatchersCall() {
     this.getMethod().hasName("requestMatchers") and
     this.getMethod().getDeclaringType() instanceof TypeHttpSecurity and
     this.getArgument(0).(LambdaExpr).getExprBody() instanceof ToAnyEndpointCall
   }
 }
 
-/** A call to `HttpSecurity.authorizeRequests` method. */
-class AuthorizeRequestsCall extends MethodCall {
-  AuthorizeRequestsCall() {
-    this.getMethod().hasName("authorizeRequests") and
-    this.getMethod().getDeclaringType() instanceof TypeHttpSecurity
-  }
-}
-
-/** A call to `AuthorizedUrl.permitAll` method. */
-class PermitAllCall extends MethodCall {
-  PermitAllCall() {
-    this.getMethod().hasName("permitAll") and
-    this.getMethod().getDeclaringType() instanceof TypeAuthorizedUrl
-  }
-
-  /** Holds if `permitAll` is called on request(s) mapped to actuator endpoint(s). */
-  predicate permitsSpringBootActuators() {
-    exists(AuthorizeRequestsCall authorizeRequestsCall |
-      // .requestMatcher(EndpointRequest).authorizeRequests([...]).[...]
-      authorizeRequestsCall.getQualifier() instanceof RequestMatcherCall
-      or
-      // .requestMatchers(matcher -> EndpointRequest).authorizeRequests([...]).[...]
-      authorizeRequestsCall.getQualifier() instanceof RequestMatchersCall
-    |
-      // [...].authorizeRequests(r -> r.anyRequest().permitAll()) or
-      // [...].authorizeRequests(r -> r.requestMatchers(EndpointRequest).permitAll())
-      authorizeRequestsCall.getArgument(0).(LambdaExpr).getExprBody() = this and
-      (
-        this.getQualifier() instanceof AnyRequestCall or
-        this.getQualifier() instanceof RegistryRequestMatchersCall
-      )
-      or
-      // [...].authorizeRequests().requestMatchers(EndpointRequest).permitAll() or
-      // [...].authorizeRequests().anyRequest().permitAll()
-      authorizeRequestsCall.getNumArgument() = 0 and
-      exists(RegistryRequestMatchersCall registryRequestMatchersCall |
-        registryRequestMatchersCall.getQualifier() = authorizeRequestsCall and
-        this.getQualifier() = registryRequestMatchersCall
-      )
-      or
-      exists(AnyRequestCall anyRequestCall |
-        anyRequestCall.getQualifier() = authorizeRequestsCall and
-        this.getQualifier() = anyRequestCall
-      )
-    )
-    or
-    exists(AuthorizeRequestsCall authorizeRequestsCall |
-      // http.authorizeRequests([...]).[...]
-      authorizeRequestsCall.getQualifier() instanceof VarAccess
-    |
-      // [...].authorizeRequests(r -> r.requestMatchers(EndpointRequest).permitAll())
-      authorizeRequestsCall.getArgument(0).(LambdaExpr).getExprBody() = this and
-      this.getQualifier() instanceof RegistryRequestMatchersCall
-      or
-      // [...].authorizeRequests().requestMatchers(EndpointRequest).permitAll() or
-      authorizeRequestsCall.getNumArgument() = 0 and
-      exists(RegistryRequestMatchersCall registryRequestMatchersCall |
-        registryRequestMatchersCall.getQualifier() = authorizeRequestsCall and
-        this.getQualifier() = registryRequestMatchersCall
-      )
-    )
-  }
-}
-
-/** A call to `AbstractRequestMatcherRegistry.anyRequest` method. */
-class AnyRequestCall extends MethodCall {
-  AnyRequestCall() {
-    this.getMethod().hasName("anyRequest") and
-    this.getMethod().getDeclaringType() instanceof TypeAbstractRequestMatcherRegistry
-  }
-}
-
 /**
  * A call to `AbstractRequestMatcherRegistry.requestMatchers` method with an argument
  * `RequestMatcher.toAnyEndpoint()`.
  */
-class RegistryRequestMatchersCall extends MethodCall {
+private class RegistryRequestMatchersCall extends MethodCall {
   RegistryRequestMatchersCall() {
     this.getMethod().hasName("requestMatchers") and
     this.getMethod().getDeclaringType() instanceof TypeAbstractRequestMatcherRegistry and
     this.getAnArgument() instanceof ToAnyEndpointCall
   }
 }
+
+/** Holds if `permitAllCall` is called on request(s) mapped to actuator endpoint(s). */
+predicate permitsSpringBootActuators(PermitAllCall permitAllCall) {
+  exists(AuthorizeRequestsCall authorizeRequestsCall |
+    // .requestMatcher(EndpointRequest).authorizeRequests([...]).[...]
+    authorizeRequestsCall.getQualifier() instanceof RequestMatcherCall
+    or
+    // .requestMatchers(matcher -> EndpointRequest).authorizeRequests([...]).[...]
+    authorizeRequestsCall.getQualifier() instanceof RequestMatchersCall
+  |
+    // [...].authorizeRequests(r -> r.anyRequest().permitAll()) or
+    // [...].authorizeRequests(r -> r.requestMatchers(EndpointRequest).permitAll())
+    authorizeRequestsCall.getArgument(0).(LambdaExpr).getExprBody() = permitAllCall and
+    (
+      permitAllCall.getQualifier() instanceof AnyRequestCall or
+      permitAllCall.getQualifier() instanceof RegistryRequestMatchersCall
+    )
+    or
+    // [...].authorizeRequests().requestMatchers(EndpointRequest).permitAll() or
+    // [...].authorizeRequests().anyRequest().permitAll()
+    authorizeRequestsCall.getNumArgument() = 0 and
+    exists(RegistryRequestMatchersCall registryRequestMatchersCall |
+      registryRequestMatchersCall.getQualifier() = authorizeRequestsCall and
+      permitAllCall.getQualifier() = registryRequestMatchersCall
+    )
+    or
+    exists(AnyRequestCall anyRequestCall |
+      anyRequestCall.getQualifier() = authorizeRequestsCall and
+      permitAllCall.getQualifier() = anyRequestCall
+    )
+  )
+  or
+  exists(AuthorizeRequestsCall authorizeRequestsCall |
+    // http.authorizeRequests([...]).[...]
+    authorizeRequestsCall.getQualifier() instanceof VarAccess
+  |
+    // [...].authorizeRequests(r -> r.requestMatchers(EndpointRequest).permitAll())
+    authorizeRequestsCall.getArgument(0).(LambdaExpr).getExprBody() = permitAllCall and
+    permitAllCall.getQualifier() instanceof RegistryRequestMatchersCall
+    or
+    // [...].authorizeRequests().requestMatchers(EndpointRequest).permitAll() or
+    authorizeRequestsCall.getNumArgument() = 0 and
+    exists(RegistryRequestMatchersCall registryRequestMatchersCall |
+      registryRequestMatchersCall.getQualifier() = authorizeRequestsCall and
+      permitAllCall.getQualifier() = registryRequestMatchersCall
+    )
+  )
+}
diff --git a/java/ql/src/Security/CWE/CWE-016/SpringBootActuators.ql b/java/ql/src/Security/CWE/CWE-016/SpringBootActuators.ql
@@ -12,8 +12,9 @@
  */
 
 import java
+import semmle.code.java.frameworks.spring.SpringSecurity
 import semmle.code.java.security.SpringBootActuatorsQuery
 
 from PermitAllCall permitAllCall
-where permitAllCall.permitsSpringBootActuators()
+where permitsSpringBootActuators(permitAllCall)
 select permitAllCall, "Unauthenticated access to Spring Boot actuator is allowed."
diff --git a/java/ql/test/query-tests/security/CWE-016/SpringBootActuatorsTest.ql b/java/ql/test/query-tests/security/CWE-016/SpringBootActuatorsTest.ql
@@ -1,4 +1,5 @@
 import java
+import semmle.code.java.frameworks.spring.SpringSecurity
 import semmle.code.java.security.SpringBootActuatorsQuery
 import utils.test.InlineExpectationsTest
 
@@ -7,7 +8,7 @@ module SpringBootActuatorsTest implements TestSig {
 
   predicate hasActualResult(Location location, string element, string tag, string value) {
     tag = "hasExposedSpringBootActuator" and
-    exists(PermitAllCall permitAllCall | permitAllCall.permitsSpringBootActuators() |
+    exists(PermitAllCall permitAllCall | permitsSpringBootActuators(permitAllCall) |
       permitAllCall.getLocation() = location and
       element = permitAllCall.toString() and
       value = ""