Skip to content

Commit 0d75c6a

Browse files
authored
Merge pull request #3506 from ggolawski/spring-actuators-fix
Fixes FPs in SpringBootActuators query
2 parents ac1a338 + ac329e8 commit 0d75c6a

File tree

3 files changed

+105
-19
lines changed

3 files changed

+105
-19
lines changed

java/ql/src/experimental/Security/CWE/CWE-016/SpringBootActuators.qll

Lines changed: 37 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,7 @@ class TypeAuthorizedUrl extends Class {
2222
}
2323

2424
/**
25-
* The class
26-
* `org.springframework.security.config.annotation.web.AbstractRequestMatcherRegistry`.
25+
* The class `org.springframework.security.config.annotation.web.AbstractRequestMatcherRegistry`.
2726
*/
2827
class TypeAbstractRequestMatcherRegistry extends Class {
2928
TypeAbstractRequestMatcherRegistry() {
@@ -34,38 +33,44 @@ class TypeAbstractRequestMatcherRegistry extends Class {
3433
}
3534

3635
/**
37-
* The class
38-
* `org.springframework.boot.actuate.autoconfigure.security.servlet.EndpointRequest.EndpointRequestMatcher`.
36+
* The class `org.springframework.boot.actuate.autoconfigure.security.servlet.EndpointRequest`.
3937
*/
40-
class TypeEndpointRequestMatcher extends Class {
41-
TypeEndpointRequestMatcher() {
38+
class TypeEndpointRequest extends Class {
39+
TypeEndpointRequest() {
4240
this
4341
.hasQualifiedName("org.springframework.boot.actuate.autoconfigure.security.servlet",
44-
"EndpointRequest$EndpointRequestMatcher")
42+
"EndpointRequest")
43+
}
44+
}
45+
46+
/** A call to `EndpointRequest.toAnyEndpoint` method. */
47+
class ToAnyEndpointCall extends MethodAccess {
48+
ToAnyEndpointCall() {
49+
getMethod().hasName("toAnyEndpoint") and
50+
getMethod().getDeclaringType() instanceof TypeEndpointRequest
4551
}
4652
}
4753

4854
/**
49-
* A call to `HttpSecurity.requestMatcher` method with argument of type
50-
* `EndpointRequestMatcher`.
55+
* A call to `HttpSecurity.requestMatcher` method with argument `RequestMatcher.toAnyEndpoint()`.
5156
*/
5257
class RequestMatcherCall extends MethodAccess {
5358
RequestMatcherCall() {
5459
getMethod().hasName("requestMatcher") and
5560
getMethod().getDeclaringType() instanceof TypeHttpSecurity and
56-
getArgument(0).getType() instanceof TypeEndpointRequestMatcher
61+
getArgument(0) instanceof ToAnyEndpointCall
5762
}
5863
}
5964

6065
/**
61-
* A call to `HttpSecurity.requestMatchers` method with lambda argument resolving to
62-
* `EndpointRequestMatcher` type.
66+
* A call to `HttpSecurity.requestMatchers` method with lambda argument
67+
* `RequestMatcher.toAnyEndpoint()`.
6368
*/
6469
class RequestMatchersCall extends MethodAccess {
6570
RequestMatchersCall() {
6671
getMethod().hasName("requestMatchers") and
6772
getMethod().getDeclaringType() instanceof TypeHttpSecurity and
68-
getArgument(0).(LambdaExpr).getExprBody().getType() instanceof TypeEndpointRequestMatcher
73+
getArgument(0).(LambdaExpr).getExprBody() instanceof ToAnyEndpointCall
6974
}
7075
}
7176

@@ -92,9 +97,6 @@ class PermitAllCall extends MethodAccess {
9297
or
9398
// .requestMatchers(matcher -> EndpointRequest).authorizeRequests([...]).[...]
9499
authorizeRequestsCall.getQualifier() instanceof RequestMatchersCall
95-
or
96-
// http.authorizeRequests([...]).[...]
97-
authorizeRequestsCall.getQualifier() instanceof VarAccess
98100
|
99101
// [...].authorizeRequests(r -> r.anyRequest().permitAll()) or
100102
// [...].authorizeRequests(r -> r.requestMatchers(EndpointRequest).permitAll())
@@ -117,6 +119,22 @@ class PermitAllCall extends MethodAccess {
117119
this.getQualifier() = anyRequestCall
118120
)
119121
)
122+
or
123+
exists(AuthorizeRequestsCall authorizeRequestsCall |
124+
// http.authorizeRequests([...]).[...]
125+
authorizeRequestsCall.getQualifier() instanceof VarAccess
126+
|
127+
// [...].authorizeRequests(r -> r.requestMatchers(EndpointRequest).permitAll())
128+
authorizeRequestsCall.getArgument(0).(LambdaExpr).getExprBody() = this and
129+
this.getQualifier() instanceof RegistryRequestMatchersCall
130+
or
131+
// [...].authorizeRequests().requestMatchers(EndpointRequest).permitAll() or
132+
authorizeRequestsCall.getNumArgument() = 0 and
133+
exists(RegistryRequestMatchersCall registryRequestMatchersCall |
134+
registryRequestMatchersCall.getQualifier() = authorizeRequestsCall and
135+
this.getQualifier() = registryRequestMatchersCall
136+
)
137+
)
120138
}
121139
}
122140

@@ -129,13 +147,13 @@ class AnyRequestCall extends MethodAccess {
129147
}
130148

131149
/**
132-
* A call to `AbstractRequestMatcherRegistry.requestMatchers` method with an argument of type
133-
* `EndpointRequestMatcher`.
150+
* A call to `AbstractRequestMatcherRegistry.requestMatchers` method with an argument
151+
* `RequestMatcher.toAnyEndpoint()`.
134152
*/
135153
class RegistryRequestMatchersCall extends MethodAccess {
136154
RegistryRequestMatchersCall() {
137155
getMethod().hasName("requestMatchers") and
138156
getMethod().getDeclaringType() instanceof TypeAbstractRequestMatcherRegistry and
139-
getAnArgument().getType() instanceof TypeEndpointRequestMatcher
157+
getAnArgument() instanceof ToAnyEndpointCall
140158
}
141159
}

java/ql/test/experimental/query-tests/security/CWE-016/SpringBootActuators.java

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,4 +37,68 @@ protected void configureOk1(HttpSecurity http) throws Exception {
3737
protected void configureOk2(HttpSecurity http) throws Exception {
3838
http.requestMatchers().requestMatchers(EndpointRequest.toAnyEndpoint());
3939
}
40+
41+
protected void configureOk3(HttpSecurity http) throws Exception {
42+
http.authorizeRequests().anyRequest().permitAll();
43+
}
44+
45+
protected void configureOk4(HttpSecurity http) throws Exception {
46+
http.authorizeRequests(authz -> authz.anyRequest().permitAll());
47+
}
48+
49+
protected void configureOkSafeEndpoints1(HttpSecurity http) throws Exception {
50+
http.requestMatcher(EndpointRequest.to("health", "info")).authorizeRequests(requests -> requests.anyRequest().permitAll());
51+
}
52+
53+
protected void configureOkSafeEndpoints2(HttpSecurity http) throws Exception {
54+
http.requestMatcher(EndpointRequest.to("health")).authorizeRequests().requestMatchers(EndpointRequest.to("health")).permitAll();
55+
}
56+
57+
protected void configureOkSafeEndpoints3(HttpSecurity http) throws Exception {
58+
http.requestMatchers(matcher -> EndpointRequest.to("health", "info")).authorizeRequests().requestMatchers(EndpointRequest.to("health", "info")).permitAll();
59+
}
60+
61+
protected void configureOkSafeEndpoints4(HttpSecurity http) throws Exception {
62+
http.requestMatcher(EndpointRequest.to("health", "info")).authorizeRequests().anyRequest().permitAll();
63+
}
64+
65+
protected void configureOkSafeEndpoints5(HttpSecurity http) throws Exception {
66+
http.authorizeRequests().requestMatchers(EndpointRequest.to("health", "info")).permitAll();
67+
}
68+
69+
protected void configureOkSafeEndpoints6(HttpSecurity http) throws Exception {
70+
http.authorizeRequests(requests -> requests.requestMatchers(EndpointRequest.to("health", "info")).permitAll());
71+
}
72+
73+
protected void configureOkSafeEndpoints7(HttpSecurity http) throws Exception {
74+
http.requestMatchers(matcher -> EndpointRequest.to("health", "info")).authorizeRequests().anyRequest().permitAll();
75+
}
76+
77+
protected void configureOkNoPermitAll1(HttpSecurity http) throws Exception {
78+
http.requestMatcher(EndpointRequest.toAnyEndpoint()).authorizeRequests(requests -> requests.anyRequest());
79+
}
80+
81+
protected void configureOkNoPermitAll2(HttpSecurity http) throws Exception {
82+
http.requestMatcher(EndpointRequest.toAnyEndpoint()).authorizeRequests().requestMatchers(EndpointRequest.toAnyEndpoint());
83+
}
84+
85+
protected void configureOkNoPermitAll3(HttpSecurity http) throws Exception {
86+
http.requestMatchers(matcher -> EndpointRequest.toAnyEndpoint()).authorizeRequests().requestMatchers(EndpointRequest.toAnyEndpoint());
87+
}
88+
89+
protected void configureOkNoPermitAll4(HttpSecurity http) throws Exception {
90+
http.requestMatcher(EndpointRequest.toAnyEndpoint()).authorizeRequests().anyRequest();
91+
}
92+
93+
protected void configureOkNoPermitAll5(HttpSecurity http) throws Exception {
94+
http.authorizeRequests().requestMatchers(EndpointRequest.toAnyEndpoint());
95+
}
96+
97+
protected void configureOkNoPermitAll6(HttpSecurity http) throws Exception {
98+
http.authorizeRequests(requests -> requests.requestMatchers(EndpointRequest.toAnyEndpoint()));
99+
}
100+
101+
protected void configureOkNoPermitAll7(HttpSecurity http) throws Exception {
102+
http.requestMatchers(matcher -> EndpointRequest.toAnyEndpoint()).authorizeRequests().anyRequest();
103+
}
40104
}

java/ql/test/experimental/stubs/springframework-5.2.3/org/springframework/boot/actuate/autoconfigure/security/servlet/EndpointRequest.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ public final class EndpointRequest {
77
public static EndpointRequestMatcher toAnyEndpoint() {
88
return null;
99
}
10+
11+
public static EndpointRequestMatcher to(String... endpoints) {
12+
return null;
13+
}
1014

1115
public static final class EndpointRequestMatcher extends AbstractRequestMatcher {}
1216

0 commit comments

Comments
 (0)