Skip to content

CodeQL query to detect open Spring Boot actuator endpoints #2901

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Apr 29, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
@Configuration(proxyBeanMethods = false)
public class SpringBootActuators extends WebSecurityConfigurerAdapter {

@Override
protected void configure(HttpSecurity http) throws Exception {
// BAD: Unauthenticated access to Spring Boot actuator endpoints is allowed
http.requestMatcher(EndpointRequest.toAnyEndpoint()).authorizeRequests((requests) ->
requests.anyRequest().permitAll());
}
}

@Configuration(proxyBeanMethods = false)
public class ActuatorSecurity extends WebSecurityConfigurerAdapter {

@Override
protected void configure(HttpSecurity http) throws Exception {
// GOOD: only users with ENDPOINT_ADMIN role are allowed to access the actuator endpoints
http.requestMatcher(EndpointRequest.toAnyEndpoint()).authorizeRequests((requests) ->
requests.anyRequest().hasRole("ENDPOINT_ADMIN"));
http.httpBasic();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>Spring Boot includes a number of additional features called actuators that let you monitor
and interact with your web application. Exposing unprotected actuator endpoints via JXM or HTTP
can, however, lead to information disclosure or even to remote code execution vulnerability.</p>
</overview>

<recommendation>
<p>Since actuator endpoints may contain sensitive information, careful consideration should be
given about when to expose them. You should take care to secure exposed HTTP endpoints in the same
way that you would any other sensitive URL. If Spring Security is present, endpoints are secured by
default using Spring Security’s content-negotiation strategy. If you wish to configure custom
security for HTTP endpoints, for example, only allow users with a certain role to access them,
Spring Boot provides some convenient <code>RequestMatcher</code> objects that can be used in
combination with Spring Security.</p>
</recommendation>

<example>
<p>In the first example, the custom security configuration allows unauthenticated access to all
actuator endpoints. This may lead to sensitive information disclosure and should be avoided.</p>
<p>In the second example, only users with <code>ENDPOINT_ADMIN</code> role are allowed to access
the actuator endpoints.</p>

<sample src="SpringBootActuators.java" />
</example>

<references>
<li>
Spring Boot documentation:
<a href="https://docs.spring.io/spring-boot/docs/current/reference/html/production-ready-features.html">Actuators</a>.
</li>
<li>
<a href="https://www.veracode.com/blog/research/exploiting-spring-boot-actuators">Exploiting Spring Boot Actuators</a>
</li>
</references>
</qhelp>
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/**
* @name Exposed Spring Boot actuators
* @description Exposing Spring Boot actuators may lead to internal application's information leak
* or even to remote code execution.
* @kind problem
* @problem.severity error
* @precision high
* @id java/spring-boot-exposed-actuators
* @tags security
* external/cwe/cwe-16
*/

import java
import SpringBootActuators

from PermitAllCall permitAllCall
where permitAllCall.permitsSpringBootActuators()
select permitAllCall, "Unauthenticated access to Spring Boot actuator is allowed."
141 changes: 141 additions & 0 deletions java/ql/src/experimental/Security/CWE/CWE-016/SpringBootActuators.qll
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
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<>>")
}
}

/**
* The class
* `org.springframework.boot.actuate.autoconfigure.security.servlet.EndpointRequest.EndpointRequestMatcher`.
*/
class TypeEndpointRequestMatcher extends Class {
TypeEndpointRequestMatcher() {
this
.hasQualifiedName("org.springframework.boot.actuate.autoconfigure.security.servlet",
"EndpointRequest$EndpointRequestMatcher")
}
}

/**
* A call to `HttpSecurity.requestMatcher` method with argument of type
* `EndpointRequestMatcher`.
*/
class RequestMatcherCall extends MethodAccess {
RequestMatcherCall() {
getMethod().hasName("requestMatcher") and
getMethod().getDeclaringType() instanceof TypeHttpSecurity and
getArgument(0).getType() instanceof TypeEndpointRequestMatcher
}
}

/**
* A call to `HttpSecurity.requestMatchers` method with lambda argument resolving to
* `EndpointRequestMatcher` type.
*/
class RequestMatchersCall extends MethodAccess {
RequestMatchersCall() {
getMethod().hasName("requestMatchers") and
getMethod().getDeclaringType() instanceof TypeHttpSecurity and
getArgument(0).(LambdaExpr).getExprBody().getType() instanceof TypeEndpointRequestMatcher
}
}

/** A call to `HttpSecurity.authorizeRequests` method. */
class AuthorizeRequestsCall extends MethodAccess {
AuthorizeRequestsCall() {
getMethod().hasName("authorizeRequests") and
getMethod().getDeclaringType() instanceof TypeHttpSecurity
}
}

/** A call to `AuthorizedUrl.permitAll` method. */
class PermitAllCall extends MethodAccess {
PermitAllCall() {
getMethod().hasName("permitAll") and
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
or
// http.authorizeRequests([...]).[...]
authorizeRequestsCall.getQualifier() instanceof VarAccess
Comment on lines +88 to +97
Copy link
Contributor

@pwntester pwntester May 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ggolawski this clause seems to be causing many FPs where there are no evidences that Spring Actuators are being used.

See results here https://lgtm.com/query/4456298315122374638/

If EndpointRequest is not used, we need a different way to know that Actuators are indeed enabled.

Having said that, those results may be of interest for a different query in the line of https://vulncat.fortify.com/en/weakness?q=spring%20security%20misconfiguration

Also, for lines 91 and 93 there are some FPs in the form of:

http.authorizeRequests().requestMatchers(EndpointRequest.to("info", "health")).permitAll();

which could be solved by checking that TypeEndpointRequestMatcher matches only EndpointRequestMatcher returned by toAnyEndpoint() or by to() if arguments contains an edpoint other than health and info which are of no interest and public

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for spotting this. My intention was to raise the flag only if EndpointRequest.toAnyEndpoint() is being used. I'll check what's wrong and correct this query.

|
// [...].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
)
)
}
}

/** A call to `AbstractRequestMatcherRegistry.anyRequest` method. */
class AnyRequestCall extends MethodAccess {
AnyRequestCall() {
getMethod().hasName("anyRequest") and
getMethod().getDeclaringType() instanceof TypeAbstractRequestMatcherRegistry
}
}

/**
* A call to `AbstractRequestMatcherRegistry.requestMatchers` method with an argument of type
* `EndpointRequestMatcher`.
*/
class RegistryRequestMatchersCall extends MethodAccess {
RegistryRequestMatchersCall() {
getMethod().hasName("requestMatchers") and
getMethod().getDeclaringType() instanceof TypeAbstractRequestMatcherRegistry and
getAnArgument().getType() instanceof TypeEndpointRequestMatcher
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
| SpringBootActuators.java:6:88:6:120 | permitAll(...) | Unauthenticated access to Spring Boot actuator is allowed. |
| SpringBootActuators.java:10:5:10:137 | permitAll(...) | Unauthenticated access to Spring Boot actuator is allowed. |
| SpringBootActuators.java:14:5:14:149 | permitAll(...) | Unauthenticated access to Spring Boot actuator is allowed. |
| SpringBootActuators.java:18:5:18:101 | permitAll(...) | Unauthenticated access to Spring Boot actuator is allowed. |
| SpringBootActuators.java:22:5:22:89 | permitAll(...) | Unauthenticated access to Spring Boot actuator is allowed. |
| SpringBootActuators.java:26:40:26:108 | permitAll(...) | Unauthenticated access to Spring Boot actuator is allowed. |
| SpringBootActuators.java:30:5:30:113 | permitAll(...) | Unauthenticated access to Spring Boot actuator is allowed. |
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import org.springframework.boot.actuate.autoconfigure.security.servlet.EndpointRequest;
import org.springframework.security.config.annotation.web.builders.HttpSecurity;

public class SpringBootActuators {
protected void configure(HttpSecurity http) throws Exception {
http.requestMatcher(EndpointRequest.toAnyEndpoint()).authorizeRequests(requests -> requests.anyRequest().permitAll());
}

protected void configure2(HttpSecurity http) throws Exception {
http.requestMatcher(EndpointRequest.toAnyEndpoint()).authorizeRequests().requestMatchers(EndpointRequest.toAnyEndpoint()).permitAll();
}

protected void configure3(HttpSecurity http) throws Exception {
http.requestMatchers(matcher -> EndpointRequest.toAnyEndpoint()).authorizeRequests().requestMatchers(EndpointRequest.toAnyEndpoint()).permitAll();
}

protected void configure4(HttpSecurity http) throws Exception {
http.requestMatcher(EndpointRequest.toAnyEndpoint()).authorizeRequests().anyRequest().permitAll();
}

protected void configure5(HttpSecurity http) throws Exception {
http.authorizeRequests().requestMatchers(EndpointRequest.toAnyEndpoint()).permitAll();
}

protected void configure6(HttpSecurity http) throws Exception {
http.authorizeRequests(requests -> requests.requestMatchers(EndpointRequest.toAnyEndpoint()).permitAll());
}

protected void configure7(HttpSecurity http) throws Exception {
http.requestMatchers(matcher -> EndpointRequest.toAnyEndpoint()).authorizeRequests().anyRequest().permitAll();
}

protected void configureOk1(HttpSecurity http) throws Exception {
http.requestMatcher(EndpointRequest.toAnyEndpoint());
}

protected void configureOk2(HttpSecurity http) throws Exception {
http.requestMatchers().requestMatchers(EndpointRequest.toAnyEndpoint());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
experimental/Security/CWE/CWE-016/SpringBootActuators.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
//semmle-extractor-options: --javac-args -cp ${testdir}/../../../stubs/springframework-5.2.3
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package org.springframework.beans.factory;

public interface BeanFactory {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package org.springframework.beans.factory;

public interface HierarchicalBeanFactory extends BeanFactory {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package org.springframework.beans.factory;

public interface ListableBeanFactory extends BeanFactory {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package org.springframework.boot.actuate.autoconfigure.security.servlet;

import org.springframework.boot.security.servlet.ApplicationContextRequestMatcher;
import org.springframework.web.context.WebApplicationContext;

public final class EndpointRequest {
public static EndpointRequestMatcher toAnyEndpoint() {
return null;
}

public static final class EndpointRequestMatcher extends AbstractRequestMatcher {}

private abstract static class AbstractRequestMatcher
extends ApplicationContextRequestMatcher<WebApplicationContext> {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package org.springframework.boot.security.servlet;

import org.springframework.security.web.util.matcher.RequestMatcher;

public abstract class ApplicationContextRequestMatcher<C> implements RequestMatcher {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package org.springframework.context;

import org.springframework.beans.factory.HierarchicalBeanFactory;
import org.springframework.beans.factory.ListableBeanFactory;
import org.springframework.core.env.EnvironmentCapable;
import org.springframework.core.io.support.ResourcePatternResolver;

public interface ApplicationContext extends EnvironmentCapable, ListableBeanFactory, HierarchicalBeanFactory,
MessageSource, ApplicationEventPublisher, ResourcePatternResolver {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package org.springframework.context;

@FunctionalInterface
public interface ApplicationEventPublisher {
void publishEvent(Object event);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package org.springframework.context;

public interface MessageSource {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package org.springframework.core.env;

public interface EnvironmentCapable {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package org.springframework.core.io;

public interface ResourceLoader {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package org.springframework.core.io.support;

import org.springframework.core.io.ResourceLoader;

public interface ResourcePatternResolver extends ResourceLoader {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package org.springframework.security.config;

@FunctionalInterface
public interface Customizer<T> {
void customize(T t);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
package org.springframework.security.config.annotation;

public abstract class AbstractConfiguredSecurityBuilder<O, B extends SecurityBuilder<O>>
extends AbstractSecurityBuilder<O> {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package org.springframework.security.config.annotation;

public abstract class AbstractSecurityBuilder<O> implements SecurityBuilder<O> {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package org.springframework.security.config.annotation;

public interface SecurityBuilder<O> {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package org.springframework.security.config.annotation;

public interface SecurityConfigurer<O, B extends SecurityBuilder<O>> {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
package org.springframework.security.config.annotation;

public abstract class SecurityConfigurerAdapter<O, B extends SecurityBuilder<O>>
implements SecurityConfigurer<O, B> {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package org.springframework.security.config.annotation.web;

import org.springframework.security.web.util.matcher.RequestMatcher;

public abstract class AbstractRequestMatcherRegistry<C> {
public C anyRequest() {
return null;
}

public C requestMatchers(RequestMatcher... requestMatchers) {
return null;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package org.springframework.security.config.annotation.web;

import org.springframework.security.config.annotation.SecurityBuilder;
import org.springframework.security.web.DefaultSecurityFilterChain;

public interface HttpSecurityBuilder<H extends HttpSecurityBuilder<H>> extends
SecurityBuilder<DefaultSecurityFilterChain> {}
Loading