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 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.
-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 RequestMatcher objects that can be used in
-combination with Spring Security.
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.
-In the second example, only users with ENDPOINT_ADMIN role are allowed to access
-the actuator endpoints.
-This query flags up situations in which untrusted user data is included in Log4j messages. If an application uses a Log4j version prior to 2.15.0, using untrusted user data in log messages will make an application vulnerable to remote code execution through Log4j's LDAP JNDI parser (CVE-2021-44228). -
--As per Apache's Log4j security guide: Apache Log4j2 <=2.14.1 JNDI features used in configuration, log messages, and parameters -do not protect against attacker controlled LDAP and other JNDI related endpoints. An attacker who can control log messages or -log message parameters can execute arbitrary code loaded from LDAP servers when message lookup substitution is enabled. -From Log4j 2.15.0, this behavior has been disabled by default. Note that this query will not try to determine which version of Log4j is used. -
--This issue was remediated in Log4j v2.15.0. The Apache Logging Services team provides the following mitigation advice: -
-
-In previous releases (>=2.10) this behavior can be mitigated by setting system property log4j2.formatMsgNoLookups to true
-or by removing the JndiLookup class from the classpath (example: zip -q -d log4j-core-*.jar org/apache/logging/log4j/core/lookup/JndiLookup.class).
-
-You can manually check for use of affected versions of Log4j by searching your project repository for Log4j use, which is often in a pom.xml file. -
--Where possible, upgrade to Log4j version 2.15.0. If you are using Log4j v1 there is a migration guide available. -
--Please note that Log4j v1 is End Of Life (EOL) and will not receive patches for this issue. Log4j v1 is also vulnerable to other RCE vectors and we -recommend you migrate to Log4j 2.15.0 where possible. -
--If upgrading is not possible, then ensure the -Dlog4j2.formatMsgNoLookups=true system property is set on both client- and server-side components. -
-In this example, a username, provided by the user, is logged using logger.warn (from org.apache.logging.log4j.Logger).
- If a malicious user provides ${jndi:ldap://127.0.0.1:1389/a} as a username parameter,
- Log4j will make a JNDI lookup on the specified LDAP server and potentially load arbitrary code.
-
Calling openStream on URLs created from remote source can lead to local file disclosure.
If openStream is called on a java.net.URL, that was created from a remote source,
-an attacker can try to pass absolute URLs starting with file:// or jar:// to access
-local resources in addition to remote ones.
When you construct a URL using java.net.URL from a remote source,
-don't call openStream on it. Instead, use an HTTP Client to fetch the URL and access its content.
-You should also validate the URL to check that it uses the correct protocol and host combination.
The following example shows an URL that is constructed from a request parameter. Afterwards openStream
-is called on the URL, potentially leading to a local file access.
External Control of File Name or Path, also called File Path Injection, is a vulnerability by which -a file path is created using data from outside the application (such as the HTTP request). It allows -an attacker to traverse through the filesystem and access arbitrary files.
-Unsanitized user-provided data must not be used to construct file paths. In order to prevent File
-Path Injection, it is recommended to avoid concatenating user input directly into the file path. Instead,
-user input should be checked against allowed or disallowed paths (for example, the path must be within
-/user_content/ or must not be within /internal), ensuring that neither path
-traversal using ../ nor URL encoding is used to evade these checks.
-
The following examples show the bad case and the good case respectively.
-The BAD methods show an HTTP request parameter being used directly to construct a file path
-without validating the input, which may cause file leakage. In the GOOD method, the file path
-is validated.
-
Code that passes remote user input to an arugment of a call of Runtime.exec that
-executes a scripting executable will allow the user to execute malicious code.
If possible, use hard-coded string literals to specify the command or script to run, -or library to load. Instead of passing the user input directly to the -process or library function, examine the user input and then choose -among hard-coded string literals.
- -If the applicable libraries or commands cannot be determined at -compile time, then add code to verify that the user input string is -safe before using it.
- -The following example shows code that takes a shell script that can be changed
-maliciously by a user, and passes it straight to the array going into Runtime.exec
-without examining it first.
Code that passes local user input to an arugment of a call of Runtime.exec that
-executes a scripting executable will allow the user to execute malicious code.
If possible, use hard-coded string literals to specify the command or script to run, -or library to load. Instead of passing the user input directly to the -process or library function, examine the user input and then choose -among hard-coded string literals.
- -If the applicable libraries or commands cannot be determined at -compile time, then add code to verify that the user input string is -safe before using it.
- -The following example shows code that takes a shell script that can be changed
-maliciously by a user, and passes it straight to the array going into Runtime.exec
-without examining it first.
Code that passes user input directly to Runtime.exec, or
-some other library routine that executes a command, allows the
-user to execute malicious code.
If possible, use hard-coded string literals to specify the command to run -or library to load. Instead of passing the user input directly to the -process or library function, examine the user input and then choose -among hard-coded string literals.
- -If the applicable libraries or commands cannot be determined at -compile time, then add code to verify that the user input string is -safe before using it.
- -The following example shows code that takes a shell script that can be changed
-maliciously by a user, and passes it straight to Runtime.exec
-without examining it first.
MyBatis uses methods with the annotations @Select, @Insert, etc. to construct dynamic SQL statements.
-If the syntax ${param} is used in those statements, and param is a parameter of the annotated method, attackers can exploit this to tamper with the SQL statements or execute arbitrary SQL commands.
-When writing MyBatis mapping statements, use the syntax #{xxx} whenever possible. If the syntax ${xxx} must be used, any parameters included in it should be sanitized to prevent SQL injection attacks.
-
-The following sample shows a bad and a good example of MyBatis annotations usage. The bad1 method uses $(name)
-in the @Select annotation to dynamically build a SQL statement, which causes a SQL injection vulnerability.
-The good1 method uses #{name} in the @Select annotation to dynamically include the parameter in a SQL statement, which causes the MyBatis framework to sanitize the input provided, preventing the vulnerability.
-
MyBatis allows operating the database by creating XML files to construct dynamic SQL statements.
-If the syntax ${param} is used in those statements, and param is under the user's control, attackers can exploit this to tamper with the SQL statements or execute arbitrary SQL commands.
-When writing MyBatis mapping statements, try to use the syntax #{xxx}. If the syntax ${xxx} must be used, any parameters included in it should be sanitized to prevent SQL injection attacks.
-
-The following sample shows several bad and good examples of MyBatis XML files usage. In bad1,
-bad2, bad3, bad4, and bad5 the syntax
-${xxx} is used to build dynamic SQL statements, which causes a SQL injection vulnerability. In good1,
-the program uses the ${xxx} syntax, but there are subtle restrictions on the data,
-while in good2 the syntax #{xxx} is used. In both cases the SQL injection vulnerability is prevented.
-
-BeanShell is a small, free, embeddable Java source interpreter with object scripting language -features, written in Java. BeanShell dynamically executes standard Java syntax and extends it -with common scripting conveniences such as loose types, commands, and method closures like -those in Perl and JavaScript. If a BeanShell expression is built using attacker-controlled data, -and then evaluated, then it may allow the attacker to run arbitrary code. -
--It is generally recommended to avoid using untrusted input in a BeanShell expression. -If it is not possible, BeanShell expressions should be run in a sandbox that allows accessing only -explicitly allowed classes. -
--The following example uses untrusted data to build and run a BeanShell expression. -
--It is dangerous to load Dex libraries from shared world-writable storage spaces. A malicious actor can replace a dex file with a maliciously crafted file -which when loaded by the app can lead to code execution. -
-- Loading a file from private storage instead of a world-writable one can prevent this issue, - because the attacker cannot access files stored there. -
-- The following example loads a Dex file from a shared world-writable location. in this case, - since the `/sdcard` directory is on external storage, anyone can read/write to the location. - bypassing all Android security policies. Hence, this is insecure. -
-- The next example loads a Dex file stored inside the app's private storage. - This is not exploitable as nobody else except the app can access the data stored there. -
-The Java Shell tool (JShell) is an interactive tool for learning the Java programming -language and prototyping Java code. JShell is a Read-Evaluate-Print Loop (REPL), which -evaluates declarations, statements, and expressions as they are entered and immediately -shows the results. If an expression is built using attacker-controlled data and then evaluated, -it may allow the attacker to run arbitrary code.
-It is generally recommended to avoid using untrusted input in a JShell expression. -If it is not possible, JShell expressions should be run in a sandbox that allows accessing only -explicitly allowed classes.
-The following example calls JShell.eval(...) or SourceCodeAnalysis.wrappers(...)
-to execute untrusted data.
-Jakarta Expression Language (EL) is an expression language for Java applications. -There is a single language specification and multiple implementations -such as Glassfish, Juel, Apache Commons EL, etc. -The language allows invocation of methods available in the JVM. -If an expression is built using attacker-controlled data, -and then evaluated, it may allow the attacker to run arbitrary code. -
--It is generally recommended to avoid using untrusted data in an EL expression. -Before using untrusted data to build an EL expression, the data should be validated -to ensure it is not evaluated as expression language. If the EL implementation offers -configuring a sandbox for EL expressions, they should be run in a restrictive sandbox -that allows accessing only explicitly allowed classes. If the EL implementation -does not support sandboxing, consider using other expression language implementations -with sandboxing capabilities such as Apache Commons JEXL or the Spring Expression Language. -
--The following example shows how untrusted data is used to build and run an expression -using the JUEL interpreter: -
--JUEL does not support running expressions in a sandbox. To prevent running arbitrary code, -incoming data has to be checked before including it in an expression. The next example -uses a Regex pattern to check whether a user tries to run an allowed expression or not: -
-Python has been the most widely used programming language in recent years, and Jython - (formerly known as JPython) is a popular Java implementation of Python. It allows - embedded Python scripting inside Java applications and provides an interactive interpreter - that can be used to interact with Java packages or with running Java applications. If an - expression is built using attacker-controlled data and then evaluated, it may allow the - attacker to run arbitrary code.
-In general, including user input in Jython expression should be avoided. If user input - must be included in an expression, it should be then evaluated in a safe context that - doesn't allow arbitrary code invocation.
-The following code could execute arbitrary code in Jython Interpreter
-", 1, null);
- response.getWriter().print(Context.toString(result));
- } catch(RhinoException ex) {
- response.getWriter().println(ex.getMessage());
- } finally {
- Context.exit();
- }
- }
-}
diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/SaferExpressionEvaluationWithJuel.java b/java/ql/src/experimental/Security/CWE/CWE-094/SaferExpressionEvaluationWithJuel.java
deleted file mode 100644
index 3dfaaead68a5..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-094/SaferExpressionEvaluationWithJuel.java
+++ /dev/null
@@ -1,10 +0,0 @@
-String input = getRemoteUserInput();
-String pattern = "(inside|outside)\\.(temperature|humidity)";
-if (!input.matches(pattern)) {
- throw new IllegalArgumentException("Unexpected expression");
-}
-String expression = "${" + input + "}";
-ExpressionFactory factory = new de.odysseus.el.ExpressionFactoryImpl();
-ValueExpression e = factory.createValueExpression(context, expression, Object.class);
-SimpleContext context = getContext();
-Object result = e.getValue(context);
diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/ScriptEngine.java b/java/ql/src/experimental/Security/CWE/CWE-094/ScriptEngine.java
deleted file mode 100644
index 3612fcb1561e..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-094/ScriptEngine.java
+++ /dev/null
@@ -1,4 +0,0 @@
-// Bad: ScriptEngine allows arbitrary code injection
-ScriptEngineManager scriptEngineManager = new ScriptEngineManager();
-ScriptEngine scriptEngine = scriptEngineManager.getEngineByExtension("js");
-Object result = scriptEngine.eval(code);
\ No newline at end of file
diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/ScriptInjection.qhelp b/java/ql/src/experimental/Security/CWE/CWE-094/ScriptInjection.qhelp
deleted file mode 100644
index 2683cf9ad29e..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-094/ScriptInjection.qhelp
+++ /dev/null
@@ -1,52 +0,0 @@
-
-
-
-
-The Java Scripting API has been available since the release of Java 6. It allows
- applications to interact with scripts written in languages such as JavaScript. It serves
- as an embedded scripting engine inside Java applications which allows Java-to-JavaScript
- interoperability and provides a seamless integration between the two languages. If an
- expression is built using attacker-controlled data, and then evaluated in a powerful
- context, it may allow the attacker to run arbitrary code.
-
-
-
-In general, including user input in a Java Script Engine expression should be avoided.
- If user input must be included in the expression, it should be then evaluated in a safe
- context that doesn't allow arbitrary code invocation. Use "Cloudbees Rhino Sandbox" or
- sandboxing with SecurityManager, which will be deprecated in a future release, or use
- GraalVM instead.
-
-
-
-The following code could execute user-supplied JavaScript code in ScriptEngine
-The following example shows two ways of using Rhino expression. In the 'BAD' case,
- an unsafe context is initialized with initStandardObjects that allows arbitrary
- Java code to be executed. In the 'GOOD' case, a safe context is initialized with
- initSafeStandardObjects or setClassShutter.
-
-
-
-
-CERT coding standard: ScriptEngine code injection
-
-
-GraalVM: Secure by Default
-
-
- Mozilla Rhino: Rhino: JavaScript in Java
-
-
- Rhino Sandbox: A sandbox to execute JavaScript code with Rhino in Java
-
-
- GuardRails: Code Injection
-
-
-
diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/ScriptInjection.ql b/java/ql/src/experimental/Security/CWE/CWE-094/ScriptInjection.ql
deleted file mode 100644
index c85c67422b81..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-094/ScriptInjection.ql
+++ /dev/null
@@ -1,154 +0,0 @@
-/**
- * @name Injection in Java Script Engine
- * @description Evaluation of user-controlled data using the Java Script Engine may
- * lead to remote code execution.
- * @kind path-problem
- * @problem.severity error
- * @precision high
- * @id java/unsafe-eval
- * @tags security
- * experimental
- * external/cwe/cwe-094
- */
-
-import java
-import semmle.code.java.dataflow.TaintTracking
-import semmle.code.java.dataflow.FlowSources
-import ScriptInjectionFlow::PathGraph
-
-/** A method of ScriptEngine that allows code injection. */
-class ScriptEngineMethod extends Method {
- ScriptEngineMethod() {
- this.getDeclaringType().getAnAncestor().hasQualifiedName("javax.script", "ScriptEngine") and
- this.hasName("eval")
- or
- this.getDeclaringType().getAnAncestor().hasQualifiedName("javax.script", "Compilable") and
- this.hasName("compile")
- or
- this.getDeclaringType().getAnAncestor().hasQualifiedName("javax.script", "ScriptEngineFactory") and
- this.hasName(["getProgram", "getMethodCallSyntax"])
- }
-}
-
-/** The context class `org.mozilla.javascript.Context` of Rhino Java Script Engine. */
-class RhinoContext extends RefType {
- RhinoContext() { this.hasQualifiedName("org.mozilla.javascript", "Context") }
-}
-
-/** A method that evaluates a Rhino expression with `org.mozilla.javascript.Context`. */
-class RhinoEvaluateExpressionMethod extends Method {
- RhinoEvaluateExpressionMethod() {
- this.getDeclaringType().getAnAncestor*() instanceof RhinoContext and
- this.hasName([
- "evaluateString", "evaluateReader", "compileFunction", "compileReader", "compileString"
- ])
- }
-}
-
-/**
- * A method that compiles a Rhino expression with
- * `org.mozilla.javascript.optimizer.ClassCompiler`.
- */
-class RhinoCompileClassMethod extends Method {
- RhinoCompileClassMethod() {
- this.getDeclaringType()
- .getAnAncestor()
- .hasQualifiedName("org.mozilla.javascript.optimizer", "ClassCompiler") and
- this.hasName("compileToClassFiles")
- }
-}
-
-/**
- * A method that defines a Java class from a Rhino expression with
- * `org.mozilla.javascript.GeneratedClassLoader`.
- */
-class RhinoDefineClassMethod extends Method {
- RhinoDefineClassMethod() {
- this.getDeclaringType()
- .getAnAncestor()
- .hasQualifiedName("org.mozilla.javascript", "GeneratedClassLoader") and
- this.hasName("defineClass")
- }
-}
-
-/**
- * Holds if `ma` is a call to a `ScriptEngineMethod` and `sink` is an argument that
- * will be executed.
- */
-predicate isScriptArgument(MethodCall ma, Expr sink) {
- exists(ScriptEngineMethod m |
- m = ma.getMethod() and
- if m.getDeclaringType().getAnAncestor().hasQualifiedName("javax.script", "ScriptEngineFactory")
- then sink = ma.getArgument(_) // all arguments allow script injection
- else sink = ma.getArgument(0)
- )
-}
-
-/**
- * Holds if a Rhino expression evaluation method is vulnerable to code injection.
- */
-predicate evaluatesRhinoExpression(MethodCall ma, Expr sink) {
- exists(RhinoEvaluateExpressionMethod m | m = ma.getMethod() |
- (
- if ma.getMethod().getName() = "compileReader"
- then sink = ma.getArgument(0) // The first argument is the input reader
- else sink = ma.getArgument(1) // The second argument is the JavaScript or Java input
- ) and
- not exists(MethodCall ca |
- ca.getMethod().hasName(["initSafeStandardObjects", "setClassShutter"]) and // safe mode or `ClassShutter` constraint is enforced
- ma.getQualifier() = ca.getQualifier().(VarAccess).getVariable().getAnAccess()
- )
- )
-}
-
-/**
- * Holds if a Rhino expression compilation method is vulnerable to code injection.
- */
-predicate compilesScript(MethodCall ma, Expr sink) {
- exists(RhinoCompileClassMethod m | m = ma.getMethod() | sink = ma.getArgument(0))
-}
-
-/**
- * Holds if a Rhino class loading method is vulnerable to code injection.
- */
-predicate definesRhinoClass(MethodCall ma, Expr sink) {
- exists(RhinoDefineClassMethod m | m = ma.getMethod() | sink = ma.getArgument(1))
-}
-
-/** A script injection sink. */
-class ScriptInjectionSink extends DataFlow::ExprNode {
- MethodCall methodAccess;
-
- ScriptInjectionSink() {
- isScriptArgument(methodAccess, this.getExpr()) or
- evaluatesRhinoExpression(methodAccess, this.getExpr()) or
- compilesScript(methodAccess, this.getExpr()) or
- definesRhinoClass(methodAccess, this.getExpr())
- }
-
- /** An access to the method associated with this sink. */
- MethodCall getMethodCall() { result = methodAccess }
-}
-
-/**
- * A taint tracking configuration that tracks flow from `ActiveThreatModelSource` to an argument
- * of a method call that executes injected script.
- */
-module ScriptInjectionConfig implements DataFlow::ConfigSig {
- predicate isSource(DataFlow::Node source) { source instanceof ActiveThreatModelSource }
-
- predicate isSink(DataFlow::Node sink) { sink instanceof ScriptInjectionSink }
-}
-
-module ScriptInjectionFlow = TaintTracking::Global;
-
-deprecated query predicate problems(
- MethodCall sinkCall, ScriptInjectionFlow::PathNode source, ScriptInjectionFlow::PathNode sink,
- string message1, DataFlow::Node sourceNode, string message2
-) {
- ScriptInjectionFlow::flowPath(source, sink) and
- sinkCall = sink.getNode().(ScriptInjectionSink).getMethodCall() and
- message1 = "Java Script Engine evaluate $@." and
- sourceNode = source.getNode() and
- message2 = "user input"
-}
diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/SpringFrameworkLib.qll b/java/ql/src/experimental/Security/CWE/CWE-094/SpringFrameworkLib.qll
deleted file mode 100644
index b569c7c11dc5..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-094/SpringFrameworkLib.qll
+++ /dev/null
@@ -1,29 +0,0 @@
-deprecated module;
-
-import java
-import semmle.code.java.dataflow.DataFlow
-
-/**
- * `WebRequest` interface is a source of tainted data.
- */
-class WebRequestSource extends DataFlow::Node {
- WebRequestSource() {
- exists(MethodCall ma, Method m | ma.getMethod() = m |
- m.getDeclaringType() instanceof WebRequest and
- (
- m.hasName("getHeader") or
- m.hasName("getHeaderValues") or
- m.hasName("getHeaderNames") or
- m.hasName("getParameter") or
- m.hasName("getParameterValues") or
- m.hasName("getParameterNames") or
- m.hasName("getParameterMap")
- ) and
- ma = this.asExpr()
- )
- }
-}
-
-class WebRequest extends RefType {
- WebRequest() { this.hasQualifiedName("org.springframework.web.context.request", "WebRequest") }
-}
diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/SpringImplicitViewManipulation.qhelp b/java/ql/src/experimental/Security/CWE/CWE-094/SpringImplicitViewManipulation.qhelp
deleted file mode 100644
index ffacd8f8b03a..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-094/SpringImplicitViewManipulation.qhelp
+++ /dev/null
@@ -1,4 +0,0 @@
-
-
-
diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/SpringImplicitViewManipulation.ql b/java/ql/src/experimental/Security/CWE/CWE-094/SpringImplicitViewManipulation.ql
deleted file mode 100644
index faef29d1fde6..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-094/SpringImplicitViewManipulation.ql
+++ /dev/null
@@ -1,65 +0,0 @@
-/**
- * @name Spring Implicit View Manipulation
- * @description Untrusted input in a Spring View Controller can lead to RCE.
- * @kind problem
- * @problem.severity error
- * @precision high
- * @id java/spring-view-manipulation-implicit
- * @tags security
- * experimental
- * external/cwe/cwe-094
- */
-
-import java
-deprecated import SpringViewManipulationLib
-
-deprecated private predicate canResultInImplicitViewConversion(Method m) {
- m.getReturnType() instanceof VoidType
- or
- m.getReturnType() instanceof MapType
- or
- m.getReturnType().(RefType).hasQualifiedName("org.springframework.ui", "Model")
-}
-
-private predicate maybeATestMethod(Method m) {
- exists(string s |
- s = m.getName() or
- s = m.getFile().getRelativePath() or
- s = m.getDeclaringType().getName()
- |
- s.matches(["%test%", "%example%", "%exception%"])
- )
-}
-
-deprecated private predicate mayBeExploitable(Method m) {
- // There should be a attacker controlled parameter in the URI for the attack to be exploitable.
- // This is possible only when there exists a parameter with the Spring `@PathVariable` annotation
- // applied to it.
- exists(Parameter p |
- p = m.getAParameter() and
- p.hasAnnotation("org.springframework.web.bind.annotation", "PathVariable") and
- // Having a parameter of say type `Long` is non exploitable as Java type
- // checking rules are applied prior to view name resolution, rendering the exploit useless.
- // hence, here we check for the param type to be a Java `String`.
- p.getType() instanceof TypeString and
- // Exclude cases where a regex check is applied on a parameter to prevent false positives.
- not m.(SpringRequestMappingMethod).getValue().matches("%{%:[%]%}%")
- ) and
- not maybeATestMethod(m)
-}
-
-deprecated query predicate problems(SpringRequestMappingMethod m, string message) {
- thymeleafIsUsed() and
- mayBeExploitable(m) and
- canResultInImplicitViewConversion(m) and
- // If there's a parameter of type`HttpServletResponse`, Spring Framework does not interpret
- // it as a view name, but just returns this string in HTTP Response preventing exploitation
- // This also applies to `@ResponseBody` annotation.
- not m.getParameterType(_) instanceof HttpServletResponse and
- // A spring request mapping method which does not have response body annotation applied to it
- m.getAnAnnotation().getType() instanceof SpringRequestMappingAnnotationType and
- not m.getAnAnnotation().getType() instanceof SpringResponseBodyAnnotationType and
- // `@RestController` inherits `@ResponseBody` internally so it should be ignored.
- not m.getDeclaringType() instanceof SpringRestController and
- message = "This method may be vulnerable to spring view manipulation vulnerabilities."
-}
diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/SpringViewBad.java b/java/ql/src/experimental/Security/CWE/CWE-094/SpringViewBad.java
deleted file mode 100644
index bb8121f7b11b..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-094/SpringViewBad.java
+++ /dev/null
@@ -1,17 +0,0 @@
-@Controller
-public class SptingViewManipulationController {
-
- Logger log = LoggerFactory.getLogger(HelloController.class);
-
- @GetMapping("/safe/fragment")
- public String Fragment(@RequestParam String section) {
- // bad as template path is attacker controlled
- return "welcome :: " + section;
- }
-
- @GetMapping("/doc/{document}")
- public void getDocument(@PathVariable String document) {
- // returns void, so view name is taken from URI
- log.info("Retrieving " + document);
- }
-}
diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/SpringViewGood.java b/java/ql/src/experimental/Security/CWE/CWE-094/SpringViewGood.java
deleted file mode 100644
index 046150cae955..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-094/SpringViewGood.java
+++ /dev/null
@@ -1,20 +0,0 @@
-@Controller
-public class SptingViewManipulationController {
-
- Logger log = LoggerFactory.getLogger(HelloController.class);
-
- @GetMapping("/safe/fragment")
- @ResponseBody
- public String Fragment(@RequestParam String section) {
- // good, as `@ResponseBody` annotation tells Spring
- // to process the return values as body, instead of view name
- return "welcome :: " + section;
- }
-
- @GetMapping("/safe/doc/{document}")
- public void getDocument(@PathVariable String document, HttpServletResponse response) {
- // good as `HttpServletResponse param tells Spring that the response is already
- // processed.
- log.info("Retrieving " + document); // FP
- }
-}
diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/SpringViewManipulation.qhelp b/java/ql/src/experimental/Security/CWE/CWE-094/SpringViewManipulation.qhelp
deleted file mode 100644
index 67d348dfdb32..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-094/SpringViewManipulation.qhelp
+++ /dev/null
@@ -1,50 +0,0 @@
-
-
-
-
-
- The Spring Expression Language (SpEL) is a powerful expression language
- provided by Spring Framework. The language offers many features
- including invocation of methods available in the JVM.
-
-
- An unrestricted view name manipulation vulnerability in Spring Framework could lead to attacker-controlled arbitrary SpEL expressions being evaluated using attacker-controlled data, which may in turn allow an attacker to run arbitrary code.
-
-
- Note: two related variants of this problem are detected by different queries, `java/spring-view-manipulation` and `java/spring-view-manipulation-implicit`. The first detects taint flow problems where the return types is always String. While the latter, `java/spring-view-manipulation-implicit` detects cases where the request mapping method has a non-string return type such as void.
-
-
-
-
-
- In general, using user input to determine Spring view name should be avoided.
- If user input must be included in the expression, the controller can be annotated by
- a @ResponseBody annotation. In this case, Spring Framework does not interpret
- it as a view name, but just returns this string in HTTP Response. The same applies to using
- a @RestController annotation on a class, as internally it inherits @ResponseBody.
-
-
-
-
-
- In the following example, the Fragment method uses an externally controlled variable section to generate the view name. Hence, it is vulnerable to Spring View Manipulation attacks.
-
-
- This can be easily prevented by using the ResponseBody annotation which marks the response is already processed preventing exploitation of Spring View Manipulation vulnerabilities. Alternatively, this can also be fixed by adding a HttpServletResponse parameter to the method definition as shown in the example below.
-
-
-
-
-
- Veracode Research : Spring View Manipulation
-
-
- Spring Framework Reference Documentation: Spring Expression Language (SpEL)
-
-
- OWASP: Expression Language Injection
-
-
-
diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/SpringViewManipulation.ql b/java/ql/src/experimental/Security/CWE/CWE-094/SpringViewManipulation.ql
deleted file mode 100644
index d9705e7c304b..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-094/SpringViewManipulation.ql
+++ /dev/null
@@ -1,29 +0,0 @@
-/**
- * @name Spring View Manipulation
- * @description Untrusted input in a Spring View can lead to RCE.
- * @kind path-problem
- * @problem.severity error
- * @precision high
- * @id java/spring-view-manipulation
- * @tags security
- * experimental
- * external/cwe/cwe-094
- */
-
-import java
-import semmle.code.java.dataflow.DataFlow
-deprecated import SpringViewManipulationLib
-deprecated import SpringViewManipulationFlow::PathGraph
-
-deprecated query predicate problems(
- DataFlow::Node sinkNode, SpringViewManipulationFlow::PathNode source,
- SpringViewManipulationFlow::PathNode sink, string message1, DataFlow::Node sourceNode,
- string message2
-) {
- thymeleafIsUsed() and
- SpringViewManipulationFlow::flowPath(source, sink) and
- sinkNode = sink.getNode() and
- message1 = "Potential Spring Expression Language injection from $@." and
- sourceNode = source.getNode() and
- message2 = "this user input"
-}
diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/SpringViewManipulationLib.qll b/java/ql/src/experimental/Security/CWE/CWE-094/SpringViewManipulationLib.qll
deleted file mode 100644
index 5d65431b415a..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-094/SpringViewManipulationLib.qll
+++ /dev/null
@@ -1,141 +0,0 @@
-/**
- * Provides classes for reasoning about Spring View Manipulation vulnerabilities
- */
-deprecated module;
-
-import java
-import semmle.code.java.dataflow.FlowSources
-import semmle.code.java.dataflow.TaintTracking
-import semmle.code.java.frameworks.spring.Spring
-import SpringFrameworkLib
-
-/** Holds if `Thymeleaf` templating engine is used in the project. */
-predicate thymeleafIsUsed() {
- exists(Pom p |
- p.getADependency().getArtifact().getValue() in [
- "spring-boot-starter-thymeleaf", "thymeleaf-spring4", "springmvc-xml-thymeleaf",
- "thymeleaf-spring5"
- ]
- )
- or
- exists(SpringBean b | b.getClassNameRaw().matches("org.thymeleaf.spring%"))
-}
-
-/** Models methods from the `javax.portlet.RenderState` package which return data from externally controlled sources. */
-class PortletRenderRequestMethod extends Method {
- PortletRenderRequestMethod() {
- exists(RefType c, Interface t |
- c.extendsOrImplements*(t) and
- t.hasQualifiedName("javax.portlet", "RenderState") and
- this = c.getAMethod()
- |
- this.hasName([
- "getCookies", "getParameter", "getRenderParameters", "getParameterNames",
- "getParameterValues", "getParameterMap"
- ])
- )
- }
-}
-
-/**
- * A taint-tracking configuration for unsafe user input
- * that can lead to Spring View Manipulation vulnerabilities.
- */
-module SpringViewManipulationConfig implements DataFlow::ConfigSig {
- predicate isSource(DataFlow::Node source) {
- source instanceof ActiveThreatModelSource or
- source instanceof WebRequestSource or
- source.asExpr().(MethodCall).getMethod() instanceof PortletRenderRequestMethod
- }
-
- predicate isSink(DataFlow::Node sink) { sink instanceof SpringViewManipulationSink }
-
- predicate isBarrier(DataFlow::Node node) {
- // Block flows like
- // ```
- // a = "redirect:" + taint`
- // ```
- exists(AddExpr e, StringLiteral sl |
- node.asExpr() = e.getControlFlowNode().getASuccessor*().asExpr() and
- sl = e.getLeftOperand*() and
- sl.getValue().matches(["redirect:%", "ajaxredirect:%", "forward:%"])
- )
- or
- // Block flows like
- // ```
- // x.append("redirect:");
- // x.append(tainted());
- // return x.toString();
- //
- // "redirect:".concat(taint)
- //
- // String.format("redirect:%s",taint);
- // ```
- exists(Call ca, StringLiteral sl |
- (
- sl = ca.getArgument(_)
- or
- sl = ca.getQualifier()
- ) and
- ca = getAStringCombiningCall() and
- sl.getValue().matches(["redirect:%", "ajaxredirect:%", "forward:%"])
- |
- exists(Call cc | DataFlow::localExprFlow(ca.getQualifier(), cc.getQualifier()) |
- cc = node.asExpr()
- )
- )
- }
-}
-
-module SpringViewManipulationFlow = TaintTracking::Global;
-
-private Call getAStringCombiningCall() {
- exists(StringCombiningMethod m | result = m.getAReference())
-}
-
-abstract private class StringCombiningMethod extends Method { }
-
-private class AppendableAppendMethod extends StringCombiningMethod {
- AppendableAppendMethod() {
- exists(RefType t |
- t.hasQualifiedName("java.lang", "Appendable") and
- this.getDeclaringType().extendsOrImplements*(t) and
- this.hasName("append")
- )
- }
-}
-
-private class StringConcatMethod extends StringCombiningMethod {
- StringConcatMethod() {
- this.getDeclaringType() instanceof TypeString and
- this.hasName("concat")
- }
-}
-
-private class StringFormatMethod extends StringCombiningMethod {
- StringFormatMethod() {
- this.getDeclaringType() instanceof TypeString and
- this.hasName("format")
- }
-}
-
-/**
- * A sink for Spring View Manipulation vulnerabilities,
- */
-class SpringViewManipulationSink extends DataFlow::ExprNode {
- SpringViewManipulationSink() {
- exists(ReturnStmt r, SpringRequestMappingMethod m |
- r.getResult() = this.asExpr() and
- m.getBody().getAStmt() = r and
- not m.isResponseBody() and
- r.getResult().getType() instanceof TypeString
- )
- or
- exists(ConstructorCall c | c.getConstructedType() instanceof ModelAndView |
- this.asExpr() = c.getArgument(0) and
- c.getConstructor().getParameterType(0) instanceof TypeString
- )
- or
- exists(SpringModelAndViewSetViewNameCall c | this.asExpr() = c.getArgument(0))
- }
-}
diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/UnsafeExpressionEvaluationWithJuel.java b/java/ql/src/experimental/Security/CWE/CWE-094/UnsafeExpressionEvaluationWithJuel.java
deleted file mode 100644
index 27afa0fcb497..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-094/UnsafeExpressionEvaluationWithJuel.java
+++ /dev/null
@@ -1,5 +0,0 @@
-String expression = "${" + getRemoteUserInput() + "}";
-ExpressionFactory factory = new de.odysseus.el.ExpressionFactoryImpl();
-ValueExpression e = factory.createValueExpression(context, expression, Object.class);
-SimpleContext context = getContext();
-Object result = e.getValue(context);
\ No newline at end of file
diff --git a/java/ql/src/experimental/Security/CWE/CWE-1004/InsecureTomcatConfig.qhelp b/java/ql/src/experimental/Security/CWE/CWE-1004/InsecureTomcatConfig.qhelp
deleted file mode 100644
index 842402e9252d..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-1004/InsecureTomcatConfig.qhelp
+++ /dev/null
@@ -1,35 +0,0 @@
-
-
-
-
-When you add an application to a Tomcat server, it will generate a new JSESSIONID when you call request.getSession()
-or if you invoke a JSP from a servlet. If cookies are generated without the HttpOnly flag,
-an attacker can use a cross-site scripting (XSS) attack to get another user's session ID.
-
-
-
-
-Tomcat version 7+ automatically sets an HttpOnly flag on all session cookies to
-prevent client side scripts from accessing the session ID.
-In most situations, you should not override this behavior.
-
-
-
-The following example shows a Tomcat configuration with useHttpOnly disabled. Usually you should not set this.
-
-
-
-
-
-CWE:
-Sensitive Cookie Without 'HttpOnly' Flag.
-
-
-OWASP:
-
- HttpOnly
-.
-
-
-
diff --git a/java/ql/src/experimental/Security/CWE/CWE-1004/InsecureTomcatConfig.ql b/java/ql/src/experimental/Security/CWE/CWE-1004/InsecureTomcatConfig.ql
deleted file mode 100644
index 63b818e53a3d..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-1004/InsecureTomcatConfig.ql
+++ /dev/null
@@ -1,28 +0,0 @@
-/**
- * @name Tomcat config disables 'HttpOnly' flag (XSS risk)
- * @description Disabling 'HttpOnly' leaves session cookies vulnerable to an XSS attack.
- * @kind problem
- * @problem.severity warning
- * @precision medium
- * @id java/tomcat-disabled-httponly
- * @tags security
- * experimental
- * external/cwe/cwe-1004
- */
-
-import java
-import semmle.code.xml.WebXML
-
-private class HttpOnlyConfig extends WebContextParameter {
- HttpOnlyConfig() { this.getParamName().getValue() = "useHttpOnly" }
-
- string getParamValueElementValue() { result = this.getParamValue().getValue() }
-
- predicate isHttpOnlySet() { this.getParamValueElementValue().toLowerCase() = "false" }
-}
-
-deprecated query predicate problems(HttpOnlyConfig config, string message) {
- config.isHttpOnlySet() and
- message =
- "'httpOnly' should be enabled in tomcat config file to help mitigate cross-site scripting (XSS) attacks."
-}
diff --git a/java/ql/src/experimental/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.java b/java/ql/src/experimental/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.java
deleted file mode 100644
index 48d80707ff83..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.java
+++ /dev/null
@@ -1,44 +0,0 @@
-class SensitiveCookieNotHttpOnly {
- // GOOD - Create a sensitive cookie with the `HttpOnly` flag set.
- public void addCookie(String jwt_token, HttpServletRequest request, HttpServletResponse response) {
- Cookie jwtCookie =new Cookie("jwt_token", jwt_token);
- jwtCookie.setPath("/");
- jwtCookie.setMaxAge(3600*24*7);
- jwtCookie.setHttpOnly(true);
- response.addCookie(jwtCookie);
- }
-
- // BAD - Create a sensitive cookie without the `HttpOnly` flag set.
- public void addCookie2(String jwt_token, String userId, HttpServletRequest request, HttpServletResponse response) {
- Cookie jwtCookie =new Cookie("jwt_token", jwt_token);
- jwtCookie.setPath("/");
- jwtCookie.setMaxAge(3600*24*7);
- response.addCookie(jwtCookie);
- }
-
- // GOOD - Set a sensitive cookie header with the `HttpOnly` flag set.
- public void addCookie3(String authId, HttpServletRequest request, HttpServletResponse response) {
- response.addHeader("Set-Cookie", "token=" +authId + ";HttpOnly;Secure");
- }
-
- // BAD - Set a sensitive cookie header without the `HttpOnly` flag set.
- public void addCookie4(String authId, HttpServletRequest request, HttpServletResponse response) {
- response.addHeader("Set-Cookie", "token=" +authId + ";Secure");
- }
-
- // GOOD - Set a sensitive cookie header using the class `javax.ws.rs.core.Cookie` with the `HttpOnly` flag set through string concatenation.
- public void addCookie5(String accessKey, HttpServletRequest request, HttpServletResponse response) {
- response.setHeader("Set-Cookie", new NewCookie("session-access-key", accessKey, "/", null, null, 0, true) + ";HttpOnly");
- }
-
- // BAD - Set a sensitive cookie header using the class `javax.ws.rs.core.Cookie` without the `HttpOnly` flag set.
- public void addCookie6(String accessKey, HttpServletRequest request, HttpServletResponse response) {
- response.setHeader("Set-Cookie", new NewCookie("session-access-key", accessKey, "/", null, null, 0, true).toString());
- }
-
- // GOOD - Set a sensitive cookie header using the class `javax.ws.rs.core.Cookie` with the `HttpOnly` flag set through the constructor.
- public void addCookie7(String accessKey, HttpServletRequest request, HttpServletResponse response) {
- NewCookie accessKeyCookie = new NewCookie("session-access-key", accessKey, "/", null, null, 0, true, true);
- response.setHeader("Set-Cookie", accessKeyCookie.toString());
- }
-}
diff --git a/java/ql/src/experimental/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.qhelp b/java/ql/src/experimental/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.qhelp
deleted file mode 100644
index ee3e8a4181a9..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.qhelp
+++ /dev/null
@@ -1,27 +0,0 @@
-
-
-
-
- Cross-Site Scripting (XSS) is categorized as one of the OWASP Top 10 Security Vulnerabilities. The HttpOnly flag directs compatible browsers to prevent client-side script from accessing cookies. Including the HttpOnly flag in the Set-Cookie HTTP response header for a sensitive cookie helps mitigate the risk associated with XSS where an attacker's script code attempts to read the contents of a cookie and exfiltrate information obtained.
-
-
-
- Use the HttpOnly flag when generating a cookie containing sensitive information to help mitigate the risk of client side script accessing the protected cookie.
-
-
-
- The following example shows two ways of generating sensitive cookies. In the 'BAD' cases, the HttpOnly flag is not set. In the 'GOOD' cases, the HttpOnly flag is set.
-
-
-
-
- PortSwigger:
- Cookie without HttpOnly flag set
-
-
- OWASP:
- HttpOnly
-
-
-
diff --git a/java/ql/src/experimental/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql b/java/ql/src/experimental/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql
deleted file mode 100644
index fa5237d32bb9..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql
+++ /dev/null
@@ -1,224 +0,0 @@
-/**
- * @name Sensitive cookies without the HttpOnly response header set
- * @description Sensitive cookies without the 'HttpOnly' flag set leaves session cookies vulnerable to
- * an XSS attack.
- * @kind path-problem
- * @problem.severity warning
- * @precision medium
- * @id java/sensitive-cookie-not-httponly
- * @tags security
- * experimental
- * external/cwe/cwe-1004
- */
-
-/*
- * Sketch of the structure of this query: we track cookie names that appear to be sensitive
- * (e.g. `session` or `token`) to a `ServletResponse.addHeader(...)` or `.addCookie(...)`
- * method that does not set the `httpOnly` flag. Subsidiary configurations
- * `MatchesHttpOnlyConfiguration` and `SetHttpOnlyInCookieConfiguration` are used to establish
- * when the `httpOnly` flag is likely to have been set, before configuration
- * `MissingHttpOnlyConfiguration` establishes that a non-`httpOnly` cookie has a sensitive-seeming name.
- */
-
-import java
-import semmle.code.java.dataflow.FlowSteps
-import semmle.code.java.frameworks.Servlets
-import semmle.code.java.dataflow.TaintTracking
-import MissingHttpOnlyFlow::PathGraph
-
-/** Gets a regular expression for matching common names of sensitive cookies. */
-string getSensitiveCookieNameRegex() { result = "(?i).*(auth|session|token|key|credential).*" }
-
-/** Gets a regular expression for matching CSRF cookies. */
-string getCsrfCookieNameRegex() { result = "(?i).*(csrf).*" }
-
-/**
- * Holds if a string is concatenated with the name of a sensitive cookie. Excludes CSRF cookies since
- * they are special cookies implementing the Synchronizer Token Pattern that can be used in JavaScript.
- */
-predicate isSensitiveCookieNameExpr(Expr expr) {
- exists(string s | s = expr.(CompileTimeConstantExpr).getStringValue() |
- s.regexpMatch(getSensitiveCookieNameRegex()) and not s.regexpMatch(getCsrfCookieNameRegex())
- )
- or
- isSensitiveCookieNameExpr(expr.(AddExpr).getAnOperand())
-}
-
-/** A sensitive cookie name. */
-class SensitiveCookieNameExpr extends Expr {
- SensitiveCookieNameExpr() { isSensitiveCookieNameExpr(this) }
-}
-
-/** A method call that sets a `Set-Cookie` header. */
-class SetCookieMethodCall extends MethodCall {
- SetCookieMethodCall() {
- (
- this.getMethod() instanceof ResponseAddHeaderMethod or
- this.getMethod() instanceof ResponseSetHeaderMethod
- ) and
- this.getArgument(0).(CompileTimeConstantExpr).getStringValue().toLowerCase() = "set-cookie"
- }
-}
-
-/**
- * A taint configuration tracking flow from the text `httponly` to argument 1 of
- * `SetCookieMethodCall`.
- */
-module MatchesHttpOnlyConfig implements DataFlow::ConfigSig {
- predicate isSource(DataFlow::Node source) {
- source.asExpr().(CompileTimeConstantExpr).getStringValue().toLowerCase().matches("%httponly%")
- }
-
- predicate isSink(DataFlow::Node sink) {
- sink.asExpr() = any(SetCookieMethodCall ma).getArgument(1)
- }
-}
-
-module MatchesHttpOnlyFlow = TaintTracking::Global;
-
-/** A class descended from `javax.servlet.http.Cookie`. */
-class CookieClass extends RefType {
- CookieClass() { this.getAnAncestor().hasQualifiedName("javax.servlet.http", "Cookie") }
-}
-
-/** Holds if `expr` is any boolean-typed expression other than literal `false`. */
-// Inlined because this could be a very large result set if computed out of context
-pragma[inline]
-predicate mayBeBooleanTrue(Expr expr) {
- expr.getType() instanceof BooleanType and
- not expr.(CompileTimeConstantExpr).getBooleanValue() = false
-}
-
-/** Holds if the method call may set the `HttpOnly` flag. */
-predicate setsCookieHttpOnly(MethodCall ma) {
- ma.getMethod().getName() = "setHttpOnly" and
- // any use of setHttpOnly(x) where x isn't false is probably safe
- mayBeBooleanTrue(ma.getArgument(0))
-}
-
-/** Holds if `ma` removes a cookie. */
-predicate removesCookie(MethodCall ma) {
- ma.getMethod().getName() = "setMaxAge" and
- ma.getArgument(0).(IntegerLiteral).getIntValue() = 0
-}
-
-/**
- * Holds if the MethodCall `ma` is a test method call indicated by:
- * a) in a test directory such as `src/test/java`
- * b) in a test package whose name has the word `test`
- * c) in a test class whose name has the word `test`
- * d) in a test class implementing a test framework such as JUnit or TestNG
- */
-predicate isTestMethod(MethodCall ma) {
- exists(Method m |
- m = ma.getEnclosingCallable() and
- (
- m.getDeclaringType().getName().toLowerCase().matches("%test%") or // Simple check to exclude test classes to reduce FPs
- m.getDeclaringType().getPackage().getName().toLowerCase().matches("%test%") or // Simple check to exclude classes in test packages to reduce FPs
- exists(m.getLocation().getFile().getAbsolutePath().indexOf("/src/test/java")) or // Match test directory structure of build tools like maven
- m instanceof TestMethod // Test method of a test case implementing a test framework such as JUnit or TestNG
- )
- )
-}
-
-/**
- * A taint configuration tracking flow of a method that sets the `HttpOnly` flag,
- * or one that removes a cookie, to a `ServletResponse.addCookie` call.
- */
-module SetHttpOnlyOrRemovesCookieConfig implements DataFlow::ConfigSig {
- predicate isSource(DataFlow::Node source) {
- source.asExpr() =
- any(MethodCall ma | setsCookieHttpOnly(ma) or removesCookie(ma)).getQualifier()
- }
-
- predicate isSink(DataFlow::Node sink) {
- sink.asExpr() =
- any(MethodCall ma | ma.getMethod() instanceof ResponseAddCookieMethod).getArgument(0)
- }
-}
-
-module SetHttpOnlyOrRemovesCookieFlow = TaintTracking::Global;
-
-/**
- * A cookie that is added to an HTTP response and which doesn't have `httpOnly` set, used as a sink
- * in `MissingHttpOnlyConfiguration`.
- */
-class CookieResponseSink extends DataFlow::ExprNode {
- CookieResponseSink() {
- exists(MethodCall ma |
- (
- ma.getMethod() instanceof ResponseAddCookieMethod and
- this.getExpr() = ma.getArgument(0) and
- not SetHttpOnlyOrRemovesCookieFlow::flowTo(this)
- or
- ma instanceof SetCookieMethodCall and
- this.getExpr() = ma.getArgument(1) and
- not MatchesHttpOnlyFlow::flowTo(this) // response.addHeader("Set-Cookie", "token=" +authId + ";HttpOnly;Secure")
- ) and
- not isTestMethod(ma) // Test class or method
- )
- }
-}
-
-/** Holds if `cie` is an invocation of a JAX-RS `NewCookie` constructor that sets `HttpOnly` to true. */
-predicate setsHttpOnlyInNewCookie(ClassInstanceExpr cie) {
- cie.getConstructedType().hasQualifiedName(["javax.ws.rs.core", "jakarta.ws.rs.core"], "NewCookie") and
- (
- cie.getNumArgument() = 6 and
- mayBeBooleanTrue(cie.getArgument(5)) // NewCookie(Cookie cookie, String comment, int maxAge, Date expiry, boolean secure, boolean httpOnly)
- or
- cie.getNumArgument() = 8 and
- cie.getArgument(6).getType() instanceof BooleanType and
- mayBeBooleanTrue(cie.getArgument(7)) // NewCookie(String name, String value, String path, String domain, String comment, int maxAge, boolean secure, boolean httpOnly)
- or
- cie.getNumArgument() = 10 and
- mayBeBooleanTrue(cie.getArgument(9)) // NewCookie(String name, String value, String path, String domain, int version, String comment, int maxAge, Date expiry, boolean secure, boolean httpOnly)
- )
-}
-
-/**
- * A taint configuration tracking flow from a sensitive cookie without the `HttpOnly` flag
- * set to its HTTP response.
- */
-module MissingHttpOnlyConfig implements DataFlow::ConfigSig {
- predicate isSource(DataFlow::Node source) { source.asExpr() instanceof SensitiveCookieNameExpr }
-
- predicate isSink(DataFlow::Node sink) { sink instanceof CookieResponseSink }
-
- predicate isBarrier(DataFlow::Node node) {
- // JAX-RS's `new NewCookie("session-access-key", accessKey, "/", null, null, 0, true, true)` and similar
- setsHttpOnlyInNewCookie(node.asExpr())
- }
-
- predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) {
- exists(
- ConstructorCall cc // new Cookie(...)
- |
- cc.getConstructedType() instanceof CookieClass and
- pred.asExpr() = cc.getAnArgument() and
- succ.asExpr() = cc
- )
- or
- exists(
- MethodCall ma // cookie.toString()
- |
- ma.getMethod().getName() = "toString" and
- ma.getQualifier().getType() instanceof CookieClass and
- pred.asExpr() = ma.getQualifier() and
- succ.asExpr() = ma
- )
- }
-}
-
-module MissingHttpOnlyFlow = TaintTracking::Global;
-
-deprecated query predicate problems(
- DataFlow::Node sinkNode, MissingHttpOnlyFlow::PathNode source, MissingHttpOnlyFlow::PathNode sink,
- string message1, DataFlow::Node sourceNode, string message2
-) {
- MissingHttpOnlyFlow::flowPath(source, sink) and
- sinkNode = sink.getNode() and
- message1 = "$@ doesn't have the HttpOnly flag set." and
- sourceNode = source.getNode() and
- message2 = "This sensitive cookie"
-}
diff --git a/java/ql/src/experimental/Security/CWE/CWE-1004/insecure-web.xml b/java/ql/src/experimental/Security/CWE/CWE-1004/insecure-web.xml
deleted file mode 100644
index 2140acc28440..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-1004/insecure-web.xml
+++ /dev/null
@@ -1,9 +0,0 @@
-
- Sample Tomcat Web Application
-
- useHttpOnly
- false
-
-
\ No newline at end of file
diff --git a/java/ql/src/experimental/Security/CWE/CWE-200/AndroidFileIntentSink.qll b/java/ql/src/experimental/Security/CWE/CWE-200/AndroidFileIntentSink.qll
deleted file mode 100644
index 2472637241fd..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-200/AndroidFileIntentSink.qll
+++ /dev/null
@@ -1,61 +0,0 @@
-/** Provides Android sink models related to file creation. */
-deprecated module;
-
-import java
-import semmle.code.java.dataflow.DataFlow
-private import semmle.code.java.dataflow.ExternalFlow
-import semmle.code.java.frameworks.android.Android
-import semmle.code.java.frameworks.android.Intent
-
-/** A sink representing methods creating a file in Android. */
-class AndroidFileSink extends DataFlow::Node {
- AndroidFileSink() { sinkNode(this, "path-injection") }
-}
-
-/**
- * The Android class `android.os.AsyncTask` for running tasks off the UI thread to achieve
- * better user experience.
- */
-class AsyncTask extends RefType {
- AsyncTask() { this.hasQualifiedName("android.os", "AsyncTask") }
-}
-
-/** The `execute` or `executeOnExecutor` method of Android's `AsyncTask` class. */
-class ExecuteAsyncTaskMethod extends Method {
- int paramIndex;
-
- ExecuteAsyncTaskMethod() {
- this.getDeclaringType().getSourceDeclaration().getASourceSupertype*() instanceof AsyncTask and
- (
- this.getName() = "execute" and paramIndex = 0
- or
- this.getName() = "executeOnExecutor" and paramIndex = 1
- )
- }
-
- int getParamIndex() { result = paramIndex }
-}
-
-/** The `doInBackground` method of Android's `AsyncTask` class. */
-class AsyncTaskRunInBackgroundMethod extends Method {
- AsyncTaskRunInBackgroundMethod() {
- this.getDeclaringType().getSourceDeclaration().getASourceSupertype*() instanceof AsyncTask and
- this.getName() = "doInBackground"
- }
-}
-
-/** The service start method of Android's `Context` class. */
-class ContextStartServiceMethod extends Method {
- ContextStartServiceMethod() {
- this.getName() = ["startService", "startForegroundService"] and
- this.getDeclaringType().getAnAncestor() instanceof TypeContext
- }
-}
-
-/** The `onStartCommand` method of Android's `Service` class. */
-class ServiceOnStartCommandMethod extends Method {
- ServiceOnStartCommandMethod() {
- this.hasName("onStartCommand") and
- this.getDeclaringType() instanceof AndroidService
- }
-}
diff --git a/java/ql/src/experimental/Security/CWE/CWE-200/AndroidFileIntentSource.qll b/java/ql/src/experimental/Security/CWE/CWE-200/AndroidFileIntentSource.qll
deleted file mode 100644
index 03cabcfbaaa0..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-200/AndroidFileIntentSource.qll
+++ /dev/null
@@ -1,74 +0,0 @@
-/** Provides summary models relating to file content inputs of Android. */
-deprecated module;
-
-import java
-import semmle.code.java.dataflow.FlowSources
-import semmle.code.java.dataflow.TaintTracking
-import semmle.code.java.frameworks.android.Android
-
-/** The `startActivityForResult` method of Android's `Activity` class. */
-class StartActivityForResultMethod extends Method {
- StartActivityForResultMethod() {
- this.getDeclaringType().getAnAncestor() instanceof AndroidActivity and
- this.getName() = "startActivityForResult"
- }
-}
-
-/** An instance of `android.content.Intent` constructed passing `GET_CONTENT` to the constructor. */
-class GetContentIntent extends ClassInstanceExpr {
- GetContentIntent() {
- this.getConstructedType() instanceof TypeIntent and
- this.getArgument(0).(CompileTimeConstantExpr).getStringValue() =
- "android.intent.action.GET_CONTENT"
- or
- exists(Field f |
- this.getArgument(0) = f.getAnAccess() and
- f.hasName("ACTION_GET_CONTENT") and
- f.getDeclaringType() instanceof TypeIntent
- )
- }
-}
-
-/** Taint configuration that identifies `GET_CONTENT` `Intent` instances passed to `startActivityForResult`. */
-module GetContentIntentConfig implements DataFlow::ConfigSig {
- predicate isSource(DataFlow::Node src) { src.asExpr() instanceof GetContentIntent }
-
- predicate isSink(DataFlow::Node sink) {
- exists(MethodCall ma |
- ma.getMethod() instanceof StartActivityForResultMethod and sink.asExpr() = ma.getArgument(0)
- )
- }
-
- predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet content) {
- // Allow the wrapped intent created by Intent.getChooser to be consumed
- // by at the sink:
- isSink(node) and
- allowIntentExtrasImplicitRead(node, content)
- }
-}
-
-module GetContentsIntentFlow = TaintTracking::Global;
-
-/** A `GET_CONTENT` `Intent` instances that is passed to `startActivityForResult`. */
-class AndroidFileIntentInput extends DataFlow::Node {
- MethodCall ma;
-
- AndroidFileIntentInput() {
- this.asExpr() = ma.getArgument(0) and
- ma.getMethod() instanceof StartActivityForResultMethod and
- exists(GetContentIntent gi |
- GetContentsIntentFlow::flow(DataFlow::exprNode(gi), DataFlow::exprNode(ma.getArgument(0)))
- )
- }
-
- /** The request code passed to `startActivityForResult`, which is to be matched in `onActivityResult()`. */
- int getRequestCode() { result = ma.getArgument(1).(CompileTimeConstantExpr).getIntValue() }
-}
-
-/** The `onActivityForResult` method of Android `Activity` */
-class OnActivityForResultMethod extends Method {
- OnActivityForResultMethod() {
- this.getDeclaringType().getAnAncestor() instanceof AndroidActivity and
- this.getName() = "onActivityResult"
- }
-}
diff --git a/java/ql/src/experimental/Security/CWE/CWE-200/AndroidWebResourceResponse.qll b/java/ql/src/experimental/Security/CWE/CWE-200/AndroidWebResourceResponse.qll
deleted file mode 100644
index bd898df205a8..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-200/AndroidWebResourceResponse.qll
+++ /dev/null
@@ -1,79 +0,0 @@
-/** Provides Android methods relating to web resource response. */
-deprecated module;
-
-import java
-private import semmle.code.java.dataflow.DataFlow
-private import semmle.code.java.dataflow.ExternalFlow
-private import semmle.code.java.dataflow.FlowSteps
-private import semmle.code.java.frameworks.android.WebView
-
-private class ActivateModels extends ActiveExperimentalModels {
- ActivateModels() { this = "android-web-resource-response" }
-}
-
-/**
- * The Android class `android.webkit.WebResourceRequest` for handling web requests.
- */
-class WebResourceRequest extends RefType {
- WebResourceRequest() { this.hasQualifiedName("android.webkit", "WebResourceRequest") }
-}
-
-/**
- * The Android class `android.webkit.WebResourceResponse` for rendering web responses.
- */
-class WebResourceResponse extends RefType {
- WebResourceResponse() { this.hasQualifiedName("android.webkit", "WebResourceResponse") }
-}
-
-/** The `shouldInterceptRequest` method of a class implementing `WebViewClient`. */
-class ShouldInterceptRequestMethod extends Method {
- ShouldInterceptRequestMethod() {
- this.hasName("shouldInterceptRequest") and
- this.getDeclaringType().getASupertype*() instanceof TypeWebViewClient
- }
-}
-
-/** A method call to `WebView.setWebViewClient`. */
-class SetWebViewClientMethodCall extends MethodCall {
- SetWebViewClientMethodCall() {
- this.getMethod().hasName("setWebViewClient") and
- this.getMethod().getDeclaringType().getASupertype*() instanceof TypeWebView
- }
-}
-
-/** A sink representing the data argument of a call to the constructor of `WebResourceResponse`. */
-class WebResourceResponseSink extends DataFlow::Node {
- WebResourceResponseSink() {
- exists(ConstructorCall cc |
- cc.getConstructedType() instanceof WebResourceResponse and
- (
- this.asExpr() = cc.getArgument(2) and cc.getNumArgument() = 3 // WebResourceResponse(String mimeType, String encoding, InputStream data)
- or
- this.asExpr() = cc.getArgument(5) and cc.getNumArgument() = 6 // WebResourceResponse(String mimeType, String encoding, int statusCode, String reasonPhrase, Map responseHeaders, InputStream data)
- )
- )
- }
-}
-
-/**
- * A taint step from the URL argument of `WebView::loadUrl` to the URL/WebResourceRequest parameter of
- * `WebViewClient::shouldInterceptRequest`.
- *
- * TODO: This ought to be a value step when it is targeting the URL parameter,
- * and it ought to check the parameter type in both cases to ensure that we only
- * hit the overloads we intend to.
- */
-private class FetchUrlStep extends AdditionalTaintStep {
- override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
- exists(
- // webview.loadUrl(url) -> webview.setWebViewClient(new WebViewClient() { shouldInterceptRequest(view, url) });
- MethodCall lma, ShouldInterceptRequestMethod im, SetWebViewClientMethodCall sma
- |
- sma.getArgument(0).getType() = im.getDeclaringType().getASupertype*() and
- lma.getMethod() instanceof WebViewLoadUrlMethod and
- lma.getQualifier().getType() = sma.getQualifier().getType() and
- pred.asExpr() = lma.getArgument(0) and
- succ.asParameter() = im.getParameter(1)
- )
- }
-}
diff --git a/java/ql/src/experimental/Security/CWE/CWE-200/InsecureWebResourceResponse.java b/java/ql/src/experimental/Security/CWE/CWE-200/InsecureWebResourceResponse.java
deleted file mode 100644
index 35d86ce2229c..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-200/InsecureWebResourceResponse.java
+++ /dev/null
@@ -1,17 +0,0 @@
-// BAD: no URI validation
-Uri uri = Uri.parse(url);
-FileInputStream inputStream = new FileInputStream(uri.getPath());
-String mimeType = getMimeTypeFromPath(uri.getPath());
-return new WebResourceResponse(mimeType, "UTF-8", inputStream);
-
-
-// GOOD: check for a trusted prefix, ensuring path traversal is not used to erase that prefix:
-// (alternatively use `WebViewAssetsLoader`)
-if (uri.getPath().startsWith("/local_cache/") && !uri.getPath().contains("..")) {
- File cacheFile = new File(getCacheDir(), uri.getLastPathSegment());
- FileInputStream inputStream = new FileInputStream(cacheFile);
- String mimeType = getMimeTypeFromPath(uri.getPath());
- return new WebResourceResponse(mimeType, "UTF-8", inputStream);
-}
-
-return assetLoader.shouldInterceptRequest(request.getUrl());
diff --git a/java/ql/src/experimental/Security/CWE/CWE-200/InsecureWebResourceResponse.qhelp b/java/ql/src/experimental/Security/CWE/CWE-200/InsecureWebResourceResponse.qhelp
deleted file mode 100644
index b89330832f2f..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-200/InsecureWebResourceResponse.qhelp
+++ /dev/null
@@ -1,43 +0,0 @@
-
-
-
-Android provides a WebResourceResponse class, which allows an Android application to behave
-as a web server by handling requests of popular protocols such as http(s), file,
-as well as javascript and returning a response (including status code, content type, content
-encoding, headers and the response body). Improper implementation with insufficient input validation can lead
-to leakage of sensitive configuration files or user data because requests could refer to paths intended to be
-application-private.
-
-
-
-
-
-Unsanitized user-provided URLs must not be used to serve a response directly. When handling a request,
-always validate that the requested file path is not in the receiver's protected directory. Alternatively
-the Android class WebViewAssetLoader can be used, which safely processes data from resources,
-assets or a predefined directory.
-
-
-
-
-
-The following examples show a bad scenario and two good scenarios respectively. In the bad scenario, a
-response is served without path validation. In the good scenario, a response is either served with path
-validation or through the safe WebViewAssetLoader implementation.
-
-
-
-
-
-Oversecured:
-Android: Exploring vulnerabilities in WebResourceResponse.
-
-
-CVE:
-CVE-2014-3502: Cordova apps can potentially leak data to other apps via URL loading.
-
-
-
\ No newline at end of file
diff --git a/java/ql/src/experimental/Security/CWE/CWE-200/InsecureWebResourceResponse.ql b/java/ql/src/experimental/Security/CWE/CWE-200/InsecureWebResourceResponse.ql
deleted file mode 100644
index 7c12d79027fd..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-200/InsecureWebResourceResponse.ql
+++ /dev/null
@@ -1,42 +0,0 @@
-/**
- * @name Insecure Android WebView Resource Response
- * @description An insecure implementation of Android `WebResourceResponse` may lead to leakage of arbitrary
- * sensitive content.
- * @kind path-problem
- * @id java/insecure-webview-resource-response
- * @problem.severity error
- * @tags security
- * experimental
- * external/cwe/cwe-200
- */
-
-import java
-import semmle.code.java.controlflow.Guards
-import semmle.code.java.dataflow.FlowSources
-import semmle.code.java.dataflow.TaintTracking
-import semmle.code.java.security.PathSanitizer
-deprecated import AndroidWebResourceResponse
-deprecated import InsecureWebResourceResponseFlow::PathGraph
-
-deprecated module InsecureWebResourceResponseConfig implements DataFlow::ConfigSig {
- predicate isSource(DataFlow::Node src) { src instanceof ActiveThreatModelSource }
-
- predicate isSink(DataFlow::Node sink) { sink instanceof WebResourceResponseSink }
-
- predicate isBarrier(DataFlow::Node node) { node instanceof PathInjectionSanitizer }
-}
-
-deprecated module InsecureWebResourceResponseFlow =
- TaintTracking::Global;
-
-deprecated query predicate problems(
- DataFlow::Node sinkNode, InsecureWebResourceResponseFlow::PathNode source,
- InsecureWebResourceResponseFlow::PathNode sink, string message1, DataFlow::Node sourceNode,
- string message2
-) {
- InsecureWebResourceResponseFlow::flowPath(source, sink) and
- sinkNode = sink.getNode() and
- message1 = "Leaking arbitrary content in Android from $@." and
- sourceNode = source.getNode() and
- message2 = "this user input"
-}
diff --git a/java/ql/src/experimental/Security/CWE/CWE-200/LoadFileFromAppActivity.java b/java/ql/src/experimental/Security/CWE/CWE-200/LoadFileFromAppActivity.java
deleted file mode 100644
index 8c4d2a2f0da9..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-200/LoadFileFromAppActivity.java
+++ /dev/null
@@ -1,31 +0,0 @@
-public class LoadFileFromAppActivity extends Activity {
- public static final int REQUEST_CODE__SELECT_CONTENT_FROM_APPS = 99;
-
- @Override
- protected void onActivityResult(int requestCode, int resultCode, Intent data) {
- if (requestCode == LoadFileFromAppActivity.REQUEST_CODE__SELECT_CONTENT_FROM_APPS &&
- resultCode == RESULT_OK) {
-
- {
- // BAD: Load file without validation
- loadOfContentFromApps(data, resultCode);
- }
-
- {
- // GOOD: load file with validation
- if (!data.getData().getPath().startsWith("/data/data")) {
- loadOfContentFromApps(data, resultCode);
- }
- }
- }
- }
-
- private void loadOfContentFromApps(Intent contentIntent, int resultCode) {
- Uri streamsToUpload = contentIntent.getData();
- try {
- RandomAccessFile file = new RandomAccessFile(streamsToUpload.getPath(), "r");
- } catch (Exception ex) {
- ex.printStackTrace();
- }
- }
-}
diff --git a/java/ql/src/experimental/Security/CWE/CWE-200/SensitiveAndroidFileLeak.qhelp b/java/ql/src/experimental/Security/CWE/CWE-200/SensitiveAndroidFileLeak.qhelp
deleted file mode 100644
index ca4a7e668ea9..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-200/SensitiveAndroidFileLeak.qhelp
+++ /dev/null
@@ -1,38 +0,0 @@
-
-
-
-The Android API allows to start an activity in another mobile application and receive a result back.
-When starting an activity to retrieve a file from another application, missing input validation can
-lead to leaking of sensitive configuration file or user data because the intent could refer to paths
-which are accessible to the receiver application, but are intended to be application-private.
-
-
-
-
-
-When loading file data from an activity of another application, validate that the file path is not the receiver's
-protected directory, which is a subdirectory of the Android application directory /data/data/.
-
-
-
-
-
-The following examples show a bad situation and a good situation respectively. In the bad situation, a
-file is loaded without path validation. In the good situation, a file is loaded with path validation.
-
-
-
-
-
-Google:
-Android: Interacting with Other Apps.
-
-
-CVE:
-CVE-2021-32695: File Sharing Flow Initiated by a Victim Leaks Sensitive Data to a Malicious App.
-
-
-
\ No newline at end of file
diff --git a/java/ql/src/experimental/Security/CWE/CWE-200/SensitiveAndroidFileLeak.ql b/java/ql/src/experimental/Security/CWE/CWE-200/SensitiveAndroidFileLeak.ql
deleted file mode 100644
index 5bfd5c194349..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-200/SensitiveAndroidFileLeak.ql
+++ /dev/null
@@ -1,89 +0,0 @@
-/**
- * @name Leaking sensitive Android file
- * @description Using a path specified in an Android Intent without validation could leak arbitrary
- * Android configuration file and sensitive user data.
- * @kind path-problem
- * @id java/sensitive-android-file-leak
- * @problem.severity warning
- * @tags security
- * experimental
- * external/cwe/cwe-200
- */
-
-import java
-import semmle.code.java.controlflow.Guards
-deprecated import AndroidFileIntentSink
-deprecated import AndroidFileIntentSource
-deprecated import AndroidFileLeakFlow::PathGraph
-
-private predicate startsWithSanitizer(Guard g, Expr e, boolean branch) {
- exists(MethodCall ma |
- g = ma and
- ma.getMethod().hasName("startsWith") and
- e = [ma.getQualifier(), ma.getQualifier().(MethodCall).getQualifier()] and
- branch = false
- )
-}
-
-deprecated module AndroidFileLeakConfig implements DataFlow::ConfigSig {
- /**
- * Holds if `src` is a read of some Intent-typed variable guarded by a check like
- * `requestCode == someCode`, where `requestCode` is the first
- * argument to `Activity.onActivityResult` and `someCode` is
- * any request code used in a call to `startActivityForResult(intent, someCode)`.
- */
- predicate isSource(DataFlow::Node src) {
- exists(
- OnActivityForResultMethod oafr, ConditionBlock cb, CompileTimeConstantExpr cc,
- VarAccess intentVar
- |
- cb.getCondition()
- .(ValueOrReferenceEqualsExpr)
- .hasOperands(oafr.getParameter(0).getAnAccess(), cc) and
- cc.getIntValue() = any(AndroidFileIntentInput fi).getRequestCode() and
- intentVar.getType() instanceof TypeIntent and
- cb.controls(intentVar.getBasicBlock(), true) and
- src.asExpr() = intentVar
- )
- }
-
- /** Holds if it is a sink of file access in Android. */
- predicate isSink(DataFlow::Node sink) { sink instanceof AndroidFileSink }
-
- predicate isAdditionalFlowStep(DataFlow::Node prev, DataFlow::Node succ) {
- exists(MethodCall aema, AsyncTaskRunInBackgroundMethod arm |
- // fileAsyncTask.execute(params) will invoke doInBackground(params) of FileAsyncTask
- aema.getQualifier().getType() = arm.getDeclaringType() and
- aema.getMethod() instanceof ExecuteAsyncTaskMethod and
- prev.asExpr() = aema.getArgument(aema.getMethod().(ExecuteAsyncTaskMethod).getParamIndex()) and
- succ.asParameter() = arm.getParameter(0)
- )
- or
- exists(MethodCall csma, ServiceOnStartCommandMethod ssm, ClassInstanceExpr ce |
- // An intent passed to startService will later be passed to the onStartCommand event of the corresponding service
- csma.getMethod() instanceof ContextStartServiceMethod and
- ce.getConstructedType() instanceof TypeIntent and // Intent intent = new Intent(context, FileUploader.class);
- ce.getArgument(1).(TypeLiteral).getReferencedType() = ssm.getDeclaringType() and
- DataFlow::localExprFlow(ce, csma.getArgument(0)) and // context.startService(intent);
- prev.asExpr() = csma.getArgument(0) and
- succ.asParameter() = ssm.getParameter(0) // public int onStartCommand(Intent intent, int flags, int startId) {...} in FileUploader
- )
- }
-
- predicate isBarrier(DataFlow::Node node) {
- node = DataFlow::BarrierGuard::getABarrierNode()
- }
-}
-
-deprecated module AndroidFileLeakFlow = TaintTracking::Global;
-
-deprecated query predicate problems(
- DataFlow::Node sinkNode, AndroidFileLeakFlow::PathNode source, AndroidFileLeakFlow::PathNode sink,
- string message1, DataFlow::Node sourceNode, string message2
-) {
- AndroidFileLeakFlow::flowPath(source, sink) and
- sinkNode = sink.getNode() and
- message1 = "Leaking arbitrary Android file from $@." and
- sourceNode = source.getNode() and
- message2 = "this user input"
-}
diff --git a/java/ql/src/experimental/Security/CWE/CWE-208/NonConstantTimeCheckOnSignatureQuery.qll b/java/ql/src/experimental/Security/CWE/CWE-208/NonConstantTimeCheckOnSignatureQuery.qll
deleted file mode 100644
index 2fb6de113b76..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-208/NonConstantTimeCheckOnSignatureQuery.qll
+++ /dev/null
@@ -1,320 +0,0 @@
-/**
- * Provides classes and predicates for queries that detect timing attacks.
- */
-deprecated module;
-
-import semmle.code.java.controlflow.Guards
-import semmle.code.java.dataflow.TaintTracking
-import semmle.code.java.dataflow.FlowSources
-
-/** A method call that produces cryptographic result. */
-abstract private class ProduceCryptoCall extends MethodCall {
- Expr output;
-
- /** Gets the result of cryptographic operation. */
- Expr output() { result = output }
-
- /** Gets a type of cryptographic operation such as MAC, signature or ciphertext. */
- abstract string getResultType();
-}
-
-/** A method call that produces a MAC. */
-private class ProduceMacCall extends ProduceCryptoCall {
- ProduceMacCall() {
- this.getMethod().getDeclaringType().hasQualifiedName("javax.crypto", "Mac") and
- (
- this.getMethod().hasStringSignature(["doFinal()", "doFinal(byte[])"]) and this = output
- or
- this.getMethod().hasStringSignature("doFinal(byte[], int)") and this.getArgument(0) = output
- )
- }
-
- override string getResultType() { result = "MAC" }
-}
-
-/** A method call that produces a signature. */
-private class ProduceSignatureCall extends ProduceCryptoCall {
- ProduceSignatureCall() {
- this.getMethod().getDeclaringType().hasQualifiedName("java.security", "Signature") and
- (
- this.getMethod().hasStringSignature("sign()") and this = output
- or
- this.getMethod().hasStringSignature("sign(byte[], int, int)") and this.getArgument(0) = output
- )
- }
-
- override string getResultType() { result = "signature" }
-}
-
-/**
- * A config that tracks data flow from initializing a cipher for encryption
- * to producing a ciphertext using this cipher.
- */
-private module InitializeEncryptorConfig implements DataFlow::ConfigSig {
- predicate isSource(DataFlow::Node source) {
- exists(MethodCall ma |
- ma.getMethod().hasQualifiedName("javax.crypto", "Cipher", "init") and
- ma.getArgument(0).(VarAccess).getVariable().hasName("ENCRYPT_MODE") and
- ma.getQualifier() = source.asExpr()
- )
- }
-
- predicate isSink(DataFlow::Node sink) {
- exists(MethodCall ma |
- ma.getMethod().hasQualifiedName("javax.crypto", "Cipher", "doFinal") and
- ma.getQualifier() = sink.asExpr()
- )
- }
-}
-
-private module InitializeEncryptorFlow = DataFlow::Global;
-
-/** A method call that produces a ciphertext. */
-private class ProduceCiphertextCall extends ProduceCryptoCall {
- ProduceCiphertextCall() {
- exists(Method m | m = this.getMethod() |
- m.getDeclaringType().hasQualifiedName("javax.crypto", "Cipher") and
- (
- m.hasStringSignature(["doFinal()", "doFinal(byte[])", "doFinal(byte[], int, int)"]) and
- this = output
- or
- m.hasStringSignature("doFinal(byte[], int)") and this.getArgument(0) = output
- or
- m.hasStringSignature([
- "doFinal(byte[], int, int, byte[])", "doFinal(byte[], int, int, byte[], int)"
- ]) and
- this.getArgument(3) = output
- or
- m.hasStringSignature("doFinal(ByteBuffer, ByteBuffer)") and
- this.getArgument(1) = output
- )
- ) and
- InitializeEncryptorFlow::flowToExpr(this.getQualifier())
- }
-
- override string getResultType() { result = "ciphertext" }
-}
-
-/** Holds if `fromNode` to `toNode` is a dataflow step that updates a cryptographic operation. */
-private predicate updateCryptoOperationStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
- exists(MethodCall call, Method m |
- m = call.getMethod() and
- call.getQualifier() = toNode.asExpr() and
- call.getArgument(0) = fromNode.asExpr()
- |
- m.hasQualifiedName("java.security", "Signature", "update")
- or
- m.hasQualifiedName("javax.crypto", ["Mac", "Cipher"], "update")
- or
- m.hasQualifiedName("javax.crypto", ["Mac", "Cipher"], "doFinal") and
- not m.hasStringSignature("doFinal(byte[], int)")
- )
-}
-
-/** Holds if `fromNode` to `toNode` is a dataflow step that creates a hash. */
-private predicate createMessageDigestStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
- exists(MethodCall ma, Method m | m = ma.getMethod() |
- m.getDeclaringType().hasQualifiedName("java.security", "MessageDigest") and
- m.hasStringSignature("digest()") and
- ma.getQualifier() = fromNode.asExpr() and
- ma = toNode.asExpr()
- )
- or
- exists(MethodCall ma, Method m | m = ma.getMethod() |
- m.getDeclaringType().hasQualifiedName("java.security", "MessageDigest") and
- m.hasStringSignature("digest(byte[], int, int)") and
- ma.getQualifier() = fromNode.asExpr() and
- ma.getArgument(0) = toNode.asExpr()
- )
- or
- exists(MethodCall ma, Method m | m = ma.getMethod() |
- m.getDeclaringType().hasQualifiedName("java.security", "MessageDigest") and
- m.hasStringSignature("digest(byte[])") and
- ma.getArgument(0) = fromNode.asExpr() and
- ma = toNode.asExpr()
- )
-}
-
-/** Holds if `fromNode` to `toNode` is a dataflow step that updates a hash. */
-private predicate updateMessageDigestStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
- exists(MethodCall ma, Method m | m = ma.getMethod() |
- m.hasQualifiedName("java.security", "MessageDigest", "update") and
- ma.getArgument(0) = fromNode.asExpr() and
- ma.getQualifier() = toNode.asExpr()
- )
-}
-
-/**
- * A config that tracks data flow from remote user input to a cryptographic operation
- * such as cipher, MAC or signature.
- */
-private module UserInputInCryptoOperationConfig implements DataFlow::ConfigSig {
- predicate isSource(DataFlow::Node source) { source instanceof ActiveThreatModelSource }
-
- predicate isSink(DataFlow::Node sink) {
- exists(ProduceCryptoCall call | call.getQualifier() = sink.asExpr())
- }
-
- predicate isAdditionalFlowStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
- updateCryptoOperationStep(fromNode, toNode)
- or
- createMessageDigestStep(fromNode, toNode)
- or
- updateMessageDigestStep(fromNode, toNode)
- }
-}
-
-/**
- * Taint-tracking flow from remote user input to a cryptographic operation
- * such as cipher, MAC or signature.
- */
-private module UserInputInCryptoOperationFlow =
- TaintTracking::Global;
-
-/** A source that produces result of cryptographic operation. */
-class CryptoOperationSource extends DataFlow::Node {
- ProduceCryptoCall call;
-
- CryptoOperationSource() { call.output() = this.asExpr() }
-
- /** Holds if remote user input was used in the cryptographic operation. */
- predicate includesUserInput() {
- exists(UserInputInCryptoOperationFlow::PathNode sink |
- UserInputInCryptoOperationFlow::flowPath(_, sink)
- |
- sink.getNode().asExpr() = call.getQualifier()
- )
- }
-
- /** Gets a method call that produces cryptographic result. */
- ProduceCryptoCall getCall() { result = call }
-}
-
-/** Methods that use a non-constant-time algorithm for comparing inputs. */
-private class NonConstantTimeEqualsCall extends MethodCall {
- NonConstantTimeEqualsCall() {
- this.getMethod()
- .hasQualifiedName("java.lang", "String", ["equals", "contentEquals", "equalsIgnoreCase"]) or
- this.getMethod().hasQualifiedName("java.nio", "ByteBuffer", ["equals", "compareTo"])
- }
-}
-
-/** A static method that uses a non-constant-time algorithm for comparing inputs. */
-private class NonConstantTimeComparisonCall extends StaticMethodCall {
- NonConstantTimeComparisonCall() {
- this.getMethod().hasQualifiedName("java.util", "Arrays", ["equals", "deepEquals"]) or
- this.getMethod().hasQualifiedName("java.util", "Objects", "deepEquals") or
- this.getMethod()
- .hasQualifiedName("org.apache.commons.lang3", "StringUtils",
- ["equals", "equalsAny", "equalsAnyIgnoreCase", "equalsIgnoreCase"])
- }
-}
-
-/**
- * A config that tracks data flow from remote user input to methods
- * that compare inputs using a non-constant-time algorithm.
- */
-private module UserInputInComparisonConfig implements DataFlow::ConfigSig {
- predicate isSource(DataFlow::Node source) { source instanceof ActiveThreatModelSource }
-
- predicate isSink(DataFlow::Node sink) {
- exists(NonConstantTimeEqualsCall call |
- sink.asExpr() = [call.getAnArgument(), call.getQualifier()]
- )
- or
- exists(NonConstantTimeComparisonCall call | sink.asExpr() = call.getAnArgument())
- }
-}
-
-private module UserInputInComparisonFlow = TaintTracking::Global;
-
-/** Holds if `expr` looks like a constant. */
-private predicate looksLikeConstant(Expr expr) {
- expr.isCompileTimeConstant()
- or
- expr.(VarAccess).getVariable().isFinal() and expr.getType() instanceof TypeString
-}
-
-/**
- * Holds if `firstObject` and `secondObject` are compared using a method
- * that does not use a constant-time algorithm, for example, `String.equals()`.
- */
-private predicate isNonConstantTimeEqualsCall(Expr firstObject, Expr secondObject) {
- exists(NonConstantTimeEqualsCall call |
- firstObject = call.getQualifier() and
- secondObject = call.getAnArgument()
- or
- firstObject = call.getAnArgument() and
- secondObject = call.getQualifier()
- )
-}
-
-/**
- * Holds if `firstInput` and `secondInput` are compared using a static method
- * that does not use a constant-time algorithm, for example, `Arrays.equals()`.
- */
-private predicate isNonConstantTimeComparisonCall(Expr firstInput, Expr secondInput) {
- exists(NonConstantTimeComparisonCall call |
- firstInput = call.getArgument(0) and secondInput = call.getArgument(1)
- or
- firstInput = call.getArgument(1) and secondInput = call.getArgument(0)
- )
-}
-
-/**
- * Holds if there is a fast-fail check while comparing `firstArray` and `secondArray`.
- */
-private predicate existsFailFastCheck(Expr firstArray, Expr secondArray) {
- exists(
- Guard guard, EqualityTest eqTest, boolean branch, Stmt fastFailingStmt,
- ArrayAccess firstArrayAccess, ArrayAccess secondArrayAccess
- |
- guard = eqTest and
- // For `==` false branch is fail fast; for `!=` true branch is fail fast
- branch = eqTest.polarity().booleanNot() and
- (
- fastFailingStmt instanceof ReturnStmt or
- fastFailingStmt instanceof BreakStmt or
- fastFailingStmt instanceof ThrowStmt
- ) and
- guard.controls(fastFailingStmt.getBasicBlock(), branch) and
- DataFlow::localExprFlow(firstArrayAccess, eqTest.getLeftOperand()) and
- DataFlow::localExprFlow(secondArrayAccess, eqTest.getRightOperand())
- |
- firstArrayAccess.getArray() = firstArray and secondArray = secondArrayAccess
- or
- secondArrayAccess.getArray() = firstArray and secondArray = firstArrayAccess
- )
-}
-
-/** A sink that compares input using a non-constant-time algorithm. */
-class NonConstantTimeComparisonSink extends DataFlow::Node {
- Expr anotherParameter;
-
- NonConstantTimeComparisonSink() {
- (
- isNonConstantTimeEqualsCall(this.asExpr(), anotherParameter)
- or
- isNonConstantTimeComparisonCall(this.asExpr(), anotherParameter)
- or
- existsFailFastCheck(this.asExpr(), anotherParameter)
- ) and
- not looksLikeConstant(anotherParameter)
- }
-
- /** Holds if remote user input was used in the comparison. */
- predicate includesUserInput() { UserInputInComparisonFlow::flowToExpr(anotherParameter) }
-}
-
-/**
- * A configuration that tracks data flow from cryptographic operations
- * to methods that compare data using a non-constant-time algorithm.
- */
-module NonConstantTimeCryptoComparisonConfig implements DataFlow::ConfigSig {
- predicate isSource(DataFlow::Node source) { source instanceof CryptoOperationSource }
-
- predicate isSink(DataFlow::Node sink) { sink instanceof NonConstantTimeComparisonSink }
-}
-
-module NonConstantTimeCryptoComparisonFlow =
- TaintTracking::Global;
diff --git a/java/ql/src/experimental/Security/CWE/CWE-208/PossibleTimingAttackAgainstSignature.qhelp b/java/ql/src/experimental/Security/CWE/CWE-208/PossibleTimingAttackAgainstSignature.qhelp
deleted file mode 100644
index aee0196686f7..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-208/PossibleTimingAttackAgainstSignature.qhelp
+++ /dev/null
@@ -1,4 +0,0 @@
-
-
-
diff --git a/java/ql/src/experimental/Security/CWE/CWE-208/PossibleTimingAttackAgainstSignature.ql b/java/ql/src/experimental/Security/CWE/CWE-208/PossibleTimingAttackAgainstSignature.ql
deleted file mode 100644
index 925652d2fd06..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-208/PossibleTimingAttackAgainstSignature.ql
+++ /dev/null
@@ -1,31 +0,0 @@
-/**
- * @name Possible timing attack against signature validation
- * @description When checking a signature over a message, a constant-time algorithm should be used.
- * Otherwise, there is a risk of a timing attack that allows an attacker
- * to forge a valid signature for an arbitrary message. For a successful attack,
- * the attacker has to be able to send to the validation procedure both the message and the signature.
- * @kind path-problem
- * @problem.severity warning
- * @precision medium
- * @id java/possible-timing-attack-against-signature
- * @tags security
- * experimental
- * external/cwe/cwe-208
- */
-
-import java
-import semmle.code.java.dataflow.DataFlow
-deprecated import NonConstantTimeCheckOnSignatureQuery
-deprecated import NonConstantTimeCryptoComparisonFlow::PathGraph
-
-deprecated query predicate problems(
- DataFlow::Node sinkNode, NonConstantTimeCryptoComparisonFlow::PathNode source,
- NonConstantTimeCryptoComparisonFlow::PathNode sink, string message1,
- NonConstantTimeCryptoComparisonFlow::PathNode source0, string message2
-) {
- NonConstantTimeCryptoComparisonFlow::flowPath(source, sink) and
- sinkNode = sink.getNode() and
- message1 = "Possible timing attack against $@ validation." and
- source = source0 and
- message2 = source.getNode().(CryptoOperationSource).getCall().getResultType()
-}
diff --git a/java/ql/src/experimental/Security/CWE/CWE-208/SafeMacComparison.java b/java/ql/src/experimental/Security/CWE/CWE-208/SafeMacComparison.java
deleted file mode 100644
index fbdb6131a5c3..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-208/SafeMacComparison.java
+++ /dev/null
@@ -1,9 +0,0 @@
-public boolean validate(HttpRequest request, SecretKey key) throws Exception {
- byte[] message = getMessageFrom(request);
- byte[] signature = getSignatureFrom(request);
-
- Mac mac = Mac.getInstance("HmacSHA256");
- mac.init(new SecretKeySpec(key.getEncoded(), "HmacSHA256"));
- byte[] actual = mac.doFinal(message);
- return MessageDigest.isEqual(signature, actual);
-}
\ No newline at end of file
diff --git a/java/ql/src/experimental/Security/CWE/CWE-208/TimingAttackAgainstHeader.java b/java/ql/src/experimental/Security/CWE/CWE-208/TimingAttackAgainstHeader.java
deleted file mode 100644
index 52ad7b3604bb..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-208/TimingAttackAgainstHeader.java
+++ /dev/null
@@ -1,20 +0,0 @@
-import javax.servlet.http.HttpServletRequest;
-import java.nio.charset.StandardCharsets;
-import java.security.MessageDigest;
-import java.lang.String;
-
-
-public class Test {
- private boolean UnsafeComparison(HttpServletRequest request) {
- String Key = "secret";
- return Key.equals(request.getHeader("X-Auth-Token"));
- }
-
- private boolean safeComparison(HttpServletRequest request) {
- String token = request.getHeader("X-Auth-Token");
- String Key = "secret";
- return MessageDigest.isEqual(Key.getBytes(StandardCharsets.UTF_8), token.getBytes(StandardCharsets.UTF_8));
- }
-
-}
-
diff --git a/java/ql/src/experimental/Security/CWE/CWE-208/TimingAttackAgainstHeader.qhelp b/java/ql/src/experimental/Security/CWE/CWE-208/TimingAttackAgainstHeader.qhelp
deleted file mode 100644
index d447d398fb93..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-208/TimingAttackAgainstHeader.qhelp
+++ /dev/null
@@ -1,28 +0,0 @@
-
-
-
-
-
-A constant-time algorithm should be used for checking the value of sensitive headers.
-In other words, the comparison time should not depend on the content of the input.
-Otherwise timing information could be used to infer the header's expected, secret value.
-
-
-
-
-
-
-Use MessageDigest.isEqual() method to check the value of headers.
-If this method is used, then the calculation time depends only on the length of input byte arrays,
-and does not depend on the contents of the arrays.
-
-
-
-
-The following example uses String.equals() method for validating a csrf token.
-This method implements a non-constant-time algorithm. The example also demonstrates validation using a safe constant-time algorithm.
-
-
-
-
diff --git a/java/ql/src/experimental/Security/CWE/CWE-208/TimingAttackAgainstHeader.ql b/java/ql/src/experimental/Security/CWE/CWE-208/TimingAttackAgainstHeader.ql
deleted file mode 100644
index ebb8ebafa3ed..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-208/TimingAttackAgainstHeader.ql
+++ /dev/null
@@ -1,78 +0,0 @@
-/**
- * @name Timing attack against header value
- * @description Use of a non-constant-time verification routine to check the value of an HTTP header,
- * possibly allowing a timing attack to infer the header's expected value.
- * @kind path-problem
- * @problem.severity error
- * @precision high
- * @id java/timing-attack-against-headers-value
- * @tags security
- * experimental
- * external/cwe/cwe-208
- */
-
-import java
-import semmle.code.java.dataflow.FlowSources
-import semmle.code.java.dataflow.TaintTracking
-import NonConstantTimeComparisonFlow::PathGraph
-
-/** A static method that uses a non-constant-time algorithm for comparing inputs. */
-private class NonConstantTimeComparisonCall extends StaticMethodCall {
- NonConstantTimeComparisonCall() {
- this.getMethod()
- .hasQualifiedName("org.apache.commons.lang3", "StringUtils",
- ["equals", "equalsAny", "equalsAnyIgnoreCase", "equalsIgnoreCase"])
- }
-}
-
-/** Methods that use a non-constant-time algorithm for comparing inputs. */
-private class NonConstantTimeEqualsCall extends MethodCall {
- NonConstantTimeEqualsCall() {
- this.getMethod()
- .hasQualifiedName("java.lang", "String", ["equals", "contentEquals", "equalsIgnoreCase"])
- }
-}
-
-private predicate isNonConstantEqualsCallArgument(Expr e) {
- exists(NonConstantTimeEqualsCall call | e = [call.getQualifier(), call.getArgument(0)])
-}
-
-private predicate isNonConstantComparisonCallArgument(Expr p) {
- exists(NonConstantTimeComparisonCall call | p = [call.getArgument(0), call.getArgument(1)])
-}
-
-class ClientSuppliedIpTokenCheck extends DataFlow::Node {
- ClientSuppliedIpTokenCheck() {
- exists(MethodCall ma |
- ma.getMethod().hasName("getHeader") and
- ma.getArgument(0).(CompileTimeConstantExpr).getStringValue().toLowerCase() in [
- "x-auth-token", "x-csrf-token", "http_x_csrf_token", "x-csrf-param", "x-csrf-header",
- "http_x_csrf_token", "x-api-key", "authorization", "proxy-authorization"
- ] and
- ma = this.asExpr()
- )
- }
-}
-
-module NonConstantTimeComparisonConfig implements DataFlow::ConfigSig {
- predicate isSource(DataFlow::Node source) { source instanceof ClientSuppliedIpTokenCheck }
-
- predicate isSink(DataFlow::Node sink) {
- isNonConstantEqualsCallArgument(sink.asExpr()) or
- isNonConstantComparisonCallArgument(sink.asExpr())
- }
-}
-
-module NonConstantTimeComparisonFlow = TaintTracking::Global;
-
-deprecated query predicate problems(
- DataFlow::Node sinkNode, NonConstantTimeComparisonFlow::PathNode source,
- NonConstantTimeComparisonFlow::PathNode sink, string message1, DataFlow::Node sourceNode,
- string message2
-) {
- NonConstantTimeComparisonFlow::flowPath(source, sink) and
- sinkNode = sink.getNode() and
- message1 = "Possible timing attack against $@ validation." and
- sourceNode = source.getNode() and
- message2 = "client-supplied token"
-}
diff --git a/java/ql/src/experimental/Security/CWE/CWE-208/TimingAttackAgainstSignature.qhelp b/java/ql/src/experimental/Security/CWE/CWE-208/TimingAttackAgainstSignature.qhelp
deleted file mode 100644
index 7815312414ae..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-208/TimingAttackAgainstSignature.qhelp
+++ /dev/null
@@ -1,55 +0,0 @@
-
-
-
-
-
-A constant-time algorithm should be used for checking a MAC or a digital signature.
-In other words, the comparison time should not depend on the content of the input.
-Otherwise, an attacker may be able to forge a valid signature for an arbitrary message
-by running a timing attack if they can send to the validation procedure
-both the message and the signature. A successful attack can result in authentication bypass.
-
-
-
-
-
-Use MessageDigest.isEqual() method to check MACs and signatures.
-If this method is used, then the calculation time depends only on the length of input byte arrays,
-and does not depend on the contents of the arrays.
-
-
-
-
-
-The following example uses Arrays.equals() method for validating a MAC over a message.
-This method implements a non-constant-time algorithm.
-Both the message and the signature come from an untrusted HTTP request:
-
-
-The next example uses a safe constant-time algorithm for validating a MAC:
-
-
-
-
-
- Wikipedia:
- Timing attack.
-
-
- Coursera:
- Timing attacks on MAC verification
-
-
- NCC Group:
- Time Trial: Racing Towards Practical Remote Timing Attacks
-
-
- Java API Specification:
- MessageDigest.isEqual() method
-
-
-
-
diff --git a/java/ql/src/experimental/Security/CWE/CWE-208/TimingAttackAgainstSignature.ql b/java/ql/src/experimental/Security/CWE/CWE-208/TimingAttackAgainstSignature.ql
deleted file mode 100644
index 2b79fb7a17c7..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-208/TimingAttackAgainstSignature.ql
+++ /dev/null
@@ -1,36 +0,0 @@
-/**
- * @name Timing attack against signature validation
- * @description When checking a signature over a message, a constant-time algorithm should be used.
- * Otherwise, an attacker may be able to forge a valid signature for an arbitrary message
- * by running a timing attack if they can send to the validation procedure
- * both the message and the signature.
- * A successful attack can result in authentication bypass.
- * @kind path-problem
- * @problem.severity error
- * @precision high
- * @id java/timing-attack-against-signature
- * @tags security
- * experimental
- * external/cwe/cwe-208
- */
-
-import java
-import semmle.code.java.dataflow.DataFlow
-deprecated import NonConstantTimeCheckOnSignatureQuery
-deprecated import NonConstantTimeCryptoComparisonFlow::PathGraph
-
-deprecated query predicate problems(
- DataFlow::Node sinkNode, NonConstantTimeCryptoComparisonFlow::PathNode source,
- NonConstantTimeCryptoComparisonFlow::PathNode sink, string message1,
- NonConstantTimeCryptoComparisonFlow::PathNode source0, string message2
-) {
- NonConstantTimeCryptoComparisonFlow::flowPath(source, sink) and
- (
- source.getNode().(CryptoOperationSource).includesUserInput() and
- sinkNode.(NonConstantTimeComparisonSink).includesUserInput()
- ) and
- sinkNode = sink.getNode() and
- message1 = "Timing attack against $@ validation." and
- source = source0 and
- message2 = source.getNode().(CryptoOperationSource).getCall().getResultType()
-}
diff --git a/java/ql/src/experimental/Security/CWE/CWE-208/UnsafeMacComparison.java b/java/ql/src/experimental/Security/CWE/CWE-208/UnsafeMacComparison.java
deleted file mode 100644
index 1785ff2e7c6c..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-208/UnsafeMacComparison.java
+++ /dev/null
@@ -1,9 +0,0 @@
-public boolean validate(HttpRequest request, SecretKey key) throws Exception {
- byte[] message = getMessageFrom(request);
- byte[] signature = getSignatureFrom(request);
-
- Mac mac = Mac.getInstance("HmacSHA256");
- mac.init(new SecretKeySpec(key.getEncoded(), "HmacSHA256"));
- byte[] actual = mac.doFinal(message);
- return Arrays.equals(signature, actual);
-}
\ No newline at end of file
diff --git a/java/ql/src/experimental/Security/CWE/CWE-295/JxBrowserWithoutCertValidation.java b/java/ql/src/experimental/Security/CWE/CWE-295/JxBrowserWithoutCertValidation.java
deleted file mode 100644
index e45039e0fef1..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-295/JxBrowserWithoutCertValidation.java
+++ /dev/null
@@ -1,23 +0,0 @@
-public static void main(String[] args) {
- {
- Browser browser = new Browser();
- browser.loadURL("https://example.com");
- // no further calls
- // BAD: The browser ignores any certificate error by default!
- }
-
- {
- Browser browser = new Browser();
- browser.setLoadHandler(new LoadHandler() {
- public boolean onLoad(LoadParams params) {
- return true;
- }
-
- public boolean onCertificateError(CertificateErrorParams params){
- return true; // GOOD: This means that loading will be cancelled on certificate errors
- }
- }); // GOOD: A secure `LoadHandler` is used.
- browser.loadURL("https://example.com");
-
- }
-}
\ No newline at end of file
diff --git a/java/ql/src/experimental/Security/CWE/CWE-295/JxBrowserWithoutCertValidation.qhelp b/java/ql/src/experimental/Security/CWE/CWE-295/JxBrowserWithoutCertValidation.qhelp
deleted file mode 100644
index 31e427611407..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-295/JxBrowserWithoutCertValidation.qhelp
+++ /dev/null
@@ -1,32 +0,0 @@
-
-
-
-
-JxBrowser is a Java library that allows to embed the Chromium browser inside Java applications.
-Versions smaller than 6.24 by default ignore any HTTPS certificate errors thereby allowing man-in-the-middle attacks.
-
-
-
-
-Do either of these:
-
- - Update to version 6.24 or 7.x.x as these correctly reject certificate errors by default.
- - Add a custom implementation of the
LoadHandler interface whose onCertificateError method always returns true indicating that loading should be cancelled.
- Then use the setLoadHandler method with your custom LoadHandler on every Browser you use.
-
-
-
-
-The following two examples show two ways of using a Browser. In the 'BAD' case,
-all certificate errors are ignored. In the 'GOOD' case, certificate errors are rejected.
-
-
-
-Teamdev:
-
-Changelog of JxBrowser 6.24.
-
-
diff --git a/java/ql/src/experimental/Security/CWE/CWE-295/JxBrowserWithoutCertValidation.ql b/java/ql/src/experimental/Security/CWE/CWE-295/JxBrowserWithoutCertValidation.ql
deleted file mode 100644
index 48c49d5c071d..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-295/JxBrowserWithoutCertValidation.ql
+++ /dev/null
@@ -1,91 +0,0 @@
-/**
- * @name JxBrowser with disabled certificate validation
- * @description Insecure configuration of JxBrowser disables certificate
- * validation making the app vulnerable to man-in-the-middle
- * attacks.
- * @kind problem
- * @problem.severity warning
- * @precision medium
- * @id java/jxbrowser/disabled-certificate-validation
- * @tags security
- * experimental
- * external/cwe/cwe-295
- */
-
-import java
-import semmle.code.java.security.Encryption
-import semmle.code.java.dataflow.DataFlow
-
-/*
- * This query is version specific to JxBrowser < 6.24. The version is indirectly detected.
- * In version 6.x.x the `Browser` class is in a different package compared to version 7.x.x.
- */
-
-/**
- * Holds if a safe JxBrowser 6.x.x version is used, such as version 6.24.
- * This is detected by the the presence of the `addBoundsListener` in the `Browser` class.
- */
-private predicate isSafeJxBrowserVersion() {
- exists(Method m | m.getDeclaringType() instanceof JxBrowser | m.hasName("addBoundsListener"))
-}
-
-/** The `com.teamdev.jxbrowser.chromium.Browser` class. */
-private class JxBrowser extends RefType {
- JxBrowser() { this.hasQualifiedName("com.teamdev.jxbrowser.chromium", "Browser") }
-}
-
-/** The `setLoadHandler` method on the `com.teamdev.jxbrowser.chromium.Browser` class. */
-private class JxBrowserSetLoadHandler extends Method {
- JxBrowserSetLoadHandler() {
- this.hasName("setLoadHandler") and this.getDeclaringType() instanceof JxBrowser
- }
-}
-
-/** The `com.teamdev.jxbrowser.chromium.LoadHandler` interface. */
-private class JxBrowserLoadHandler extends RefType {
- JxBrowserLoadHandler() { this.hasQualifiedName("com.teamdev.jxbrowser.chromium", "LoadHandler") }
-}
-
-private predicate isOnCertificateErrorMethodSafe(Method m) {
- forex(ReturnStmt rs | rs.getEnclosingCallable() = m |
- rs.getResult().(CompileTimeConstantExpr).getBooleanValue() = true
- )
-}
-
-/** A class that securely implements the `com.teamdev.jxbrowser.chromium.LoadHandler` interface. */
-private class JxBrowserSafeLoadHandler extends RefType {
- JxBrowserSafeLoadHandler() {
- this.getASupertype() instanceof JxBrowserLoadHandler and
- exists(Method m | m.hasName("onCertificateError") and m.getDeclaringType() = this |
- isOnCertificateErrorMethodSafe(m)
- )
- }
-}
-
-/**
- * Models flow from the source `new Browser()` to a sink `browser.setLoadHandler(loadHandler)` where `loadHandler`
- * has been determined to be safe.
- */
-private module JxBrowserFlowConfig implements DataFlow::ConfigSig {
- predicate isSource(DataFlow::Node src) {
- exists(ClassInstanceExpr newJxBrowser | newJxBrowser.getConstructedType() instanceof JxBrowser |
- newJxBrowser = src.asExpr()
- )
- }
-
- predicate isSink(DataFlow::Node sink) {
- exists(MethodCall ma | ma.getMethod() instanceof JxBrowserSetLoadHandler |
- ma.getArgument(0).getType() instanceof JxBrowserSafeLoadHandler and
- ma.getQualifier() = sink.asExpr()
- )
- }
-}
-
-private module JxBrowserFlow = DataFlow::Global;
-
-deprecated query predicate problems(DataFlow::Node src, string message) {
- JxBrowserFlowConfig::isSource(src) and
- not JxBrowserFlow::flow(src, _) and
- not isSafeJxBrowserVersion() and
- message = "This JxBrowser instance may not check HTTPS certificates."
-}
diff --git a/java/ql/src/experimental/Security/CWE/CWE-297/CheckedHostnameVerification.java b/java/ql/src/experimental/Security/CWE/CWE-297/CheckedHostnameVerification.java
deleted file mode 100644
index 9f17b1fc9726..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-297/CheckedHostnameVerification.java
+++ /dev/null
@@ -1,10 +0,0 @@
-public SSLSocket connect(String host, int port, HostnameVerifier verifier) {
- SSLSocket socket = (SSLSocket) SSLSocketFactory.getDefault().createSocket(host, port);
- socket.startHandshake();
- boolean successful = verifier.verify(host, socket.getSession());
- if (!successful) {
- socket.close();
- throw new SSLException("Oops! Hostname verification failed!");
- }
- return socket;
-}
\ No newline at end of file
diff --git a/java/ql/src/experimental/Security/CWE/CWE-297/IgnoredHostnameVerification.java b/java/ql/src/experimental/Security/CWE/CWE-297/IgnoredHostnameVerification.java
deleted file mode 100644
index 25436051dbca..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-297/IgnoredHostnameVerification.java
+++ /dev/null
@@ -1,6 +0,0 @@
-public SSLSocket connect(String host, int port, HostnameVerifier verifier) {
- SSLSocket socket = (SSLSocket) SSLSocketFactory.getDefault().createSocket(host, port);
- socket.startHandshake();
- verifier.verify(host, socket.getSession());
- return socket;
-}
\ No newline at end of file
diff --git a/java/ql/src/experimental/Security/CWE/CWE-297/IgnoredHostnameVerification.qhelp b/java/ql/src/experimental/Security/CWE/CWE-297/IgnoredHostnameVerification.qhelp
deleted file mode 100644
index e5756d9caee9..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-297/IgnoredHostnameVerification.qhelp
+++ /dev/null
@@ -1,42 +0,0 @@
-
-
-
-
-
-The method HostnameVerifier.verify() checks that the hostname from the server's certificate
-matches the server hostname after an HTTPS connection is established.
-The method returns true if the hostname is acceptable and false otherwise. The contract of the method
-does not require it to throw an exception if the verification failed.
-Therefore, a caller has to check the result and drop the connection if the hostname verification failed.
-Otherwise, an attacker may be able to implement a man-in-the-middle attack and impersonate the server.
-
-
-
-
-
-Always check the result of HostnameVerifier.verify() and drop the connection
-if the method returns false.
-
-
-
-
-
-In the following example, the method HostnameVerifier.verify() is called but its result is ignored.
-As a result, no hostname verification actually happens.
-
-
-In the next example, the result of the HostnameVerifier.verify() method is checked
-and an exception is thrown if the verification failed.
-
-
-
-
-
- Java API Specification:
- HostnameVerifier.verify() method.
-
-
-
diff --git a/java/ql/src/experimental/Security/CWE/CWE-297/IgnoredHostnameVerification.ql b/java/ql/src/experimental/Security/CWE/CWE-297/IgnoredHostnameVerification.ql
deleted file mode 100644
index 981d2287c03d..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-297/IgnoredHostnameVerification.ql
+++ /dev/null
@@ -1,31 +0,0 @@
-/**
- * @name Ignored result of hostname verification
- * @description The method HostnameVerifier.verify() returns a result of hostname verification.
- * A caller has to check the result and drop the connection if the verification failed.
- * @kind problem
- * @problem.severity error
- * @precision high
- * @id java/ignored-hostname-verification
- * @tags security
- * experimental
- * external/cwe/cwe-297
- */
-
-import java
-import semmle.code.java.security.Encryption
-
-/** A `HostnameVerifier.verify()` call that is not wrapped in another `HostnameVerifier`. */
-private class HostnameVerificationCall extends MethodCall {
- HostnameVerificationCall() {
- this.getMethod() instanceof HostnameVerifierVerify and
- not this.getCaller() instanceof HostnameVerifierVerify
- }
-
- /** Holds if the result of the call is not used. */
- predicate isIgnored() { this instanceof ValueDiscardingExpr }
-}
-
-deprecated query predicate problems(HostnameVerificationCall verification, string message) {
- verification.isIgnored() and
- message = "Ignored result of hostname verification."
-}
diff --git a/java/ql/src/experimental/Security/CWE/CWE-297/InsecureLdapEndpoint.java b/java/ql/src/experimental/Security/CWE/CWE-297/InsecureLdapEndpoint.java
deleted file mode 100644
index 6f7494f88117..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-297/InsecureLdapEndpoint.java
+++ /dev/null
@@ -1,18 +0,0 @@
-public class InsecureLdapEndpoint {
- public Hashtable createConnectionEnv() {
- Hashtable env = new Hashtable();
- env.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory");
- env.put(Context.PROVIDER_URL, "ldaps://ad.your-server.com:636");
-
- env.put(Context.SECURITY_AUTHENTICATION, "simple");
- env.put(Context.SECURITY_PRINCIPAL, "username");
- env.put(Context.SECURITY_CREDENTIALS, "secpassword");
-
- // BAD - Test configuration with disabled SSL endpoint check.
- {
- System.setProperty("com.sun.jndi.ldap.object.disableEndpointIdentification", "true");
- }
-
- return env;
- }
-}
diff --git a/java/ql/src/experimental/Security/CWE/CWE-297/InsecureLdapEndpoint.qhelp b/java/ql/src/experimental/Security/CWE/CWE-297/InsecureLdapEndpoint.qhelp
deleted file mode 100644
index 50e1febf8940..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-297/InsecureLdapEndpoint.qhelp
+++ /dev/null
@@ -1,40 +0,0 @@
-
-
-
-
-Java versions 8u181 or greater have enabled LDAPS endpoint identification by default. Nowadays
- infrastructure services like LDAP are commonly deployed behind load balancers therefore the LDAP
- server name can be different from the FQDN of the LDAPS endpoint. If a service certificate does not
- properly contain a matching DNS name as part of the certificate, Java will reject it by default.
-Instead of addressing the issue properly by having a compliant certificate deployed, frequently
- developers simply disable the LDAPS endpoint check.
-Failing to validate the certificate makes the SSL session susceptible to a man-in-the-middle attack.
- This query checks whether the LDAPS endpoint check is disabled in system properties.
-
-
-
-Replace any non-conforming LDAP server certificates to include a DNS name in the subjectAltName field
- of the certificate that matches the FQDN of the service.
-
-
-
-The following two examples show two ways of configuring LDAPS endpoint. In the 'BAD' case,
- endpoint check is disabled. In the 'GOOD' case, endpoint check is left enabled through the
- default Java configuration.
-
-
-
-
- Oracle Java 8 Update 181 (8u181):
- Endpoint identification enabled on LDAPS connections
-
-
- IBM:
- Fix this LDAP SSL error
-
-
-
diff --git a/java/ql/src/experimental/Security/CWE/CWE-297/InsecureLdapEndpoint.ql b/java/ql/src/experimental/Security/CWE/CWE-297/InsecureLdapEndpoint.ql
deleted file mode 100644
index ba860a1309f0..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-297/InsecureLdapEndpoint.ql
+++ /dev/null
@@ -1,111 +0,0 @@
-/**
- * @name Insecure LDAPS Endpoint Configuration
- * @description Java application configured to disable LDAPS endpoint
- * identification does not validate the SSL certificate to
- * properly ensure that it is actually associated with that host.
- * @kind problem
- * @problem.severity warning
- * @precision medium
- * @id java/insecure-ldaps-endpoint
- * @tags security
- * experimental
- * external/cwe/cwe-297
- */
-
-import java
-
-/** The method to set a system property. */
-class SetSystemPropertyMethod extends Method {
- SetSystemPropertyMethod() {
- this.hasName("setProperty") and
- this.getDeclaringType().hasQualifiedName("java.lang", "System")
- }
-}
-
-/** The class `java.util.Hashtable`. */
-class TypeHashtable extends Class {
- TypeHashtable() { this.getSourceDeclaration().hasQualifiedName("java.util", "Hashtable") }
-}
-
-/**
- * The method to set Java properties either through `setProperty` declared in the class `Properties`
- * or `put` declared in its parent class `HashTable`.
- */
-class SetPropertyMethod extends Method {
- SetPropertyMethod() {
- this.getDeclaringType().getAnAncestor() instanceof TypeHashtable and
- this.hasName(["put", "setProperty"])
- }
-}
-
-/** The `setProperties` method declared in `java.lang.System`. */
-class SetSystemPropertiesMethod extends Method {
- SetSystemPropertiesMethod() {
- this.hasName("setProperties") and
- this.getDeclaringType().hasQualifiedName("java.lang", "System")
- }
-}
-
-/**
- * Holds if `Expr` expr is evaluated to the string literal
- * `com.sun.jndi.ldap.object.disableEndpointIdentification`.
- */
-predicate isPropertyDisableLdapEndpointId(Expr expr) {
- expr.(CompileTimeConstantExpr).getStringValue() =
- "com.sun.jndi.ldap.object.disableEndpointIdentification"
- or
- exists(Field f |
- expr = f.getAnAccess() and
- f.getAnAssignedValue().(StringLiteral).getValue() =
- "com.sun.jndi.ldap.object.disableEndpointIdentification"
- )
-}
-
-/** Holds if an expression is evaluated to the boolean value true. */
-predicate isBooleanTrue(Expr expr) {
- expr.(CompileTimeConstantExpr).getStringValue() = "true" // "true"
- or
- expr.(BooleanLiteral).getBooleanValue() = true // true
- or
- exists(MethodCall ma |
- expr = ma and
- ma.getMethod() instanceof ToStringMethod and
- ma.getQualifier().(FieldAccess).getField().hasName("TRUE") and
- ma.getQualifier()
- .(FieldAccess)
- .getField()
- .getDeclaringType()
- .hasQualifiedName("java.lang", "Boolean") // Boolean.TRUE.toString()
- )
-}
-
-/** Holds if `ma` is in a test class or method. */
-predicate isTestMethod(MethodCall ma) {
- ma.getEnclosingCallable() instanceof TestMethod or
- ma.getEnclosingCallable().getDeclaringType() instanceof TestClass or
- ma.getEnclosingCallable().getDeclaringType().getPackage().getName().matches("%test%") or
- ma.getEnclosingCallable().getDeclaringType().getName().toLowerCase().matches("%test%")
-}
-
-/** Holds if `MethodCall` ma disables SSL endpoint check. */
-predicate isInsecureSslEndpoint(MethodCall ma) {
- (
- ma.getMethod() instanceof SetSystemPropertyMethod and
- isPropertyDisableLdapEndpointId(ma.getArgument(0)) and
- isBooleanTrue(ma.getArgument(1)) //com.sun.jndi.ldap.object.disableEndpointIdentification=true
- or
- ma.getMethod() instanceof SetSystemPropertiesMethod and
- exists(MethodCall ma2 |
- ma2.getMethod() instanceof SetPropertyMethod and
- isPropertyDisableLdapEndpointId(ma2.getArgument(0)) and
- isBooleanTrue(ma2.getArgument(1)) and //com.sun.jndi.ldap.object.disableEndpointIdentification=true
- ma2.getQualifier().(VarAccess).getVariable().getAnAccess() = ma.getArgument(0) // systemProps.setProperties(properties)
- )
- )
-}
-
-deprecated query predicate problems(MethodCall ma, string message) {
- isInsecureSslEndpoint(ma) and
- not isTestMethod(ma) and
- message = "LDAPS configuration allows insecure endpoint identification."
-}
diff --git a/java/ql/src/experimental/Security/CWE/CWE-297/InsecureLdapEndpoint2.java b/java/ql/src/experimental/Security/CWE/CWE-297/InsecureLdapEndpoint2.java
deleted file mode 100644
index 2a5c3c87bc75..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-297/InsecureLdapEndpoint2.java
+++ /dev/null
@@ -1,17 +0,0 @@
-public class InsecureLdapEndpoint2 {
- public Hashtable createConnectionEnv() {
- Hashtable env = new Hashtable();
- env.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory");
- env.put(Context.PROVIDER_URL, "ldaps://ad.your-server.com:636");
-
- env.put(Context.SECURITY_AUTHENTICATION, "simple");
- env.put(Context.SECURITY_PRINCIPAL, "username");
- env.put(Context.SECURITY_CREDENTIALS, "secpassword");
-
- // GOOD - No configuration to disable SSL endpoint check since it is enabled by default.
- {
- }
-
- return env;
- }
-}
diff --git a/java/ql/src/experimental/Security/CWE/CWE-299/CustomRevocationChecking.java b/java/ql/src/experimental/Security/CWE/CWE-299/CustomRevocationChecking.java
deleted file mode 100644
index 57b076f6a368..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-299/CustomRevocationChecking.java
+++ /dev/null
@@ -1,10 +0,0 @@
-public void validate(KeyStore cacerts, CertPath certPath) throws Exception {
- CertPathValidator validator = CertPathValidator.getInstance("PKIX");
- PKIXParameters params = new PKIXParameters(cacerts);
- params.setRevocationEnabled(false);
- PKIXRevocationChecker checker = (PKIXRevocationChecker) validator.getRevocationChecker();
- checker.setOcspResponder(OCSP_RESPONDER_URL);
- checker.setOcspResponderCert(OCSP_RESPONDER_CERT);
- params.addCertPathChecker(checker);
- validator.validate(certPath, params);
-}
\ No newline at end of file
diff --git a/java/ql/src/experimental/Security/CWE/CWE-299/DefaultRevocationChecking.java b/java/ql/src/experimental/Security/CWE/CWE-299/DefaultRevocationChecking.java
deleted file mode 100644
index 82bb697ce4a8..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-299/DefaultRevocationChecking.java
+++ /dev/null
@@ -1,5 +0,0 @@
-public void validate(KeyStore cacerts, CertPath chain) throws Exception {
- CertPathValidator validator = CertPathValidator.getInstance("PKIX");
- PKIXParameters params = new PKIXParameters(cacerts);
- validator.validate(chain, params);
-}
\ No newline at end of file
diff --git a/java/ql/src/experimental/Security/CWE/CWE-299/DisabledRevocationChecking.qhelp b/java/ql/src/experimental/Security/CWE/CWE-299/DisabledRevocationChecking.qhelp
deleted file mode 100644
index 9883a64bc7a5..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-299/DisabledRevocationChecking.qhelp
+++ /dev/null
@@ -1,63 +0,0 @@
-
-
-
-
-Validating a certificate chain includes multiple steps. One of them is checking whether or not
-certificates in the chain have been revoked. A certificate may be revoked due to multiple reasons.
-One of the reasons why the certificate authority (CA) may revoke a certificate is that its private key
-has been compromised. For example, the private key might have been stolen by an adversary.
-In this case, the adversary may be able to impersonate the owner of the private key.
-Therefore, trusting a revoked certificate may be dangerous.
-
-The Java Certification Path API provides a revocation checking mechanism
-that supports both CRL and OCSP.
-Revocation checking happens while building and validating certificate chains.
-If at least one of the certificates is revoked, then an exception is thrown.
-This mechanism is enabled by default. However, it may be disabled
-by passing false to the PKIXParameters.setRevocationEnabled() method.
-If an application doesn't set a custom PKIXRevocationChecker
-via PKIXParameters.addCertPathChecker()
-or PKIXParameters.setCertPathCheckers() methods,
-then revocation checking is not going to happen.
-
-
-
-
-An application should not disable the default revocation checking mechanism
-unless it provides a custom revocation checker.
-
-
-
-
-The following example turns off revocation checking for validating a certificate chain.
-That should be avoided.
-
-The next example uses the default revocation checking mechanism.
-
-The third example turns off the default revocation mechanism. However, it registers another
-revocation checker that uses OCSP to obtain revocation status of certificates.
-
-
-
-
-
- Wikipedia:
- Public key certificate
-
-
- Java SE Documentation:
- Java PKI Programmer's Guide
-
-
- Java API Specification:
- CertPathValidator
-
-
-
-
\ No newline at end of file
diff --git a/java/ql/src/experimental/Security/CWE/CWE-299/DisabledRevocationChecking.ql b/java/ql/src/experimental/Security/CWE/CWE-299/DisabledRevocationChecking.ql
deleted file mode 100644
index 6b28c0bc068e..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-299/DisabledRevocationChecking.ql
+++ /dev/null
@@ -1,25 +0,0 @@
-/**
- * @name Disabled certificate revocation checking
- * @description Using revoked certificates is dangerous.
- * Therefore, revocation status of certificates in a chain should be checked.
- * @kind path-problem
- * @problem.severity error
- * @precision high
- * @id java/disabled-certificate-revocation-checking
- * @tags security
- * experimental
- * external/cwe/cwe-299
- */
-
-import java
-deprecated import RevocationCheckingLib
-deprecated import DisabledRevocationCheckingFlow::PathGraph
-
-deprecated query predicate problems(
- DataFlow::Node sourceNode, DisabledRevocationCheckingFlow::PathNode source,
- DisabledRevocationCheckingFlow::PathNode sink, string message
-) {
- DisabledRevocationCheckingFlow::flowPath(source, sink) and
- sourceNode = source.getNode() and
- message = "This disables revocation checking."
-}
diff --git a/java/ql/src/experimental/Security/CWE/CWE-299/NoRevocationChecking.java b/java/ql/src/experimental/Security/CWE/CWE-299/NoRevocationChecking.java
deleted file mode 100644
index 24aec8da1e75..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-299/NoRevocationChecking.java
+++ /dev/null
@@ -1,6 +0,0 @@
-public void validateUnsafe(KeyStore cacerts, CertPath chain) throws Exception {
- CertPathValidator validator = CertPathValidator.getInstance("PKIX");
- PKIXParameters params = new PKIXParameters(cacerts);
- params.setRevocationEnabled(false);
- validator.validate(chain, params);
-}
\ No newline at end of file
diff --git a/java/ql/src/experimental/Security/CWE/CWE-299/RevocationCheckingLib.qll b/java/ql/src/experimental/Security/CWE/CWE-299/RevocationCheckingLib.qll
deleted file mode 100644
index c664e9e7771c..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-299/RevocationCheckingLib.qll
+++ /dev/null
@@ -1,62 +0,0 @@
-deprecated module;
-
-import java
-import semmle.code.java.dataflow.FlowSources
-import DataFlow
-
-/**
- * A taint-tracking configuration for disabling revocation checking.
- */
-module DisabledRevocationCheckingConfig implements DataFlow::ConfigSig {
- predicate isSource(DataFlow::Node source) {
- source.asExpr().(BooleanLiteral).getBooleanValue() = false
- }
-
- predicate isSink(DataFlow::Node sink) { sink instanceof SetRevocationEnabledSink }
-}
-
-module DisabledRevocationCheckingFlow = TaintTracking::Global;
-
-/**
- * A sink that disables revocation checking,
- * i.e. calling `PKIXParameters.setRevocationEnabled(false)`
- * without setting a custom revocation checker in `PKIXParameters`.
- */
-class SetRevocationEnabledSink extends DataFlow::ExprNode {
- SetRevocationEnabledSink() {
- exists(MethodCall setRevocationEnabledCall |
- setRevocationEnabledCall.getMethod() instanceof SetRevocationEnabledMethod and
- setRevocationEnabledCall.getArgument(0) = this.getExpr() and
- not exists(MethodCall ma, Method m | m = ma.getMethod() |
- (m instanceof AddCertPathCheckerMethod or m instanceof SetCertPathCheckersMethod) and
- ma.getQualifier().(VarAccess).getVariable() =
- setRevocationEnabledCall.getQualifier().(VarAccess).getVariable()
- )
- )
- }
-}
-
-class SetRevocationEnabledMethod extends Method {
- SetRevocationEnabledMethod() {
- this.getDeclaringType() instanceof PKIXParameters and
- this.hasName("setRevocationEnabled")
- }
-}
-
-class AddCertPathCheckerMethod extends Method {
- AddCertPathCheckerMethod() {
- this.getDeclaringType() instanceof PKIXParameters and
- this.hasName("addCertPathChecker")
- }
-}
-
-class SetCertPathCheckersMethod extends Method {
- SetCertPathCheckersMethod() {
- this.getDeclaringType() instanceof PKIXParameters and
- this.hasName("setCertPathCheckers")
- }
-}
-
-class PKIXParameters extends RefType {
- PKIXParameters() { this.hasQualifiedName("java.security.cert", "PKIXParameters") }
-}
diff --git a/java/ql/src/experimental/Security/CWE/CWE-327/Azure/UnsafeUsageOfClientSideEncryptionVersion.java b/java/ql/src/experimental/Security/CWE/CWE-327/Azure/UnsafeUsageOfClientSideEncryptionVersion.java
deleted file mode 100644
index 83157c142516..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-327/Azure/UnsafeUsageOfClientSideEncryptionVersion.java
+++ /dev/null
@@ -1,46 +0,0 @@
-
-// BAD: Using an outdated SDK that does not support client side encryption version V2_0
-new EncryptedBlobClientBuilder()
- .blobClient(blobClient)
- .key(resolver.buildAsyncKeyEncryptionKey(keyid).block(), keyWrapAlgorithm)
- .buildEncryptedBlobClient()
- .uploadWithResponse(new BlobParallelUploadOptions(data)
- .setMetadata(metadata)
- .setHeaders(headers)
- .setTags(tags)
- .setTier(tier)
- .setRequestConditions(requestConditions)
- .setComputeMd5(computeMd5)
- .setParallelTransferOptions(parallelTransferOptions),
- timeout, context);
-
-// BAD: Using the deprecatedd client side encryption version V1_0
-new EncryptedBlobClientBuilder(EncryptionVersion.V1)
- .blobClient(blobClient)
- .key(resolver.buildAsyncKeyEncryptionKey(keyid).block(), keyWrapAlgorithm)
- .buildEncryptedBlobClient()
- .uploadWithResponse(new BlobParallelUploadOptions(data)
- .setMetadata(metadata)
- .setHeaders(headers)
- .setTags(tags)
- .setTier(tier)
- .setRequestConditions(requestConditions)
- .setComputeMd5(computeMd5)
- .setParallelTransferOptions(parallelTransferOptions),
- timeout, context);
-
-
-// GOOD: Using client side encryption version V2_0
-new EncryptedBlobClientBuilder(EncryptionVersion.V2)
- .blobClient(blobClient)
- .key(resolver.buildAsyncKeyEncryptionKey(keyid).block(), keyWrapAlgorithm)
- .buildEncryptedBlobClient()
- .uploadWithResponse(new BlobParallelUploadOptions(data)
- .setMetadata(metadata)
- .setHeaders(headers)
- .setTags(tags)
- .setTier(tier)
- .setRequestConditions(requestConditions)
- .setComputeMd5(computeMd5)
- .setParallelTransferOptions(parallelTransferOptions),
- timeout, context);
diff --git a/java/ql/src/experimental/Security/CWE/CWE-327/Azure/UnsafeUsageOfClientSideEncryptionVersion.qhelp b/java/ql/src/experimental/Security/CWE/CWE-327/Azure/UnsafeUsageOfClientSideEncryptionVersion.qhelp
deleted file mode 100644
index b6884aed9141..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-327/Azure/UnsafeUsageOfClientSideEncryptionVersion.qhelp
+++ /dev/null
@@ -1,29 +0,0 @@
-
-
-
-
-
- Azure Storage .NET, Java, and Python SDKs support encryption on the client with a customer-managed key that is maintained in Azure Key Vault or another key store.
- The Azure Storage SDK version 12.18.0 or later supports version V2 for client-side encryption. All previous versions of Azure Storage SDK only support client-side encryption V1 which is unsafe.
-
-
-
-
- Consider switching to V2 client-side encryption.
-
-
-
-
-
-
-
- Azure Storage Client Encryption Blog.
-
-
- CVE-2022-30187
-
-
-
-
diff --git a/java/ql/src/experimental/Security/CWE/CWE-327/Azure/UnsafeUsageOfClientSideEncryptionVersion.ql b/java/ql/src/experimental/Security/CWE/CWE-327/Azure/UnsafeUsageOfClientSideEncryptionVersion.ql
deleted file mode 100644
index a5d17250491b..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-327/Azure/UnsafeUsageOfClientSideEncryptionVersion.ql
+++ /dev/null
@@ -1,91 +0,0 @@
-/**
- * @name Unsafe usage of v1 version of Azure Storage client-side encryption (CVE-2022-30187).
- * @description Unsafe usage of v1 version of Azure Storage client-side encryption, please refer to http://aka.ms/azstorageclientencryptionblog
- * @kind problem
- * @tags security
- * cryptography
- * external/cwe/cwe-327
- * @id java/azure-storage/unsafe-client-side-encryption-in-use
- * @problem.severity error
- * @precision high
- */
-
-import java
-import semmle.code.java.dataflow.DataFlow
-
-/**
- * Holds if `call` is an object creation for a class `EncryptedBlobClientBuilder`
- * that takes no arguments, which means that it is using V1 encryption.
- */
-predicate isCreatingOutdatedAzureClientSideEncryptionObject(Call call, Class c) {
- exists(string package, string type, Constructor constructor |
- c.hasQualifiedName(package, type) and
- c.getAConstructor() = constructor and
- call.getCallee() = constructor and
- (
- type = "EncryptedBlobClientBuilder" and
- package = "com.azure.storage.blob.specialized.cryptography" and
- constructor.hasNoParameters()
- or
- type = "BlobEncryptionPolicy" and package = "com.microsoft.azure.storage.blob"
- )
- )
-}
-
-/**
- * Holds if `call` is an object creation for a class `EncryptedBlobClientBuilder`
- * that takes `versionArg` as the argument specifying the encryption version.
- */
-predicate isCreatingAzureClientSideEncryptionObjectNewVersion(Call call, Class c, Expr versionArg) {
- exists(string package, string type, Constructor constructor |
- c.hasQualifiedName(package, type) and
- c.getAConstructor() = constructor and
- call.getCallee() = constructor and
- type = "EncryptedBlobClientBuilder" and
- package = "com.azure.storage.blob.specialized.cryptography" and
- versionArg = call.getArgument(0)
- )
-}
-
-/**
- * A dataflow config that tracks `EncryptedBlobClientBuilder.version` argument initialization.
- */
-private module EncryptedBlobClientBuilderSafeEncryptionVersionConfig implements DataFlow::ConfigSig {
- predicate isSource(DataFlow::Node source) {
- exists(FieldRead fr, Field f | fr = source.asExpr() |
- f.getAnAccess() = fr and
- f.hasQualifiedName("com.azure.storage.blob.specialized.cryptography", "EncryptionVersion",
- "V2")
- )
- }
-
- predicate isSink(DataFlow::Node sink) {
- isCreatingAzureClientSideEncryptionObjectNewVersion(_, _, sink.asExpr())
- }
-}
-
-private module EncryptedBlobClientBuilderSafeEncryptionVersionFlow =
- DataFlow::Global;
-
-/**
- * Holds if `call` is an object creation for a class `EncryptedBlobClientBuilder`
- * that takes `versionArg` as the argument specifying the encryption version, and that version is safe.
- */
-predicate isCreatingSafeAzureClientSideEncryptionObject(Call call, Class c, Expr versionArg) {
- isCreatingAzureClientSideEncryptionObjectNewVersion(call, c, versionArg) and
- exists(DataFlow::Node sink | sink.asExpr() = versionArg |
- EncryptedBlobClientBuilderSafeEncryptionVersionFlow::flowTo(sink)
- )
-}
-
-deprecated query predicate problems(Expr e, string message) {
- exists(Class c |
- exists(Expr argVersion |
- isCreatingAzureClientSideEncryptionObjectNewVersion(e, c, argVersion) and
- not isCreatingSafeAzureClientSideEncryptionObject(e, c, argVersion)
- )
- or
- isCreatingOutdatedAzureClientSideEncryptionObject(e, c)
- ) and
- message = "Unsafe usage of v1 version of Azure Storage client-side encryption."
-}
diff --git a/java/ql/src/experimental/Security/CWE/CWE-327/SaferTLSVersion.java b/java/ql/src/experimental/Security/CWE/CWE-327/SaferTLSVersion.java
deleted file mode 100644
index 7204908662e9..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-327/SaferTLSVersion.java
+++ /dev/null
@@ -1,6 +0,0 @@
-public SSLSocket connect(String host, int port)
- throws NoSuchAlgorithmException, IOException {
-
- SSLContext context = SSLContext.getInstance("TLSv1.3");
- return (SSLSocket) context.getSocketFactory().createSocket(host, port);
-}
\ No newline at end of file
diff --git a/java/ql/src/experimental/Security/CWE/CWE-327/SslLib.qll b/java/ql/src/experimental/Security/CWE/CWE-327/SslLib.qll
deleted file mode 100644
index 3b3ad279d0c4..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-327/SslLib.qll
+++ /dev/null
@@ -1,99 +0,0 @@
-deprecated module;
-
-import java
-import semmle.code.java.security.Encryption
-import semmle.code.java.dataflow.TaintTracking
-
-/**
- * A taint-tracking configuration for unsafe SSL and TLS versions.
- */
-module UnsafeTlsVersionConfig implements DataFlow::ConfigSig {
- predicate isSource(DataFlow::Node source) { source.asExpr() instanceof UnsafeTlsVersion }
-
- predicate isSink(DataFlow::Node sink) {
- sink instanceof SslContextGetInstanceSink or
- sink instanceof CreateSslParametersSink or
- sink instanceof SslParametersSetProtocolsSink or
- sink instanceof SetEnabledProtocolsSink
- }
-}
-
-module UnsafeTlsVersionFlow = TaintTracking::Global;
-
-/**
- * A sink that sets protocol versions in `SSLContext`,
- * i.e `SSLContext.getInstance(protocol)`.
- */
-class SslContextGetInstanceSink extends DataFlow::ExprNode {
- SslContextGetInstanceSink() {
- exists(StaticMethodCall ma, Method m | m = ma.getMethod() |
- m.getDeclaringType() instanceof SslContext and
- m.hasName("getInstance") and
- ma.getArgument(0) = this.asExpr()
- )
- }
-}
-
-/**
- * A sink that creates `SSLParameters` with specified protocols,
- * i.e. `new SSLParameters(ciphersuites, protocols)`.
- */
-class CreateSslParametersSink extends DataFlow::ExprNode {
- CreateSslParametersSink() {
- exists(ConstructorCall cc | cc.getConstructedType() instanceof SslParameters |
- cc.getArgument(1) = this.asExpr()
- )
- }
-}
-
-/**
- * A sink that sets protocol versions for `SSLParameters`,
- * i.e. `parameters.setProtocols(versions)`.
- */
-class SslParametersSetProtocolsSink extends DataFlow::ExprNode {
- SslParametersSetProtocolsSink() {
- exists(MethodCall ma, Method m | m = ma.getMethod() |
- m.getDeclaringType() instanceof SslParameters and
- m.hasName("setProtocols") and
- ma.getArgument(0) = this.asExpr()
- )
- }
-}
-
-/**
- * A sink that sets protocol versions for `SSLSocket`, `SSLServerSocket`, and `SSLEngine`,
- * i.e. `socket.setEnabledProtocols(versions)` or `engine.setEnabledProtocols(versions)`.
- */
-class SetEnabledProtocolsSink extends DataFlow::ExprNode {
- SetEnabledProtocolsSink() {
- exists(MethodCall ma, Method m, RefType type |
- m = ma.getMethod() and type = m.getDeclaringType()
- |
- (
- type instanceof SslSocket or
- type instanceof SslServerSocket or
- type instanceof SslEngine
- ) and
- m.hasName("setEnabledProtocols") and
- ma.getArgument(0) = this.asExpr()
- )
- }
-}
-
-/**
- * Insecure SSL and TLS versions supported by JSSE.
- */
-class UnsafeTlsVersion extends StringLiteral {
- UnsafeTlsVersion() {
- this.getValue() = "SSL" or
- this.getValue() = "SSLv2" or
- this.getValue() = "SSLv3" or
- this.getValue() = "TLS" or
- this.getValue() = "TLSv1" or
- this.getValue() = "TLSv1.1"
- }
-}
-
-class SslServerSocket extends RefType {
- SslServerSocket() { this.hasQualifiedName("javax.net.ssl", "SSLServerSocket") }
-}
diff --git a/java/ql/src/experimental/Security/CWE/CWE-327/UnsafeTLSVersion.java b/java/ql/src/experimental/Security/CWE/CWE-327/UnsafeTLSVersion.java
deleted file mode 100644
index c2beff544c4e..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-327/UnsafeTLSVersion.java
+++ /dev/null
@@ -1,6 +0,0 @@
-public SSLSocket connect(String host, int port)
- throws NoSuchAlgorithmException, IOException {
-
- SSLContext context = SSLContext.getInstance("SSLv3");
- return (SSLSocket) context.getSocketFactory().createSocket(host, port);
-}
\ No newline at end of file
diff --git a/java/ql/src/experimental/Security/CWE/CWE-327/UnsafeTlsVersion.qhelp b/java/ql/src/experimental/Security/CWE/CWE-327/UnsafeTlsVersion.qhelp
deleted file mode 100644
index 6e9225f3b797..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-327/UnsafeTlsVersion.qhelp
+++ /dev/null
@@ -1,60 +0,0 @@
-
-
-
-
-Transport Layer Security (TLS) provides a number of security features such as
-confidentiality, integrity, replay prevention and authentication.
-There are several versions of TLS protocols. The latest is TLS 1.3.
-Unfortunately, older versions were found to be vulnerable to a number of attacks.
-
-
-
-
-An application should use TLS 1.3. Currently, TLS 1.2 is also considered acceptable.
-
-
-
-
-The following example shows how a socket with an unsafe TLS version may be created:
-
-The next example creates a socket with the latest TLS version:
-
-
-
-
-
- Wikipedia:
- Transport Layer Security
-
-
-
- OWASP:
- Transport Layer Protection Cheat Sheet
-
-
-
- Java SE Documentation:
- Java Secure Socket Extension (JSSE) Reference Guide
-
-
-
- Java API Specification:
- SSLContext
-
-
-
- Java API Specification:
- SSLParameters
-
-
-
- Java API Specification:
- SSLSocket
-
-
-
-
\ No newline at end of file
diff --git a/java/ql/src/experimental/Security/CWE/CWE-327/UnsafeTlsVersion.ql b/java/ql/src/experimental/Security/CWE/CWE-327/UnsafeTlsVersion.ql
deleted file mode 100644
index 25131c5042a7..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-327/UnsafeTlsVersion.ql
+++ /dev/null
@@ -1,27 +0,0 @@
-/**
- * @name Unsafe TLS version
- * @description SSL and older TLS versions are known to be vulnerable.
- * TLS 1.3 or at least TLS 1.2 should be used.
- * @kind path-problem
- * @problem.severity error
- * @precision high
- * @id java/unsafe-tls-version
- * @tags security
- * experimental
- * external/cwe/cwe-327
- */
-
-import java
-deprecated import SslLib
-deprecated import UnsafeTlsVersionFlow::PathGraph
-
-deprecated query predicate problems(
- DataFlow::Node sinkNode, UnsafeTlsVersionFlow::PathNode source,
- UnsafeTlsVersionFlow::PathNode sink, string message1, DataFlow::Node sourceNode, string message2
-) {
- UnsafeTlsVersionFlow::flowPath(source, sink) and
- sinkNode = sink.getNode() and
- message1 = "$@ is unsafe." and
- sourceNode = source.getNode() and
- message2 = source.getNode().asExpr().(StringLiteral).getValue()
-}
diff --git a/java/ql/src/experimental/Security/CWE/CWE-346/UnvalidatedCors.java b/java/ql/src/experimental/Security/CWE/CWE-346/UnvalidatedCors.java
deleted file mode 100644
index fd562d44f303..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-346/UnvalidatedCors.java
+++ /dev/null
@@ -1,45 +0,0 @@
-import java.io.IOException;
-
-import javax.servlet.Filter;
-import javax.servlet.FilterChain;
-import javax.servlet.FilterConfig;
-import javax.servlet.ServletException;
-import javax.servlet.ServletRequest;
-import javax.servlet.ServletResponse;
-import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletResponse;
-
-import org.apache.commons.lang3.StringUtils;
-
-public class CorsFilter implements Filter {
- public void init(FilterConfig filterConfig) throws ServletException {}
-
- public void doFilter(ServletRequest req, ServletResponse res,
- FilterChain chain) throws IOException, ServletException {
- HttpServletRequest request = (HttpServletRequest) req;
- HttpServletResponse response = (HttpServletResponse) res;
- String url = request.getHeader("Origin");
-
- if (!StringUtils.isEmpty(url)) {
- String val = response.getHeader("Access-Control-Allow-Origin");
-
- if (StringUtils.isEmpty(val)) {
- response.addHeader("Access-Control-Allow-Origin", url); // BAD -> User controlled CORS header being set here.
- response.addHeader("Access-Control-Allow-Credentials", "true");
- }
- }
-
- if (!StringUtils.isEmpty(url)) {
- List checkorigins = Arrays.asList("www.example.com", "www.sub.example.com");
-
- if (checkorigins.contains(url)) { // GOOD -> Origin is validated here.
- response.addHeader("Access-Control-Allow-Origin", url);
- response.addHeader("Access-Control-Allow-Credentials", "true");
- }
- }
-
- chain.doFilter(req, res);
- }
-
- public void destroy() {}
-}
diff --git a/java/ql/src/experimental/Security/CWE/CWE-346/UnvalidatedCors.qhelp b/java/ql/src/experimental/Security/CWE/CWE-346/UnvalidatedCors.qhelp
deleted file mode 100644
index da98e896a60c..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-346/UnvalidatedCors.qhelp
+++ /dev/null
@@ -1,76 +0,0 @@
-
-
-
-
-
-
- A server can send the
- Access-Control-Allow-Credentials CORS header to control
- when a browser may send user credentials in Cross-Origin HTTP
- requests.
-
-
-
-
- When the Access-Control-Allow-Credentials header
- is true, the Access-Control-Allow-Origin
- header must have a value different from * in order
- for browsers to accept the header. Therefore, to allow multiple origins
- for cross-origin requests with credentials, the server must
- dynamically compute the value of the
- Access-Control-Allow-Origin header. Computing this
- header value from information in the request to the server can
- therefore potentially allow an attacker to control the origins that
- the browser sends credentials to.
-
-
-
-
-
-
-
-
-
-
- When the Access-Control-Allow-Credentials header
- value is true, a dynamic computation of the
- Access-Control-Allow-Origin header must involve
- sanitization if it relies on user-controlled input.
-
-
-
-
-
- Since the null origin is easy to obtain for an
- attacker, it is never safe to use null as the value of
- the Access-Control-Allow-Origin header when the
- Access-Control-Allow-Credentials header value is
- true.A null origin can be set by an attacker using a sandboxed iframe.
- A more detailed explanation is available in the portswigger blogpost referenced below.
-
-
-
-
-
-
-
- In the example below, the server allows the browser to send
- user credentials in a cross-origin request. The request header
- origins controls the allowed origins for such a
- Cross-Origin request.
-
-
-
-
-
-
- Mozilla Developer Network: CORS, Access-Control-Allow-Origin.
- Mozilla Developer Network: CORS, Access-Control-Allow-Credentials.
- PortSwigger: Exploiting CORS Misconfigurations for Bitcoins and Bounties
- W3C: CORS for developers, Advice for Resource Owners
-
-
diff --git a/java/ql/src/experimental/Security/CWE/CWE-346/UnvalidatedCors.ql b/java/ql/src/experimental/Security/CWE/CWE-346/UnvalidatedCors.ql
deleted file mode 100644
index ef95db6f6c53..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-346/UnvalidatedCors.ql
+++ /dev/null
@@ -1,94 +0,0 @@
-/**
- * @name CORS is derived from untrusted input
- * @description CORS header is derived from untrusted input, allowing a remote user to control which origins are trusted.
- * @kind path-problem
- * @problem.severity error
- * @precision high
- * @id java/unvalidated-cors-origin-set
- * @tags security
- * experimental
- * external/cwe/cwe-346
- */
-
-import java
-import semmle.code.java.dataflow.FlowSources
-import semmle.code.java.frameworks.Servlets
-import semmle.code.java.dataflow.TaintTracking
-import CorsOriginFlow::PathGraph
-
-/**
- * Holds if `header` sets `Access-Control-Allow-Credentials` to `true`. This ensures fair chances of exploitability.
- */
-private predicate setsAllowCredentials(MethodCall header) {
- (
- header.getMethod() instanceof ResponseSetHeaderMethod or
- header.getMethod() instanceof ResponseAddHeaderMethod
- ) and
- header.getArgument(0).(CompileTimeConstantExpr).getStringValue().toLowerCase() =
- "access-control-allow-credentials" and
- header.getArgument(1).(CompileTimeConstantExpr).getStringValue().toLowerCase() = "true"
-}
-
-private class CorsProbableCheckAccess extends MethodCall {
- CorsProbableCheckAccess() {
- this.getMethod().hasName("contains") and
- this.getMethod().getDeclaringType().getASourceSupertype*() instanceof CollectionType
- or
- this.getMethod().hasName("containsKey") and
- this.getMethod().getDeclaringType().getASourceSupertype*() instanceof MapType
- or
- this.getMethod().hasName("equals") and
- this.getQualifier().getType() instanceof TypeString
- }
-}
-
-private Expr getAccessControlAllowOriginHeaderName() {
- result.(CompileTimeConstantExpr).getStringValue().toLowerCase() = "access-control-allow-origin"
-}
-
-/**
- * A taint-tracking configuration for flow from a source node to CorsProbableCheckAccess methods.
- */
-module CorsSourceReachesCheckConfig implements DataFlow::ConfigSig {
- predicate isSource(DataFlow::Node source) { CorsOriginFlow::flow(source, _) }
-
- predicate isSink(DataFlow::Node sink) {
- sink.asExpr() = any(CorsProbableCheckAccess check).getAnArgument()
- }
-}
-
-/**
- * Taint-tracking flow from a source node to CorsProbableCheckAccess methods.
- */
-module CorsSourceReachesCheckFlow = TaintTracking::Global;
-
-private module CorsOriginConfig implements DataFlow::ConfigSig {
- predicate isSource(DataFlow::Node source) { source instanceof ActiveThreatModelSource }
-
- predicate isSink(DataFlow::Node sink) {
- exists(MethodCall corsHeader, MethodCall allowCredentialsHeader |
- (
- corsHeader.getMethod() instanceof ResponseSetHeaderMethod or
- corsHeader.getMethod() instanceof ResponseAddHeaderMethod
- ) and
- getAccessControlAllowOriginHeaderName() = corsHeader.getArgument(0) and
- setsAllowCredentials(allowCredentialsHeader) and
- corsHeader.getEnclosingCallable() = allowCredentialsHeader.getEnclosingCallable() and
- sink.asExpr() = corsHeader.getArgument(1)
- )
- }
-}
-
-private module CorsOriginFlow = TaintTracking::Global;
-
-deprecated query predicate problems(
- DataFlow::Node sinkNode, CorsOriginFlow::PathNode source, CorsOriginFlow::PathNode sink,
- string message1, DataFlow::Node sourceNode, string message2
-) {
- CorsOriginFlow::flowPath(source, sink) and
- not CorsSourceReachesCheckFlow::flow(sourceNode, _) and
- sinkNode = sink.getNode() and
- message1 = "CORS header is being set using user controlled value $@." and
- sourceNode = source.getNode() and
- message2 = "user-provided value"
-}
diff --git a/java/ql/src/experimental/Security/CWE/CWE-347/Auth0NoVerifier.qhelp b/java/ql/src/experimental/Security/CWE/CWE-347/Auth0NoVerifier.qhelp
deleted file mode 100644
index b2258c457fe9..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-347/Auth0NoVerifier.qhelp
+++ /dev/null
@@ -1,31 +0,0 @@
-
-
-
-
- A JSON Web Token (JWT) is used for authenticating and managing users in an application. It must be verified in order to ensure the JWT is genuine.
-
-
-
-
-
-
- Don't use information from a JWT without verifying that JWT.
-
-
-
-
-
-
- The following example illustrates secure and insecure use of the Auth0 `java-jwt` library.
-
-
-
-
-
- The incorrect use of JWT in ShenyuAdminBootstrap allows an attacker to bypass authentication.
-
-
-
-
diff --git a/java/ql/src/experimental/Security/CWE/CWE-347/Auth0NoVerifier.ql b/java/ql/src/experimental/Security/CWE/CWE-347/Auth0NoVerifier.ql
deleted file mode 100644
index 778939887f0e..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-347/Auth0NoVerifier.ql
+++ /dev/null
@@ -1,65 +0,0 @@
-/**
- * @name Missing JWT signature check
- * @description Failing to check the Json Web Token (JWT) signature may allow an attacker to forge their own tokens.
- * @kind path-problem
- * @problem.severity error
- * @security-severity 7.8
- * @precision high
- * @id java/missing-jwt-signature-check-auth0
- * @tags security
- * external/cwe/cwe-347
- */
-
-import java
-import semmle.code.java.dataflow.FlowSources
-deprecated import JwtAuth0 as JwtAuth0
-
-deprecated module JwtDecodeConfig implements DataFlow::ConfigSig {
- predicate isSource(DataFlow::Node source) {
- source instanceof RemoteFlowSource and
- not FlowToJwtVerify::flow(source, _)
- }
-
- predicate isSink(DataFlow::Node sink) { sink.asExpr() = any(JwtAuth0::GetPayload a) }
-
- predicate isAdditionalFlowStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
- // Decode Should be one of the middle nodes
- exists(JwtAuth0::Decode a |
- nodeFrom.asExpr() = a.getArgument(0) and
- nodeTo.asExpr() = a
- )
- or
- exists(JwtAuth0::Verify a |
- nodeFrom.asExpr() = a.getArgument(0) and
- nodeTo.asExpr() = a
- )
- or
- exists(JwtAuth0::GetPayload a |
- nodeFrom.asExpr() = a.getQualifier() and
- nodeTo.asExpr() = a
- )
- }
-}
-
-deprecated module FlowToJwtVerifyConfig implements DataFlow::ConfigSig {
- predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
-
- predicate isSink(DataFlow::Node sink) { sink.asExpr() = any(JwtAuth0::Verify a).getArgument(0) }
-}
-
-deprecated module JwtDecode = TaintTracking::Global;
-
-deprecated module FlowToJwtVerify = TaintTracking::Global;
-
-deprecated import JwtDecode::PathGraph
-
-deprecated query predicate problems(
- DataFlow::Node sinkNode, JwtDecode::PathNode source, JwtDecode::PathNode sink, string message1,
- DataFlow::Node sourceNode, string message2
-) {
- JwtDecode::flowPath(source, sink) and
- sinkNode = sink.getNode() and
- message1 = "This parses a $@, but the signature is not verified." and
- sourceNode = source.getNode() and
- message2 = "JWT"
-}
diff --git a/java/ql/src/experimental/Security/CWE/CWE-347/Example.java b/java/ql/src/experimental/Security/CWE/CWE-347/Example.java
deleted file mode 100644
index 2777d4f2cb8e..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-347/Example.java
+++ /dev/null
@@ -1,80 +0,0 @@
-package com.example.JwtTest;
-
-import java.io.*;
-import java.security.NoSuchAlgorithmException;
-import java.util.Objects;
-import java.util.Optional;
-import javax.crypto.KeyGenerator;
-import javax.servlet.http.*;
-import javax.servlet.annotation.*;
-import com.auth0.jwt.JWT;
-import com.auth0.jwt.JWTVerifier;
-import com.auth0.jwt.algorithms.Algorithm;
-import com.auth0.jwt.exceptions.JWTCreationException;
-import com.auth0.jwt.exceptions.JWTVerificationException;
-import com.auth0.jwt.interfaces.DecodedJWT;
-
-@WebServlet(name = "JwtTest1", value = "/Auth")
-public class auth0 extends HttpServlet {
-
- public void doPost(HttpServletRequest request, HttpServletResponse response) throws IOException {
- response.setContentType("text/html");
- PrintWriter out = response.getWriter();
-
- // OK: first decode without signature verification
- // and then verify with signature verification
- String JwtToken1 = request.getParameter("JWT1");
- String userName = decodeToken(JwtToken1);
- verifyToken(JwtToken1, "A Securely generated Key");
- if (Objects.equals(userName, "Admin")) {
- out.println("");
- out.println("" + "heyyy Admin" + "
");
- out.println("");
- }
-
- out.println("");
- out.println("" + "heyyy Nobody" + "
");
- out.println("");
- }
-
- public void doGet(HttpServletRequest request, HttpServletResponse response) throws IOException {
- response.setContentType("text/html");
- PrintWriter out = response.getWriter();
-
- // NOT OK: only decode, no verification
- String JwtToken2 = request.getParameter("JWT2");
- String userName = decodeToken(JwtToken2);
- if (Objects.equals(userName, "Admin")) {
- out.println("");
- out.println("" + "heyyy Admin" + "
");
- out.println("");
- }
-
- // OK: no clue of the use of unsafe decoded JWT return value
- JwtToken2 = request.getParameter("JWT2");
- JWT.decode(JwtToken2);
-
-
- out.println("");
- out.println("" + "heyyy Nobody" + "
");
- out.println("");
- }
-
- public static boolean verifyToken(final String token, final String key) {
- try {
- JWTVerifier verifier = JWT.require(Algorithm.HMAC256(key)).build();
- verifier.verify(token);
- return true;
- } catch (JWTVerificationException e) {
- System.out.printf("jwt decode fail, token: %s", e);
- }
- return false;
- }
-
-
- public static String decodeToken(final String token) {
- DecodedJWT jwt = JWT.decode(token);
- return Optional.of(jwt).map(item -> item.getClaim("userName").asString()).orElse("");
- }
-
-}
diff --git a/java/ql/src/experimental/Security/CWE/CWE-347/JwtAuth0.qll b/java/ql/src/experimental/Security/CWE/CWE-347/JwtAuth0.qll
deleted file mode 100644
index 2f1dde4d7651..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-347/JwtAuth0.qll
+++ /dev/null
@@ -1,45 +0,0 @@
-deprecated module;
-
-import java
-
-class PayloadType extends RefType {
- PayloadType() { this.hasQualifiedName("com.auth0.jwt.interfaces", "Payload") }
-}
-
-class JwtType extends RefType {
- JwtType() { this.hasQualifiedName("com.auth0.jwt", "JWT") }
-}
-
-class JwtVerifierType extends RefType {
- JwtVerifierType() { this.hasQualifiedName("com.auth0.jwt", "JWTVerifier") }
-}
-
-/**
- * A Method that returns a Decoded Claim of JWT
- */
-class GetPayload extends MethodCall {
- GetPayload() {
- this.getCallee().getDeclaringType() instanceof PayloadType and
- this.getCallee().hasName(["getClaim", "getIssuedAt"])
- }
-}
-
-/**
- * A Method that Decode JWT without signature verification
- */
-class Decode extends MethodCall {
- Decode() {
- this.getCallee().getDeclaringType() instanceof JwtType and
- this.getCallee().hasName("decode")
- }
-}
-
-/**
- * A Method that Decode JWT with signature verification
- */
-class Verify extends MethodCall {
- Verify() {
- this.getCallee().getDeclaringType() instanceof JwtVerifierType and
- this.getCallee().hasName("verify")
- }
-}
diff --git a/java/ql/src/experimental/Security/CWE/CWE-348/ClientSuppliedIpUsedInSecurityCheck.java b/java/ql/src/experimental/Security/CWE/CWE-348/ClientSuppliedIpUsedInSecurityCheck.java
deleted file mode 100644
index 93a860981d1d..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-348/ClientSuppliedIpUsedInSecurityCheck.java
+++ /dev/null
@@ -1,49 +0,0 @@
-import javax.servlet.http.HttpServletRequest;
-import org.apache.commons.lang3.StringUtils;
-import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.stereotype.Controller;
-import org.springframework.web.bind.annotation.GetMapping;
-import org.springframework.web.bind.annotation.ResponseBody;
-
-@Controller
-public class ClientSuppliedIpUsedInSecurityCheck {
-
- @Autowired
- private HttpServletRequest request;
-
- @GetMapping(value = "bad1")
- public void bad1(HttpServletRequest request) {
- String ip = getClientIP();
- if (!StringUtils.startsWith(ip, "192.168.")) {
- new Exception("ip illegal");
- }
- }
-
- @GetMapping(value = "bad2")
- public void bad2(HttpServletRequest request) {
- String ip = getClientIP();
- if (!"127.0.0.1".equals(ip)) {
- new Exception("ip illegal");
- }
- }
-
- @GetMapping(value = "good1")
- @ResponseBody
- public String good1(HttpServletRequest request) {
- String ip = request.getHeader("X-FORWARDED-FOR");
- // Good: if this application runs behind a reverse proxy it may append the real remote IP to the end of any client-supplied X-Forwarded-For header.
- ip = ip.split(",")[ip.split(",").length - 1];
- if (!StringUtils.startsWith(ip, "192.168.")) {
- new Exception("ip illegal");
- }
- return ip;
- }
-
- protected String getClientIP() {
- String xfHeader = request.getHeader("X-Forwarded-For");
- if (xfHeader == null) {
- return request.getRemoteAddr();
- }
- return xfHeader.split(",")[0];
- }
-}
diff --git a/java/ql/src/experimental/Security/CWE/CWE-348/ClientSuppliedIpUsedInSecurityCheck.qhelp b/java/ql/src/experimental/Security/CWE/CWE-348/ClientSuppliedIpUsedInSecurityCheck.qhelp
deleted file mode 100644
index fd62ab2968b2..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-348/ClientSuppliedIpUsedInSecurityCheck.qhelp
+++ /dev/null
@@ -1,35 +0,0 @@
-
-
-
-An original client IP address is retrieved from an http header (X-Forwarded-For or X-Real-IP or Proxy-Client-IP
-etc.), which is used to ensure security. Attackers can forge the value of these identifiers to
-bypass a ban-list, for example.
-
-
-
-
-Do not trust the values of HTTP headers allegedly identifying the originating IP. If you are aware your application will run behind some reverse proxies then the last entry of a X-Forwarded-For header value may be more trustworthy than the rest of it because some reverse proxies append the IP address they observed to the end of any remote-supplied header.
-
-
-
-
-The following examples show the bad case and the good case respectively.
-In bad1 method and bad2 method, the client ip the X-Forwarded-For is split into comma-separated values, but the less-trustworthy first one is used. Both of these examples could be deceived by providing a forged HTTP header. The method
-good1 similarly splits an X-Forwarded-For value, but uses the last, more-trustworthy entry.
-
-
-
-
-Dennis Schneider:
-Prevent IP address spoofing with X-Forwarded-For header when using AWS ELB and Clojure Ring
-
-
-Security Rule Zero: A Warning about X-Forwarded-For
-
-
-
-
diff --git a/java/ql/src/experimental/Security/CWE/CWE-348/ClientSuppliedIpUsedInSecurityCheck.ql b/java/ql/src/experimental/Security/CWE/CWE-348/ClientSuppliedIpUsedInSecurityCheck.ql
deleted file mode 100644
index 05cfd814fc53..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-348/ClientSuppliedIpUsedInSecurityCheck.ql
+++ /dev/null
@@ -1,59 +0,0 @@
-/**
- * @name IP address spoofing
- * @description A remote endpoint identifier is read from an HTTP header. Attackers can modify the value
- * of the identifier to forge the client ip.
- * @kind path-problem
- * @problem.severity error
- * @precision high
- * @id java/ip-address-spoofing
- * @tags security
- * experimental
- * external/cwe/cwe-348
- */
-
-import java
-import semmle.code.java.dataflow.TaintTracking
-import semmle.code.java.dataflow.FlowSources
-import semmle.code.java.security.Sanitizers
-deprecated import ClientSuppliedIpUsedInSecurityCheckLib
-deprecated import ClientSuppliedIpUsedInSecurityCheckFlow::PathGraph
-
-/**
- * Taint-tracking configuration tracing flow from obtaining a client ip from an HTTP header to a sensitive use.
- */
-deprecated module ClientSuppliedIpUsedInSecurityCheckConfig implements DataFlow::ConfigSig {
- predicate isSource(DataFlow::Node source) {
- source instanceof ClientSuppliedIpUsedInSecurityCheck
- }
-
- predicate isSink(DataFlow::Node sink) { sink instanceof ClientSuppliedIpUsedInSecurityCheckSink }
-
- /**
- * Splitting a header value by `,` and taking an entry other than the first is sanitizing, because
- * later entries may originate from more-trustworthy intermediate proxies, not the original client.
- */
- predicate isBarrier(DataFlow::Node node) {
- exists(ArrayAccess aa, MethodCall ma | aa.getArray() = ma |
- ma.getQualifier() = node.asExpr() and
- ma.getMethod() instanceof SplitMethod and
- not aa.getIndexExpr().(CompileTimeConstantExpr).getIntValue() = 0
- )
- or
- node instanceof SimpleTypeSanitizer
- }
-}
-
-deprecated module ClientSuppliedIpUsedInSecurityCheckFlow =
- TaintTracking::Global;
-
-deprecated query predicate problems(
- DataFlow::Node sinkNode, ClientSuppliedIpUsedInSecurityCheckFlow::PathNode source,
- ClientSuppliedIpUsedInSecurityCheckFlow::PathNode sink, string message1,
- DataFlow::Node sourceNode, string message2
-) {
- ClientSuppliedIpUsedInSecurityCheckFlow::flowPath(source, sink) and
- sinkNode = sink.getNode() and
- message1 = "IP address spoofing might include code from $@." and
- sourceNode = source.getNode() and
- message2 = "this user input"
-}
diff --git a/java/ql/src/experimental/Security/CWE/CWE-348/ClientSuppliedIpUsedInSecurityCheckLib.qll b/java/ql/src/experimental/Security/CWE/CWE-348/ClientSuppliedIpUsedInSecurityCheckLib.qll
deleted file mode 100644
index 42c5f989168d..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-348/ClientSuppliedIpUsedInSecurityCheckLib.qll
+++ /dev/null
@@ -1,100 +0,0 @@
-deprecated module;
-
-import java
-import DataFlow
-import semmle.code.java.frameworks.Networking
-import semmle.code.java.security.QueryInjection
-
-/**
- * A data flow source of the client ip obtained according to the remote endpoint identifier specified
- * (`X-Forwarded-For`, `X-Real-IP`, `Proxy-Client-IP`, etc.) in the header.
- *
- * For example: `ServletRequest.getHeader("X-Forwarded-For")`.
- */
-class ClientSuppliedIpUsedInSecurityCheck extends DataFlow::Node {
- ClientSuppliedIpUsedInSecurityCheck() {
- exists(MethodCall ma |
- ma.getMethod().hasName("getHeader") and
- ma.getArgument(0).(CompileTimeConstantExpr).getStringValue().toLowerCase() in [
- "x-forwarded-for", "x-real-ip", "proxy-client-ip", "wl-proxy-client-ip",
- "http_x_forwarded_for", "http_x_forwarded", "http_x_cluster_client_ip", "http_client_ip",
- "http_forwarded_for", "http_forwarded", "http_via", "remote_addr"
- ] and
- ma = this.asExpr()
- )
- }
-}
-
-/** A data flow sink for ip address forgery vulnerabilities. */
-abstract class ClientSuppliedIpUsedInSecurityCheckSink extends DataFlow::Node { }
-
-/**
- * A data flow sink for remote client ip comparison.
- *
- * For example: `if (!StringUtils.startsWith(ipAddr, "192.168.")){...` determine whether the client ip starts
- * with `192.168.`, and the program can be deceived by forging the ip address.
- */
-private class CompareSink extends ClientSuppliedIpUsedInSecurityCheckSink {
- CompareSink() {
- exists(MethodCall ma |
- ma.getMethod().getName() in ["equals", "equalsIgnoreCase"] and
- ma.getMethod().getDeclaringType() instanceof TypeString and
- ma.getMethod().getNumberOfParameters() = 1 and
- (
- ma.getArgument(0) = this.asExpr() and
- ma.getQualifier().(CompileTimeConstantExpr).getStringValue() instanceof PrivateHostName and
- not ma.getQualifier().(CompileTimeConstantExpr).getStringValue() = "0:0:0:0:0:0:0:1"
- or
- ma.getQualifier() = this.asExpr() and
- ma.getArgument(0).(CompileTimeConstantExpr).getStringValue() instanceof PrivateHostName and
- not ma.getArgument(0).(CompileTimeConstantExpr).getStringValue() = "0:0:0:0:0:0:0:1"
- )
- )
- or
- exists(MethodCall ma |
- ma.getMethod().getName() in ["contains", "startsWith"] and
- ma.getMethod().getDeclaringType() instanceof TypeString and
- ma.getMethod().getNumberOfParameters() = 1 and
- ma.getQualifier() = this.asExpr() and
- ma.getAnArgument().(CompileTimeConstantExpr).getStringValue().regexpMatch(getIpAddressRegex()) // Matches IP-address-like strings
- )
- or
- exists(MethodCall ma |
- ma.getMethod().hasName("startsWith") and
- ma.getMethod()
- .getDeclaringType()
- .hasQualifiedName(["org.apache.commons.lang3", "org.apache.commons.lang"], "StringUtils") and
- ma.getMethod().getNumberOfParameters() = 2 and
- ma.getAnArgument() = this.asExpr() and
- ma.getAnArgument().(CompileTimeConstantExpr).getStringValue().regexpMatch(getIpAddressRegex())
- )
- or
- exists(MethodCall ma |
- ma.getMethod().getName() in ["equals", "equalsIgnoreCase"] and
- ma.getMethod()
- .getDeclaringType()
- .hasQualifiedName(["org.apache.commons.lang3", "org.apache.commons.lang"], "StringUtils") and
- ma.getMethod().getNumberOfParameters() = 2 and
- ma.getAnArgument() = this.asExpr() and
- ma.getAnArgument().(CompileTimeConstantExpr).getStringValue() instanceof PrivateHostName and
- not ma.getAnArgument().(CompileTimeConstantExpr).getStringValue() = "0:0:0:0:0:0:0:1"
- )
- }
-}
-
-/** A data flow sink for sql operation. */
-private class SqlOperationSink extends ClientSuppliedIpUsedInSecurityCheckSink instanceof QueryInjectionSink
-{ }
-
-/** A method that split string. */
-class SplitMethod extends Method {
- SplitMethod() {
- this.getNumberOfParameters() = 1 and
- this.hasQualifiedName("java.lang", "String", "split")
- }
-}
-
-string getIpAddressRegex() {
- result =
- "^((10\\.((1\\d{2})?|(2[0-4]\\d)?|(25[0-5])?|([1-9]\\d|[0-9])?)(\\.)?)|(192\\.168\\.)|172\\.(1[6789]|2[0-9]|3[01])\\.)((1\\d{2})?|(2[0-4]\\d)?|(25[0-5])?|([1-9]\\d|[0-9])?)(\\.)?((1\\d{2})?|(2[0-4]\\d)?|(25[0-5])?|([1-9]\\d|[0-9])?)$"
-}
diff --git a/java/ql/src/experimental/Security/CWE/CWE-352/JsonStringLib.qll b/java/ql/src/experimental/Security/CWE/CWE-352/JsonStringLib.qll
deleted file mode 100644
index e3cd008b603c..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-352/JsonStringLib.qll
+++ /dev/null
@@ -1,58 +0,0 @@
-deprecated module;
-
-import java
-import semmle.code.java.dataflow.DataFlow
-import semmle.code.java.dataflow.FlowSources
-
-/** Json string type data. */
-abstract class JsonStringSource extends DataFlow::Node { }
-
-/**
- * Convert to String using Gson library. *
- *
- * For example, in the method access `Gson.toJson(...)`,
- * the `Object` type data is converted to the `String` type data.
- */
-private class GsonString extends JsonStringSource {
- GsonString() {
- exists(MethodCall ma, Method m | ma.getMethod() = m |
- m.hasName("toJson") and
- m.getDeclaringType().getAnAncestor().hasQualifiedName("com.google.gson", "Gson") and
- this.asExpr() = ma
- )
- }
-}
-
-/**
- * Convert to String using Fastjson library.
- *
- * For example, in the method access `JSON.toJSONString(...)`,
- * the `Object` type data is converted to the `String` type data.
- */
-private class FastjsonString extends JsonStringSource {
- FastjsonString() {
- exists(MethodCall ma, Method m | ma.getMethod() = m |
- m.hasName("toJSONString") and
- m.getDeclaringType().getAnAncestor().hasQualifiedName("com.alibaba.fastjson", "JSON") and
- this.asExpr() = ma
- )
- }
-}
-
-/**
- * Convert to String using Jackson library.
- *
- * For example, in the method access `ObjectMapper.writeValueAsString(...)`,
- * the `Object` type data is converted to the `String` type data.
- */
-private class JacksonString extends JsonStringSource {
- JacksonString() {
- exists(MethodCall ma, Method m | ma.getMethod() = m |
- m.hasName("writeValueAsString") and
- m.getDeclaringType()
- .getAnAncestor()
- .hasQualifiedName("com.fasterxml.jackson.databind", "ObjectMapper") and
- this.asExpr() = ma
- )
- }
-}
diff --git a/java/ql/src/experimental/Security/CWE/CWE-352/JsonpInjection.java b/java/ql/src/experimental/Security/CWE/CWE-352/JsonpInjection.java
deleted file mode 100644
index 8f39efbc2b6a..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-352/JsonpInjection.java
+++ /dev/null
@@ -1,161 +0,0 @@
-import com.alibaba.fastjson.JSONObject;
-import com.fasterxml.jackson.databind.ObjectMapper;
-import com.google.gson.Gson;
-import java.io.BufferedReader;
-import java.io.IOException;
-import java.io.InputStreamReader;
-import java.io.PrintWriter;
-import java.util.HashMap;
-import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletResponse;
-import org.springframework.stereotype.Controller;
-import org.springframework.web.bind.annotation.GetMapping;
-import org.springframework.web.bind.annotation.RequestMapping;
-import org.springframework.web.bind.annotation.RequestMethod;
-import org.springframework.web.bind.annotation.RequestParam;
-import org.springframework.web.bind.annotation.ResponseBody;
-import org.springframework.web.multipart.MultipartFile;
-
-@Controller
-public class JsonpInjection {
-
- private static HashMap hashMap = new HashMap();
-
- static {
- hashMap.put("username","admin");
- hashMap.put("password","123456");
- }
-
- @GetMapping(value = "jsonp1")
- @ResponseBody
- public String bad1(HttpServletRequest request) {
- String resultStr = null;
- String jsonpCallback = request.getParameter("jsonpCallback");
- Gson gson = new Gson();
- String result = gson.toJson(hashMap);
- resultStr = jsonpCallback + "(" + result + ")";
- return resultStr;
- }
-
- @GetMapping(value = "jsonp2")
- @ResponseBody
- public String bad2(HttpServletRequest request) {
- String resultStr = null;
- String jsonpCallback = request.getParameter("jsonpCallback");
- resultStr = jsonpCallback + "(" + JSONObject.toJSONString(hashMap) + ")";
- return resultStr;
- }
-
- @GetMapping(value = "jsonp3")
- @ResponseBody
- public String bad3(HttpServletRequest request) {
- String resultStr = null;
- String jsonpCallback = request.getParameter("jsonpCallback");
- String jsonStr = getJsonStr(hashMap);
- resultStr = jsonpCallback + "(" + jsonStr + ")";
- return resultStr;
- }
-
- @GetMapping(value = "jsonp4")
- @ResponseBody
- public String bad4(HttpServletRequest request) {
- String resultStr = null;
- String jsonpCallback = request.getParameter("jsonpCallback");
- String restr = JSONObject.toJSONString(hashMap);
- resultStr = jsonpCallback + "(" + restr + ");";
- return resultStr;
- }
-
- @GetMapping(value = "jsonp5")
- @ResponseBody
- public void bad5(HttpServletRequest request,
- HttpServletResponse response) throws Exception {
- String jsonpCallback = request.getParameter("jsonpCallback");
- PrintWriter pw = null;
- Gson gson = new Gson();
- String result = gson.toJson(hashMap);
- String resultStr = null;
- pw = response.getWriter();
- resultStr = jsonpCallback + "(" + result + ")";
- pw.println(resultStr);
- }
-
- @GetMapping(value = "jsonp6")
- @ResponseBody
- public void bad6(HttpServletRequest request,
- HttpServletResponse response) throws Exception {
- String jsonpCallback = request.getParameter("jsonpCallback");
- PrintWriter pw = null;
- ObjectMapper mapper = new ObjectMapper();
- String result = mapper.writeValueAsString(hashMap);
- String resultStr = null;
- pw = response.getWriter();
- resultStr = jsonpCallback + "(" + result + ")";
- pw.println(resultStr);
- }
-
- @RequestMapping(value = "jsonp7", method = RequestMethod.GET)
- @ResponseBody
- public String bad7(HttpServletRequest request) {
- String resultStr = null;
- String jsonpCallback = request.getParameter("jsonpCallback");
- Gson gson = new Gson();
- String result = gson.toJson(hashMap);
- resultStr = jsonpCallback + "(" + result + ")";
- return resultStr;
- }
-
- @RequestMapping(value = "jsonp11")
- @ResponseBody
- public String good1(HttpServletRequest request) {
- JSONObject parameterObj = readToJSONObect(request);
- String resultStr = null;
- String jsonpCallback = request.getParameter("jsonpCallback");
- String restr = JSONObject.toJSONString(hashMap);
- resultStr = jsonpCallback + "(" + restr + ");";
- return resultStr;
- }
-
- @RequestMapping(value = "jsonp12")
- @ResponseBody
- public String good2(@RequestParam("file") MultipartFile file,HttpServletRequest request) {
- if(null == file){
- return "upload file error";
- }
- String fileName = file.getOriginalFilename();
- System.out.println("file operations");
- String resultStr = null;
- String jsonpCallback = request.getParameter("jsonpCallback");
- String restr = JSONObject.toJSONString(hashMap);
- resultStr = jsonpCallback + "(" + restr + ");";
- return resultStr;
- }
-
- public static JSONObject readToJSONObect(HttpServletRequest request){
- String jsonText = readPostContent(request);
- JSONObject jsonObj = JSONObject.parseObject(jsonText, JSONObject.class);
- return jsonObj;
- }
-
- public static String readPostContent(HttpServletRequest request){
- BufferedReader in= null;
- String content = null;
- String line = null;
- try {
- in = new BufferedReader(new InputStreamReader(request.getInputStream(),"UTF-8"));
- StringBuilder buf = new StringBuilder();
- while ((line = in.readLine()) != null) {
- buf.append(line);
- }
- content = buf.toString();
- } catch (IOException e) {
- e.printStackTrace();
- }
- String uri = request.getRequestURI();
- return content;
- }
-
- public static String getJsonStr(Object result) {
- return JSONObject.toJSONString(result);
- }
-}
\ No newline at end of file
diff --git a/java/ql/src/experimental/Security/CWE/CWE-352/JsonpInjection.qhelp b/java/ql/src/experimental/Security/CWE/CWE-352/JsonpInjection.qhelp
deleted file mode 100644
index e8fb89d3989f..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-352/JsonpInjection.qhelp
+++ /dev/null
@@ -1,37 +0,0 @@
-
-
-
-The software uses external input as the function name to wrap JSON data and returns it to the client as a request response.
-When there is a cross-domain problem, this could lead to information leakage.
-
-
-
-
-Adding Referer/Origin or random token verification processing can effectively prevent the leakage of sensitive information.
-
-
-
-
-The following examples show the bad case and the good case respectively. Bad cases, such as bad1 to bad7,
-will cause information leakage when there are cross-domain problems. In a good case, for example, in the good1
-method and the good2 method, When these two methods process the request, there must be a request body in the request, which does not meet the conditions of Jsonp injection.
-
-
-
-
-
-OWASPLondon20161124_JSON_Hijacking_Gareth_Heyes:
-JSON hijacking.
-
-
-Practical JSONP Injection:
-
- Completely controllable from the URL (GET variable)
-.
-
-
-
diff --git a/java/ql/src/experimental/Security/CWE/CWE-352/JsonpInjection.ql b/java/ql/src/experimental/Security/CWE/CWE-352/JsonpInjection.ql
deleted file mode 100644
index a555c4e99c98..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-352/JsonpInjection.ql
+++ /dev/null
@@ -1,53 +0,0 @@
-/**
- * @name JSONP Injection
- * @description User-controlled callback function names that are not verified are vulnerable
- * to jsonp injection attacks.
- * @kind path-problem
- * @problem.severity error
- * @precision high
- * @id java/jsonp-injection
- * @tags security
- * experimental
- * external/cwe/cwe-352
- */
-
-import java
-import semmle.code.java.dataflow.TaintTracking
-import semmle.code.java.dataflow.FlowSources
-import semmle.code.java.deadcode.WebEntryPoints
-import semmle.code.java.security.XSS
-deprecated import JsonpInjectionLib
-deprecated import RequestResponseFlow::PathGraph
-
-/** Taint-tracking configuration tracing flow from get method request sources to output jsonp data. */
-deprecated module RequestResponseFlowConfig implements DataFlow::ConfigSig {
- predicate isSource(DataFlow::Node source) {
- source instanceof ActiveThreatModelSource and
- any(RequestGetMethod m).polyCalls*(source.getEnclosingCallable())
- }
-
- predicate isSink(DataFlow::Node sink) {
- sink instanceof XssSink and
- any(RequestGetMethod m).polyCalls*(sink.getEnclosingCallable())
- }
-
- predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) {
- exists(MethodCall ma |
- isRequestGetParamMethod(ma) and pred.asExpr() = ma.getQualifier() and succ.asExpr() = ma
- )
- }
-}
-
-deprecated module RequestResponseFlow = TaintTracking::Global;
-
-deprecated query predicate problems(
- DataFlow::Node sinkNode, RequestResponseFlow::PathNode source, RequestResponseFlow::PathNode sink,
- string message1, DataFlow::Node sourceNode, string message2
-) {
- RequestResponseFlow::flowPath(source, sink) and
- JsonpInjectionFlow::flowTo(sink.getNode()) and
- sinkNode = sink.getNode() and
- message1 = "Jsonp response might include code from $@." and
- sourceNode = source.getNode() and
- message2 = "this user input"
-}
diff --git a/java/ql/src/experimental/Security/CWE/CWE-352/JsonpInjectionLib.qll b/java/ql/src/experimental/Security/CWE/CWE-352/JsonpInjectionLib.qll
deleted file mode 100644
index 1ed987705228..000000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-352/JsonpInjectionLib.qll
+++ /dev/null
@@ -1,117 +0,0 @@
-deprecated module;
-
-import java
-private import JsonStringLib
-private import semmle.code.java.security.XSS
-private import semmle.code.java.dataflow.TaintTracking
-private import semmle.code.java.dataflow.FlowSources
-
-/**
- * A method that is called to handle an HTTP GET request.
- */
-abstract class RequestGetMethod extends Method {
- RequestGetMethod() {
- not exists(MethodCall ma |
- // Exclude apparent GET handlers that read a request entity, because this likely indicates this is not in fact a GET handler.
- // This is particularly a problem with Spring handlers, which can sometimes neglect to specify a request method.
- // Even if it is in fact a GET handler, such a request method will be unusable in the context `