hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-25 00:35:20 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge remote-tracking branch 'origin/main' into smowton/admin/merge-rc317-into-main

Browse Source
This commit is contained in:
Chris Smowton
2025-03-19 16:01:29 +00:00
parent 36869efeb5 179bae8791
commit 9a2a13ed55
2476 changed files with 45571 additions and 29067 deletions
Download Patch File Download Diff File Expand all files Collapse all files

23
java/ql/src/Likely Bugs/Concurrency/UnreleasedLock.ql
View File

@@ -118,6 +118,26 @@ predicate heldByCurrentThreadCheck(LockType t, BasicBlock checkblock, BasicBlock
)
}
/**
* Holds if there is a variable access in `checkblock` that has `falsesucc` as the false successor.
*
* The variable access must have an assigned value that is a lock access on `t`, and
* the true successor of `checkblock` must contain an unlock access.
*/
predicate variableLockStateCheck(LockType t, BasicBlock checkblock, BasicBlock falsesucc) {
exists(ConditionBlock conditionBlock, VarAccess v |
v.getType() instanceof BooleanType and
// Ensure that a lock access is assigned to the variable
v.getVariable().getAnAssignedValue() = t.getLockAccess() and
// Ensure that the `true` successor of the condition block contains an unlock access
conditionBlock.getTestSuccessor(true) = t.getUnlockAccess().getBasicBlock() and
conditionBlock.getCondition() = v
|
conditionBlock.getBasicBlock() = checkblock and
conditionBlock.getTestSuccessor(false) = falsesucc
)
}
/**
* A control flow path from a locking call in `src` to `b` such that the number of
* locks minus the number of unlocks along the way is positive and equal to `locks`.
@@ -131,8 +151,9 @@ predicate blockIsLocked(LockType t, BasicBlock src, BasicBlock b, int locks) {
// The number of net locks from the `src` block to the predecessor block `pred` is `predlocks`.
blockIsLocked(t, src, pred, predlocks) and
// The recursive call ensures that at least one lock is held, so do not consider the false
// successor of the `isHeldByCurrentThread()` check.
// successor of the `isHeldByCurrentThread()` check or of `variableLockStateCheck`.
not heldByCurrentThreadCheck(t, pred, b) and
not variableLockStateCheck(t, pred, b) and
// Count a failed lock as an unlock so the net is zero.
(if failedLock(t, pred, b) then failedlock = 1 else failedlock = 0) and
(

25
java/ql/src/Security/CWE/CWE-200/SpringBootActuators.java Normal file
View File

@@ -0,0 +1,25 @@
@Configuration(proxyBeanMethods = false)
public class CustomSecurityConfiguration {
@Bean
public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception {
// BAD: Unauthenticated access to Spring Boot actuator endpoints is allowed
http.securityMatcher(EndpointRequest.toAnyEndpoint());
http.authorizeHttpRequests((requests) -> requests.anyRequest().permitAll());
return http.build();
}
}
@Configuration(proxyBeanMethods = false)
public class CustomSecurityConfiguration {
@Bean
public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception {
// GOOD: only users with ENDPOINT_ADMIN role are allowed to access the actuator endpoints
http.securityMatcher(EndpointRequest.toAnyEndpoint());
http.authorizeHttpRequests((requests) -> requests.anyRequest().hasRole("ENDPOINT_ADMIN"));
return http.build();
}
}

36
java/ql/src/Security/CWE/CWE-200/SpringBootActuators.qhelp Normal file
View File

@@ -0,0 +1,36 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>Spring Boot includes features called actuators that let you monitor and interact with your
web application. Exposing unprotected actuator endpoints can lead to information disclosure or
even to remote code execution.</p>
</overview>
<recommendation>
<p>Since actuator endpoints may contain sensitive information, carefully consider when to expose them,
and secure them as you would any sensitive URL. Actuators are secured by default when using Spring
Security without a custom configuration. If you wish to define a custom security configuration,
consider only allowing users with certain roles to access these endpoints.
</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 Reference Documentation:
<a href="https://docs.spring.io/spring-boot/reference/actuator/endpoints.html">Endpoints</a>.
</li>
</references>
</qhelp>

20
java/ql/src/Security/CWE/CWE-200/SpringBootActuators.ql Normal file
View File

@@ -0,0 +1,20 @@
/**
* @name Exposed Spring Boot actuators
* @description Exposing Spring Boot actuators may lead to information leak from the internal application,
* or even to remote code execution.
* @kind problem
* @problem.severity error
* @security-severity 6.5
* @precision high
* @id java/spring-boot-exposed-actuators
* @tags security
* external/cwe/cwe-200
*/
import java
import semmle.code.java.frameworks.spring.SpringSecurity
import semmle.code.java.security.SpringBootActuatorsQuery
from SpringPermitAllCall permitAllCall
where permitsSpringBootActuators(permitAllCall)
select permitAllCall, "Unauthenticated access to Spring Boot actuator is allowed."

22
java/ql/src/Security/CWE/CWE-367/TOCTOURace.ql
View File

@@ -54,30 +54,34 @@ class PossiblyConcurrentCallable extends Callable {
}
}
private VarAccess getANonInitializationAccess(Field f) {
result = f.getAnAccess() and
exists(Callable c | c = result.getEnclosingCallable() |
not (
c = f.getDeclaringType().getACallable() and
(c instanceof Constructor or c instanceof InitializerMethod)
)
)
}
/**
* Holds if all accesses to `v` (outside of initializers) are locked in the same way.
*/
predicate alwaysLocked(Field f) {
exists(Variable lock |
forex(VarAccess access |
access = f.getAnAccess() and not access.getEnclosingCallable() instanceof InitializerMethod
|
forex(VarAccess access | access = getANonInitializationAccess(f) |
locallySynchronizedOn(access, _, lock)
)
)
or
exists(RefType thisType |
forex(VarAccess access |
access = f.getAnAccess() and not access.getEnclosingCallable() instanceof InitializerMethod
|
forex(VarAccess access | access = getANonInitializationAccess(f) |
locallySynchronizedOnThis(access, thisType)
)
)
or
exists(RefType classType |
forex(VarAccess access |
access = f.getAnAccess() and not access.getEnclosingCallable() instanceof InitializerMethod
|
forex(VarAccess access | access = getANonInitializationAccess(f) |
locallySynchronizedOnClass(access, classType)
)
)

4
java/ql/src/change-notes/2025-02-24-spring-boot-actuators-promo.md Normal file
View File

@@ -0,0 +1,4 @@
---
category: newQuery
---
* The query `java/spring-boot-exposed-actuators` has been promoted from experimental to the main query pack. Its results will now appear by default, and the query itself will be removed from the [CodeQL Community Packs](https://github.com/GitHubSecurityLab/CodeQL-Community-Packs). This query was originally submitted as an experimental query [by @ggolawski](https://github.com/github/codeql/pull/2901).

4
java/ql/src/change-notes/2025-03-02-unreleased-lock-fp.md Normal file
View File

@@ -0,0 +1,4 @@
---
category: majorAnalysis
---
* Updated the `java/unreleased-lock` query so that it no longer report alerts in cases where a boolean variable is used to track lock state.

4
java/ql/src/change-notes/2025-03-03-fix-improper-intent-verification-query.md Normal file
View File

@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* Overrides of `BroadcastReceiver::onReceive` with no statements in their body are no longer considered unverified by the `java/improper-intent-verification` query. This will reduce false positives from `onReceive` methods which do not perform any actions.

4
java/ql/src/change-notes/2025-03-13-fix-toctou-false-positive.md Normal file
View File

@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* Fixed a false positive in "Time-of-check time-of-use race condition" (`java/toctou-race-condition`) where a field of a non-static class was not considered always-locked if it was accessed in a constructor.

22
java/ql/src/experimental/Security/CWE/CWE-016/SpringBootActuators.java
View File

@@ -1,22 +0,0 @@
@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();
}
}

39
java/ql/src/experimental/Security/CWE/CWE-016/SpringBootActuators.qhelp
View File

@@ -1,39 +0,0 @@
<!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>

20
java/ql/src/experimental/Security/CWE/CWE-016/SpringBootActuators.ql
View File

@@ -1,20 +0,0 @@
/**
* @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
* experimental
* external/cwe/cwe-16
*/
import java
deprecated import SpringBootActuators
deprecated query predicate problems(PermitAllCall permitAllCall, string message) {
permitAllCall.permitsSpringBootActuators() and
message = "Unauthenticated access to Spring Boot actuator is allowed."
}

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

@@ -1,157 +0,0 @@
deprecated module;
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`.
*/
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()`.
*/
class RequestMatcherCall extends MethodCall {
RequestMatcherCall() {
this.getMethod().hasName("requestMatcher") and
this.getMethod().getDeclaringType() instanceof TypeHttpSecurity and
this.getArgument(0) instanceof ToAnyEndpointCall
}
}
/**
* A call to `HttpSecurity.requestMatchers` method with lambda argument
* `RequestMatcher.toAnyEndpoint()`.
*/
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 {
RegistryRequestMatchersCall() {
this.getMethod().hasName("requestMatchers") and
this.getMethod().getDeclaringType() instanceof TypeAbstractRequestMatcherRegistry and
this.getAnArgument() instanceof ToAnyEndpointCall
}
}