diff --git a/java/ql/src/experimental/Security/CWE/CWE-016/InsecureSpringActuatorConfig.qhelp b/java/ql/src/experimental/Security/CWE/CWE-016/InsecureSpringActuatorConfig.qhelp new file mode 100644 index 000000000000..e201156728a4 --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-016/InsecureSpringActuatorConfig.qhelp @@ -0,0 +1,47 @@ + + + +

Spring Boot is a popular framework that facilitates the development of stand-alone applications +and micro services. Spring Boot Actuator helps to expose production-ready support features against +Spring Boot applications.

+ +

Endpoints of Spring Boot Actuator allow to monitor and interact with a Spring Boot application. +Exposing unprotected actuator endpoints through configuration files can lead to information disclosure +or even remote code execution vulnerability.

+ +

Rather than programmatically permitting endpoint requests or enforcing access control, frequently +developers simply leave management endpoints publicly accessible in the application configuration file +application.properties without enforcing access control through Spring Security.

+
+ + +

Declare the Spring Boot Starter Security module in XML configuration or programmatically enforce +security checks on management endpoints using Spring Security. Otherwise accessing management endpoints +on a different HTTP port other than the port that the web application is listening on also helps to +improve the security.

+
+ + +

The following examples show both 'BAD' and 'GOOD' configurations. In the 'BAD' configuration, +no security module is declared and sensitive management endpoints are exposed. In the 'GOOD' configuration, +security is enforced and only endpoints requiring exposure are exposed.

+ + + +
+ + +
  • + Spring Boot documentation: + Spring Boot Actuator: Production-ready Features +
  • +
  • + VERACODE Blog: + Exploiting Spring Boot Actuators +
  • +
  • + HackerOne Report: + Spring Actuator endpoints publicly available, leading to account takeover +
  • +
    +
    diff --git a/java/ql/src/experimental/Security/CWE/CWE-016/InsecureSpringActuatorConfig.ql b/java/ql/src/experimental/Security/CWE/CWE-016/InsecureSpringActuatorConfig.ql new file mode 100644 index 000000000000..e6965959d13f --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-016/InsecureSpringActuatorConfig.ql @@ -0,0 +1,116 @@ +/** + * @name Insecure Spring Boot Actuator Configuration + * @description Exposed Spring Boot Actuator through configuration files without declarative or procedural + * security enforcement leads to information leak or even remote code execution. + * @kind problem + * @id java/insecure-spring-actuator-config + * @tags security + * external/cwe-016 + */ + +/* + * Note this query requires properties files to be indexed before it can produce results. + * If creating your own database with the CodeQL CLI, you should run + * `codeql database index-files --language=properties ...` + * If using lgtm.com, you should add `properties_files: true` to the index block of your + * lgtm.yml file (see https://lgtm.com/help/lgtm/java-extraction) + */ + +import java +import semmle.code.configfiles.ConfigFiles +import semmle.code.xml.MavenPom + +/** The parent node of the `org.springframework.boot` group. */ +class SpringBootParent extends Parent { + SpringBootParent() { this.getGroup().getValue() = "org.springframework.boot" } +} + +/** Class of Spring Boot dependencies. */ +class SpringBootPom extends Pom { + SpringBootPom() { this.getParentElement() instanceof SpringBootParent } + + /** Holds if the Spring Boot Actuator module `spring-boot-starter-actuator` is used in the project. */ + predicate isSpringBootActuatorUsed() { + this.getADependency().getArtifact().getValue() = "spring-boot-starter-actuator" + } + + /** + * Holds if the Spring Boot Security module is used in the project, which brings in other security + * related libraries. + */ + predicate isSpringBootSecurityUsed() { + this.getADependency().getArtifact().getValue() = "spring-boot-starter-security" + } +} + +/** The properties file `application.properties`. */ +class ApplicationProperties extends ConfigPair { + ApplicationProperties() { this.getFile().getBaseName() = "application.properties" } +} + +/** The configuration property `management.security.enabled`. */ +class ManagementSecurityConfig extends ApplicationProperties { + ManagementSecurityConfig() { this.getNameElement().getName() = "management.security.enabled" } + + /** Gets the whitespace-trimmed value of this property. */ + string getValue() { result = this.getValueElement().getValue().trim() } + + /** Holds if `management.security.enabled` is set to `false`. */ + predicate hasSecurityDisabled() { getValue() = "false" } + + /** Holds if `management.security.enabled` is set to `true`. */ + predicate hasSecurityEnabled() { getValue() = "true" } +} + +/** The configuration property `management.endpoints.web.exposure.include`. */ +class ManagementEndPointInclude extends ApplicationProperties { + ManagementEndPointInclude() { + this.getNameElement().getName() = "management.endpoints.web.exposure.include" + } + + /** Gets the whitespace-trimmed value of this property. */ + string getValue() { result = this.getValueElement().getValue().trim() } +} + +/** + * Holds if `ApplicationProperties` ap of a repository managed by `SpringBootPom` pom + * has a vulnerable configuration of Spring Boot Actuator management endpoints. + */ +predicate hasConfidentialEndPointExposed(SpringBootPom pom, ApplicationProperties ap) { + pom.isSpringBootActuatorUsed() and + not pom.isSpringBootSecurityUsed() and + ap.getFile() + .getParentContainer() + .getAbsolutePath() + .matches(pom.getFile().getParentContainer().getAbsolutePath() + "%") and // in the same sub-directory + exists(string springBootVersion | springBootVersion = pom.getParentElement().getVersionString() | + springBootVersion.regexpMatch("1\\.[0-4].*") and // version 1.0, 1.1, ..., 1.4 + not exists(ManagementSecurityConfig me | + me.hasSecurityEnabled() and me.getFile() = ap.getFile() + ) + or + springBootVersion.matches("1.5%") and // version 1.5 + exists(ManagementSecurityConfig me | me.hasSecurityDisabled() and me.getFile() = ap.getFile()) + or + springBootVersion.matches("2.%") and //version 2.x + exists(ManagementEndPointInclude mi | + mi.getFile() = ap.getFile() and + ( + mi.getValue() = "*" // all endpoints are enabled + or + mi.getValue() + .matches([ + "%dump%", "%trace%", "%logfile%", "%shutdown%", "%startup%", "%mappings%", "%env%", + "%beans%", "%sessions%" + ]) // confidential endpoints to check although all endpoints apart from '/health' and '/info' are considered sensitive by Spring + ) + ) + ) +} + +from SpringBootPom pom, ApplicationProperties ap, Dependency d +where + hasConfidentialEndPointExposed(pom, ap) and + d = pom.getADependency() and + d.getArtifact().getValue() = "spring-boot-starter-actuator" +select d, "Insecure configuration of Spring Boot Actuator exposes sensitive endpoints." diff --git a/java/ql/src/experimental/Security/CWE/CWE-016/application.properties b/java/ql/src/experimental/Security/CWE/CWE-016/application.properties new file mode 100644 index 000000000000..4f5defdd948e --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-016/application.properties @@ -0,0 +1,22 @@ +#management.endpoints.web.base-path=/admin + + +#### BAD: All management endpoints are accessible #### +# vulnerable configuration (spring boot 1.0 - 1.4): exposes actuators by default + +# vulnerable configuration (spring boot 1.5+): requires value false to expose sensitive actuators +management.security.enabled=false + +# vulnerable configuration (spring boot 2+): exposes health and info only by default, here overridden to expose everything +management.endpoints.web.exposure.include=* + + +#### GOOD: All management endpoints have access control #### +# safe configuration (spring boot 1.0 - 1.4): exposes actuators by default +management.security.enabled=true + +# safe configuration (spring boot 1.5+): requires value false to expose sensitive actuators +management.security.enabled=true + +# safe configuration (spring boot 2+): exposes health and info only by default, here overridden to expose one additional endpoint which we assume is intentional and safe. +management.endpoints.web.exposure.include=beans,info,health diff --git a/java/ql/src/experimental/Security/CWE/CWE-016/pom_bad.xml b/java/ql/src/experimental/Security/CWE/CWE-016/pom_bad.xml new file mode 100644 index 000000000000..9dd5c9c188b4 --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-016/pom_bad.xml @@ -0,0 +1,50 @@ + + + 4.0.0 + + spring-boot-actuator-app + spring-boot-actuator-app + 1.0-SNAPSHOT + + + UTF-8 + 1.8 + 1.8 + + + + org.springframework.boot + spring-boot-starter-parent + 2.3.8.RELEASE + + + + + + org.springframework.boot + spring-boot-starter-web + + + org.springframework.boot + spring-boot-starter-actuator + + + org.springframework.boot + spring-boot-devtools + + + + + + + org.springframework.boot + spring-boot-test + + + + \ No newline at end of file diff --git a/java/ql/src/experimental/Security/CWE/CWE-016/pom_good.xml b/java/ql/src/experimental/Security/CWE/CWE-016/pom_good.xml new file mode 100644 index 000000000000..89f577f21e59 --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-016/pom_good.xml @@ -0,0 +1,50 @@ + + + 4.0.0 + + spring-boot-actuator-app + spring-boot-actuator-app + 1.0-SNAPSHOT + + + UTF-8 + 1.8 + 1.8 + + + + org.springframework.boot + spring-boot-starter-parent + 2.3.8.RELEASE + + + + + + org.springframework.boot + spring-boot-starter-web + + + org.springframework.boot + spring-boot-starter-actuator + + + org.springframework.boot + spring-boot-devtools + + + + + org.springframework.boot + spring-boot-starter-security + + + + org.springframework.boot + spring-boot-test + + + + \ No newline at end of file diff --git a/java/ql/test/experimental/query-tests/security/CWE-016/InsecureSpringActuatorConfig.expected b/java/ql/test/experimental/query-tests/security/CWE-016/InsecureSpringActuatorConfig.expected new file mode 100644 index 000000000000..486302939857 --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-016/InsecureSpringActuatorConfig.expected @@ -0,0 +1 @@ +| pom.xml:29:9:32:22 | dependency | Insecure configuration of Spring Boot Actuator exposes sensitive endpoints. | diff --git a/java/ql/test/experimental/query-tests/security/CWE-016/InsecureSpringActuatorConfig.qlref b/java/ql/test/experimental/query-tests/security/CWE-016/InsecureSpringActuatorConfig.qlref new file mode 100644 index 000000000000..9cd12d5e4fb1 --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-016/InsecureSpringActuatorConfig.qlref @@ -0,0 +1 @@ +experimental/Security/CWE/CWE-016/InsecureSpringActuatorConfig.ql \ No newline at end of file diff --git a/java/ql/test/experimental/query-tests/security/CWE-016/SensitiveInfo.java b/java/ql/test/experimental/query-tests/security/CWE-016/SensitiveInfo.java new file mode 100644 index 000000000000..a3ff69c1b817 --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-016/SensitiveInfo.java @@ -0,0 +1,13 @@ +import org.springframework.stereotype.Controller; +import org.springframework.web.bind.annotation.RequestParam; +import org.springframework.web.bind.annotation.RequestMapping; + +@Controller +public class SensitiveInfo { + @RequestMapping + public void handleLogin(@RequestParam String username, @RequestParam String password) throws Exception { + if (!username.equals("") && password.equals("")) { + //Blank processing + } + } +} \ No newline at end of file diff --git a/java/ql/test/experimental/query-tests/security/CWE-016/application.properties b/java/ql/test/experimental/query-tests/security/CWE-016/application.properties new file mode 100644 index 000000000000..797906a3ca3b --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-016/application.properties @@ -0,0 +1,14 @@ +#management.endpoints.web.base-path=/admin + +# vulnerable configuration (spring boot 1.0 - 1.4): exposes actuators by default + +# vulnerable configuration (spring boot 1.5+): requires value false to expose sensitive actuators +management.security.enabled=false + +# vulnerable configuration (spring boot 2+): exposes health and info only by default, here overridden to expose everything +management.endpoints.web.exposure.include=* +management.endpoints.web.exposure.exclude=beans + +management.endpoint.shutdown.enabled=true + +management.endpoint.health.show-details=when_authorized \ No newline at end of file diff --git a/java/ql/test/experimental/query-tests/security/CWE-016/pom.xml b/java/ql/test/experimental/query-tests/security/CWE-016/pom.xml new file mode 100644 index 000000000000..a9d5fa920c84 --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-016/pom.xml @@ -0,0 +1,47 @@ + + + 4.0.0 + + spring-boot-actuator-app + spring-boot-actuator-app + 1.0-SNAPSHOT + + + UTF-8 + 1.8 + 1.8 + + + + org.springframework.boot + spring-boot-starter-parent + 2.3.8.RELEASE + + + + + + org.springframework.boot + spring-boot-starter-web + + + org.springframework.boot + spring-boot-starter-actuator + + + org.springframework.boot + spring-boot-devtools + + + + org.springframework.boot + spring-boot-test + + + + \ No newline at end of file