Merge branch 'main' into atorralba/promote-unsafe-android-webview-fetch

java/ql/src/Frameworks/JavaEE/EJB/EjbContainerInterference.ql

@@ -7,6 +7,7 @@
  *              Such operations could interfere with the EJB container's operation.
  * @kind problem
  * @problem.severity error
+ * @security-severity 4.9
  * @precision low
  * @id java/ejb/container-interference
  * @tags reliability

java/ql/src/Frameworks/JavaEE/EJB/EjbFileIO.ql

@@ -5,6 +5,7 @@
  *              for enterprise components.
  * @kind problem
  * @problem.severity error
+ * @security-severity 4.9
  * @precision low
  * @id java/ejb/file-io
  * @tags reliability

java/ql/src/Frameworks/JavaEE/EJB/EjbNative.ql

@@ -4,6 +4,7 @@
  *              Such use could compromise security and system stability.
  * @kind problem
  * @problem.severity error
+ * @security-severity 4.9
  * @precision low
  * @id java/ejb/native-code
  * @tags reliability

java/ql/src/Frameworks/JavaEE/EJB/EjbReflection.ql

@@ -4,6 +4,7 @@
  *              as this could compromise security.
  * @kind problem
  * @problem.severity error
+ * @security-severity 4.9
  * @precision low
  * @id java/ejb/reflection
  * @tags external/cwe/cwe-573

java/ql/src/Frameworks/JavaEE/EJB/EjbSecurityConfiguration.ql

@@ -5,6 +5,7 @@
  *              This functionality is reserved for the EJB container for security reasons.
  * @kind problem
  * @problem.severity error
+ * @security-severity 4.9
  * @precision low
  * @id java/ejb/security-configuration-access
  * @tags external/cwe/cwe-573

java/ql/src/Frameworks/JavaEE/EJB/EjbSerialization.ql

@@ -4,6 +4,7 @@
  *              the Java serialization protocol, since their use could compromise security.
  * @kind problem
  * @problem.severity error
+ * @security-severity 4.9
  * @precision low
  * @id java/ejb/substitution-in-serialization
  * @tags external/cwe/cwe-573

java/ql/src/Frameworks/JavaEE/EJB/EjbSetSocketOrUrlFactory.ql

@@ -5,6 +5,7 @@
  *              compromise security or interfere with the EJB container's operation.
  * @kind problem
  * @problem.severity error
+ * @security-severity 4.9
  * @precision low
  * @id java/ejb/socket-or-stream-handler-factory
  * @tags reliability

java/ql/src/Likely Bugs/Arithmetic/InformationLoss.ql

@@ -5,6 +5,7 @@
  *              numeric errors such as overflows.
  * @kind problem
  * @problem.severity warning
+ * @security-severity 5.9
  * @precision very-high
  * @id java/implicit-cast-in-compound-assignment
  * @tags reliability

java/ql/src/Likely Bugs/Arithmetic/RandomUsedOnce.ql

@@ -4,6 +4,7 @@
  *              guarantee an evenly distributed sequence of random numbers.
  * @kind problem
  * @problem.severity warning
+ * @security-severity 5.9
  * @precision medium
  * @id java/random-used-once
  * @tags reliability

java/ql/src/Likely Bugs/Concurrency/UnreleasedLock.ql

@@ -4,6 +4,7 @@
  *              may cause a deadlock.
  * @kind problem
  * @problem.severity error
+ * @security-severity 6.9
  * @precision medium
  * @id java/unreleased-lock
  * @tags reliability

java/ql/src/Likely Bugs/Resource Leaks/CloseReader.qhelp

@@ -14,7 +14,7 @@ but not closed may cause a resource leak.
 
 <p>Ensure that the resource is always closed to avoid a resource leak. Note that, because of exceptions,
 it is safest to close a resource in a <code>finally</code> block. (However, this is unnecessary for
-subclasses of <code>StringReader</code> and <code>ByteArrayInputStream</code>.)
+subclasses of <code>CharArrayReader</code>, <code>StringReader</code> and <code>ByteArrayInputStream</code>.)
 </p>
 
 <p>For Java 7 or later, the recommended way to close resources that implement <code>java.lang.AutoCloseable</code>

java/ql/src/Likely Bugs/Resource Leaks/CloseReader.ql

@@ -17,16 +17,16 @@ import CloseType
 
 predicate readerType(RefType t) {
   exists(RefType sup | sup = t.getASupertype*() |
-    sup.hasName("Reader") or
-    sup.hasName("InputStream") or
+    sup.hasQualifiedName("java.io", ["Reader", "InputStream"]) or
     sup.hasQualifiedName("java.util.zip", "ZipFile")
   )
 }
 
 predicate safeReaderType(RefType t) {
   exists(RefType sup | sup = t.getASupertype*() |
-    sup.hasName("StringReader") or
-    sup.hasName("ByteArrayInputStream") or
+    sup.hasQualifiedName("java.io", ["CharArrayReader", "StringReader", "ByteArrayInputStream"])
+    or
+    // Note: It is unclear which specific class this is supposed to match
     sup.hasName("StringInputStream")
   )
 }

java/ql/src/Likely Bugs/Resource Leaks/CloseWriter.qhelp

@@ -14,7 +14,7 @@ but not properly closed later may cause a resource leak.
 
 <p>Ensure that the resource is always closed to avoid a resource leak. Note that, because of exceptions,
 it is safest to close a resource properly in a <code>finally</code> block. (However, this is unnecessary for
-subclasses of <code>StringWriter</code> and <code>ByteArrayOutputStream</code>.)</p>
+subclasses of <code>CharArrayWriter</code>, <code>StringWriter</code> and <code>ByteArrayOutputStream</code>.)</p>
 
 <p>For Java 7 or later, the recommended way to close resources that implement <code>java.lang.AutoCloseable</code>
 is to declare them within a <code>try-with-resources</code> statement, so that they are closed implicitly.</p>

java/ql/src/Likely Bugs/Resource Leaks/CloseWriter.ql

@@ -17,15 +17,13 @@ import CloseType
 
 predicate writerType(RefType t) {
   exists(RefType sup | sup = t.getASupertype*() |
-    sup.hasName("Writer") or
-    sup.hasName("OutputStream")
+    sup.hasQualifiedName("java.io", ["Writer", "OutputStream"])
   )
 }
 
 predicate safeWriterType(RefType t) {
   exists(RefType sup | sup = t.getASupertype*() |
-    sup.hasName("StringWriter") or
-    sup.hasName("ByteArrayOutputStream")
+    sup.hasQualifiedName("java.io", ["CharArrayWriter", "StringWriter", "ByteArrayOutputStream"])
   )
 }
 

java/ql/src/Metrics/Summaries/LinesOfCode.ql

@@ -6,6 +6,7 @@
  *              or comments.
  * @kind metric
  * @tags summary
+ *       lines-of-code
  */
 
 import java

java/ql/src/Security/CWE/CWE-020/UntrustedDataToExternalAPI.ql

@@ -5,6 +5,7 @@
  * @kind path-problem
  * @precision low
  * @problem.severity error
+ * @security-severity 5.9
  * @tags security external/cwe/cwe-20
  */
 

java/ql/src/Security/CWE/CWE-022/TaintedPath.ql

@@ -3,6 +3,7 @@
  * @description Accessing paths influenced by users can allow an attacker to access unexpected resources.
  * @kind path-problem
  * @problem.severity error
+ * @security-severity 6.4
  * @precision high
  * @id java/path-injection
  * @tags security

java/ql/src/Security/CWE/CWE-022/TaintedPathLocal.ql

@@ -3,6 +3,7 @@
  * @description Accessing paths influenced by users can allow an attacker to access unexpected resources.
  * @kind path-problem
  * @problem.severity recommendation
+ * @security-severity 6.4
  * @precision medium
  * @id java/path-injection-local
  * @tags security

java/ql/src/Security/CWE/CWE-022/ZipSlip.ql

@@ -6,6 +6,7 @@
  * @kind path-problem
  * @id java/zipslip
  * @problem.severity error
+ * @security-severity 6.4
  * @precision high
  * @tags security
  *       external/cwe/cwe-022

java/ql/src/Security/CWE/CWE-078/ExecRelative.ql

@@ -4,6 +4,7 @@
  *              malicious changes in the PATH environment variable.
  * @kind problem
  * @problem.severity warning
+ * @security-severity 5.9
  * @precision medium
  * @id java/relative-path-command
  * @tags security

java/ql/src/Security/CWE/CWE-078/ExecTainted.ql

@@ -4,6 +4,7 @@
  *              changes in the strings.
  * @kind path-problem
  * @problem.severity error
+ * @security-severity 5.9
  * @precision high
  * @id java/command-line-injection
  * @tags security

java/ql/src/Security/CWE/CWE-078/ExecTaintedLocal.ql

@@ -4,6 +4,7 @@
  *              changes in the strings.
  * @kind path-problem
  * @problem.severity recommendation
+ * @security-severity 5.9
  * @precision medium
  * @id java/command-line-injection-local
  * @tags security

java/ql/src/Security/CWE/CWE-078/ExecUnescaped.ql

@@ -4,6 +4,7 @@
  *              insertion of special characters in the strings.
  * @kind problem
  * @problem.severity error
+ * @security-severity 5.9
  * @precision high
  * @id java/concatenated-command-line
  * @tags security

java/ql/src/Security/CWE/CWE-079/XSS.ql

@@ -4,6 +4,7 @@
  *              allows for a cross-site scripting vulnerability.
  * @kind path-problem
  * @problem.severity error
+ * @security-severity 2.9
  * @precision high
  * @id java/xss
  * @tags security

java/ql/src/Security/CWE/CWE-079/XSSLocal.ql

@@ -4,6 +4,7 @@
  *              allows for a cross-site scripting vulnerability.
  * @kind path-problem
  * @problem.severity recommendation
+ * @security-severity 2.9
  * @precision medium
  * @id java/xss-local
  * @tags security

java/ql/src/Security/CWE/CWE-089/SqlTainted.ql

@@ -4,6 +4,7 @@
  *              malicious code by the user.
  * @kind path-problem
  * @problem.severity error
+ * @security-severity 6.4
  * @precision high
  * @id java/sql-injection
  * @tags security

java/ql/src/Security/CWE/CWE-089/SqlTaintedLocal.ql

@@ -4,6 +4,7 @@
  *              malicious code by the user.
  * @kind path-problem
  * @problem.severity recommendation
+ * @security-severity 6.4
  * @precision medium
  * @id java/sql-injection-local
  * @tags security

java/ql/src/Security/CWE/CWE-089/SqlUnescaped.ql

@@ -4,6 +4,7 @@
  *              characters is vulnerable to insertion of malicious code.
  * @kind problem
  * @problem.severity error
+ * @security-severity 6.4
  * @precision high
  * @id java/concatenated-sql-query
  * @tags security

java/ql/src/Security/CWE/CWE-090/LdapInjection.ql

@@ -4,6 +4,7 @@
  *              malicious LDAP code by the user.
  * @kind path-problem
  * @problem.severity error
+ * @security-severity 5.9
  * @precision high
  * @id java/ldap-injection
  * @tags security

java/ql/src/Security/CWE/CWE-094/InsecureBeanValidation.ql

@@ -3,6 +3,7 @@
  * @description User-controlled data may be evaluated as a Java EL expression, leading to arbitrary code execution.
  * @kind path-problem
  * @problem.severity error
+ * @security-severity 10.0
  * @precision high
  * @id java/insecure-bean-validation
  * @tags security

java/ql/src/Security/CWE/CWE-094/JexlInjection.ql

@@ -0,0 +1,38 @@
+/**
+ * @name Expression language injection (JEXL)
+ * @description Evaluation of a user-controlled JEXL expression
+ *              may lead to arbitrary code execution.
+ * @kind path-problem
+ * @problem.severity error
+ * @security-severity 10.0
+ * @precision high
+ * @id java/jexl-expression-injection
+ * @tags security
+ *       external/cwe/cwe-094
+ */
+
+import java
+import semmle.code.java.dataflow.FlowSources
+import semmle.code.java.security.JexlInjection
+import DataFlow::PathGraph
+
+/**
+ * A taint-tracking configuration for unsafe user input
+ * that is used to construct and evaluate a JEXL expression.
+ * It supports both JEXL 2 and 3.
+ */
+class JexlInjectionConfig extends TaintTracking::Configuration {
+  JexlInjectionConfig() { this = "JexlInjectionConfig" }
+
+  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
+
+  override predicate isSink(DataFlow::Node sink) { sink instanceof JexlEvaluationSink }
+
+  override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
+    any(JexlInjectionAdditionalTaintStep c).step(node1, node2)
+  }
+}
+
+from DataFlow::PathNode source, DataFlow::PathNode sink, JexlInjectionConfig conf
+where conf.hasFlowPath(source, sink)
+select sink.getNode(), source, sink, "JEXL injection from $@.", source.getNode(), "this user input"

java/ql/src/Security/CWE/CWE-1104/MavenPomDependsOnBintray.ql

@@ -3,6 +3,7 @@
  * @description Using a deprecated artifact repository may eventually give attackers access for a supply chain attack.
  * @kind problem
  * @problem.severity error
+ * @security-severity 6.5
  * @precision very-high
  * @id java/maven/dependency-upon-bintray
  * @tags security

java/ql/src/Security/CWE/CWE-113/NettyResponseSplitting.ql

@@ -5,6 +5,7 @@
  *              an HTTP header.
  * @kind problem
  * @problem.severity error
+ * @security-severity 3.6
  * @precision high
  * @id java/netty-http-response-splitting
  * @tags security

java/ql/src/Security/CWE/CWE-113/ResponseSplitting.ql

@@ -4,6 +4,7 @@
  *              makes code vulnerable to attack by header splitting.
  * @kind path-problem
  * @problem.severity error
+ * @security-severity 3.6
  * @precision high
  * @id java/http-response-splitting
  * @tags security

java/ql/src/Security/CWE/CWE-113/ResponseSplittingLocal.ql

@@ -4,6 +4,7 @@
  *              makes code vulnerable to attack by header splitting.
  * @kind path-problem
  * @problem.severity recommendation
+ * @security-severity 3.6
  * @precision medium
  * @id java/http-response-splitting-local
  * @tags security

java/ql/src/Security/CWE/CWE-129/ImproperValidationOfArrayConstruction.ql

@@ -3,6 +3,7 @@
  * @description Using unvalidated external input as the argument to a construction of an array can lead to index out of bound exceptions.
  * @kind path-problem
  * @problem.severity warning
+ * @security-severity 5.9
  * @precision medium
  * @id java/improper-validation-of-array-construction
  * @tags security

java/ql/src/Security/CWE/CWE-129/ImproperValidationOfArrayConstructionCodeSpecified.ql

@@ -4,6 +4,7 @@
  *              a construction of an array can lead to index out of bound exceptions.
  * @kind path-problem
  * @problem.severity recommendation
+ * @security-severity 5.9
  * @precision medium
  * @id java/improper-validation-of-array-construction-code-specified
  * @tags security

java/ql/src/Security/CWE/CWE-129/ImproperValidationOfArrayConstructionLocal.ql

@@ -4,6 +4,7 @@
  *              a construction of an array can lead to index out of bound exceptions.
  * @kind path-problem
  * @problem.severity recommendation
+ * @security-severity 5.9
  * @precision medium
  * @id java/improper-validation-of-array-construction-local
  * @tags security

java/ql/src/Security/CWE/CWE-129/ImproperValidationOfArrayIndex.ql

@@ -3,6 +3,7 @@
  * @description Using external input as an index to an array, without proper validation, can lead to index out of bound exceptions.
  * @kind path-problem
  * @problem.severity warning
+ * @security-severity 5.9
  * @precision medium
  * @id java/improper-validation-of-array-index
  * @tags security

java/ql/src/Security/CWE/CWE-129/ImproperValidationOfArrayIndexCodeSpecified.ql

@@ -4,6 +4,7 @@
  *              proper validation, can lead to index out of bound exceptions.
  * @kind path-problem
  * @problem.severity recommendation
+ * @security-severity 5.9
  * @precision medium
  * @id java/improper-validation-of-array-index-code-specified
  * @tags security

java/ql/src/Security/CWE/CWE-129/ImproperValidationOfArrayIndexLocal.ql

@@ -4,6 +4,7 @@
  *              proper validation, can lead to index out of bound exceptions.
  * @kind path-problem
  * @problem.severity recommendation
+ * @security-severity 5.9
  * @precision medium
  * @id java/improper-validation-of-array-index-local
  * @tags security

java/ql/src/Security/CWE/CWE-134/ExternallyControlledFormatString.ql

@@ -3,6 +3,7 @@
  * @description Using external input in format strings can lead to exceptions or information leaks.
  * @kind path-problem
  * @problem.severity error
+ * @security-severity 6.9
  * @precision high
  * @id java/tainted-format-string
  * @tags security

java/ql/src/Security/CWE/CWE-134/ExternallyControlledFormatStringLocal.ql

@@ -3,6 +3,7 @@
  * @description Using external input in format strings can lead to exceptions or information leaks.
  * @kind path-problem
  * @problem.severity recommendation
+ * @security-severity 6.9
  * @precision medium
  * @id java/tainted-format-string-local
  * @tags security

java/ql/src/Security/CWE/CWE-190/ArithmeticTainted.ql

@@ -4,6 +4,7 @@
  *              overflows.
  * @kind path-problem
  * @problem.severity warning
+ * @security-severity 5.9
  * @precision medium
  * @id java/tainted-arithmetic
  * @tags security

java/ql/src/Security/CWE/CWE-190/ArithmeticTaintedLocal.ql

@@ -4,6 +4,7 @@
  *              overflows.
  * @kind path-problem
  * @problem.severity recommendation
+ * @security-severity 5.9
  * @precision medium
  * @id java/tainted-arithmetic-local
  * @tags security

java/ql/src/Security/CWE/CWE-190/ArithmeticUncontrolled.ql

@@ -4,6 +4,7 @@
  *              overflows.
  * @kind path-problem
  * @problem.severity warning
+ * @security-severity 5.9
  * @precision medium
  * @id java/uncontrolled-arithmetic
  * @tags security

java/ql/src/Security/CWE/CWE-190/ArithmeticWithExtremeValues.ql

@@ -4,6 +4,7 @@
  *              is then used in an arithmetic expression, this may result in an overflow.
  * @kind path-problem
  * @problem.severity recommendation
+ * @security-severity 5.9
  * @precision medium
  * @id java/extreme-value-arithmetic
  * @tags security

java/ql/src/Security/CWE/CWE-190/ComparisonWithWiderType.ql

@@ -4,6 +4,7 @@
  *              to behave unexpectedly.
  * @kind problem
  * @problem.severity warning
+ * @security-severity 5.9
  * @precision medium
  * @id java/comparison-with-wider-type
  * @tags reliability

java/ql/src/Security/CWE/CWE-209/StackTraceExposure.ql

@@ -5,6 +5,7 @@
  *              that are useful to an attacker for developing a subsequent exploit.
  * @kind problem
  * @problem.severity error
+ * @security-severity 3.6
  * @precision high
  * @id java/stack-trace-exposure
  * @tags security
@@ -15,7 +16,7 @@
 import java
 import semmle.code.java.dataflow.DataFlow
 import semmle.code.java.dataflow.TaintTracking
-import semmle.code.java.security.XSS
+import semmle.code.java.security.InformationLeak
 
 /**
  * One of the `printStackTrace()` overloads on `Throwable`.
@@ -83,14 +84,14 @@ predicate stackTraceExpr(Expr exception, MethodAccess stackTraceString) {
   )
 }
 
-class StackTraceStringToXssSinkFlowConfig extends TaintTracking::Configuration {
-  StackTraceStringToXssSinkFlowConfig() {
-    this = "StackTraceExposure::StackTraceStringToXssSinkFlowConfig"
+class StackTraceStringToHttpResponseSinkFlowConfig extends TaintTracking::Configuration {
+  StackTraceStringToHttpResponseSinkFlowConfig() {
+    this = "StackTraceExposure::StackTraceStringToHttpResponseSinkFlowConfig"
   }
 
   override predicate isSource(DataFlow::Node src) { stackTraceExpr(_, src.asExpr()) }
 
-  override predicate isSink(DataFlow::Node sink) { sink instanceof XssSink }
+  override predicate isSink(DataFlow::Node sink) { sink instanceof InformationLeakSink }
 }
 
 /**
@@ -105,8 +106,8 @@ predicate printsStackExternally(MethodAccess call, Expr stackTrace) {
 /**
  * A stringified stack trace flows to an external sink.
  */
-predicate stringifiedStackFlowsExternally(XssSink externalExpr, Expr stackTrace) {
-  exists(MethodAccess stackTraceString, StackTraceStringToXssSinkFlowConfig conf |
+predicate stringifiedStackFlowsExternally(DataFlow::Node externalExpr, Expr stackTrace) {
+  exists(MethodAccess stackTraceString, StackTraceStringToHttpResponseSinkFlowConfig conf |
     stackTraceExpr(stackTrace, stackTraceString) and
     conf.hasFlow(DataFlow::exprNode(stackTraceString), externalExpr)
   )
@@ -123,21 +124,21 @@ class GetMessageFlowSource extends MethodAccess {
   }
 }
 
-class GetMessageFlowSourceToXssSinkFlowConfig extends TaintTracking::Configuration {
-  GetMessageFlowSourceToXssSinkFlowConfig() {
-    this = "StackTraceExposure::GetMessageFlowSourceToXssSinkFlowConfig"
+class GetMessageFlowSourceToHttpResponseSinkFlowConfig extends TaintTracking::Configuration {
+  GetMessageFlowSourceToHttpResponseSinkFlowConfig() {
+    this = "StackTraceExposure::GetMessageFlowSourceToHttpResponseSinkFlowConfig"
   }
 
   override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof GetMessageFlowSource }
 
-  override predicate isSink(DataFlow::Node sink) { sink instanceof XssSink }
+  override predicate isSink(DataFlow::Node sink) { sink instanceof InformationLeakSink }
 }
 
 /**
  * A call to `getMessage()` that then flows to a servlet response.
  */
-predicate getMessageFlowsExternally(XssSink externalExpr, GetMessageFlowSource getMessage) {
-  any(GetMessageFlowSourceToXssSinkFlowConfig conf)
+predicate getMessageFlowsExternally(DataFlow::Node externalExpr, GetMessageFlowSource getMessage) {
+  any(GetMessageFlowSourceToHttpResponseSinkFlowConfig conf)
       .hasFlow(DataFlow::exprNode(getMessage), externalExpr)
 }
 

java/ql/src/Security/CWE/CWE-297/UnsafeHostnameVerification.ql

@@ -3,6 +3,7 @@
  * @description Marking a certificate as valid for a host without checking the certificate hostname allows an attacker to perform a machine-in-the-middle attack.
  * @kind path-problem
  * @problem.severity error
+ * @security-severity 4.9
  * @precision high
  * @id java/unsafe-hostname-verification
  * @tags security

java/ql/src/Security/CWE/CWE-312/CleartextStorageClass.ql

@@ -3,6 +3,7 @@
  * @description Storing sensitive information in cleartext can expose it to an attacker.
  * @kind problem
  * @problem.severity recommendation
+ * @security-severity 5.9
  * @precision medium
  * @id java/cleartext-storage-in-class
  * @tags security

java/ql/src/Security/CWE/CWE-312/CleartextStorageCookie.ql

@@ -3,6 +3,7 @@
  * @description Storing sensitive information in cleartext can expose it to an attacker.
  * @kind problem
  * @problem.severity error
+ * @security-severity 2.9
  * @precision high
  * @id java/cleartext-storage-in-cookie
  * @tags security

java/ql/src/Security/CWE/CWE-312/CleartextStorageProperties.ql

@@ -3,6 +3,7 @@
  * @description Storing sensitive information in cleartext can expose it to an attacker.
  * @kind problem
  * @problem.severity warning
+ * @security-severity 6.4
  * @precision medium
  * @id java/cleartext-storage-in-properties
  * @tags security

java/ql/src/Security/CWE/CWE-319/HttpsUrls.ql

@@ -3,6 +3,7 @@
  * @description Non-HTTPS connections can be intercepted by third parties.
  * @kind path-problem
  * @problem.severity recommendation
+ * @security-severity 5.2
  * @precision medium
  * @id java/non-https-url
  * @tags security

java/ql/src/Security/CWE/CWE-319/UseSSL.ql

@@ -3,6 +3,7 @@
  * @description Non-SSL connections can be intercepted by third parties.
  * @kind problem
  * @problem.severity recommendation
+ * @security-severity 5.2
  * @precision medium
  * @id java/non-ssl-connection
  * @tags security

java/ql/src/Security/CWE/CWE-319/UseSSLSocketFactories.ql

@@ -4,6 +4,7 @@
  *              third parties.
  * @kind problem
  * @problem.severity recommendation
+ * @security-severity 5.2
  * @precision medium
  * @id java/non-ssl-socket-factory
  * @tags security

java/ql/src/Security/CWE/CWE-327/BrokenCryptoAlgorithm.ql

@@ -3,6 +3,7 @@
  * @description Using broken or weak cryptographic algorithms can allow an attacker to compromise security.
  * @kind path-problem
  * @problem.severity warning
+ * @security-severity 5.2
  * @precision high
  * @id java/weak-cryptographic-algorithm
  * @tags security

java/ql/src/Security/CWE/CWE-327/MaybeBrokenCryptoAlgorithm.ql

@@ -3,6 +3,7 @@
  * @description Using broken or weak cryptographic algorithms can allow an attacker to compromise security.
  * @kind path-problem
  * @problem.severity warning
+ * @security-severity 5.2
  * @precision medium
  * @id java/potentially-weak-cryptographic-algorithm
  * @tags security

java/ql/src/Security/CWE/CWE-335/PredictableSeed.ql

@@ -3,9 +3,11 @@
  * @description Using a predictable seed in a pseudo-random number generator can lead to predictability of the numbers generated by it.
  * @kind problem
  * @problem.severity error
+ * @security-severity 5.9
  * @precision high
  * @id java/predictable-seed
  * @tags security
+ *       external/cwe/cwe-335
  */
 
 import java

java/ql/src/Security/CWE/CWE-338/JHipsterGeneratedPRNG.ql

@@ -3,6 +3,7 @@
  * @description Using a vulnerable version of JHipster to generate random numbers makes it easier for attackers to take over accounts.
  * @kind problem
  * @problem.severity error
+ * @security-severity 5.9
  * @precision very-high
  * @id java/jhipster-prng
  * @tags security

java/ql/src/Security/CWE/CWE-352/SpringCSRFProtection.ql

@@ -4,6 +4,7 @@
  *              a Cross-Site Request Forgery (CSRF) attack.
  * @kind problem
  * @problem.severity error
+ * @security-severity 6.4
  * @precision high
  * @id java/spring-disabled-csrf-protection
  * @tags security

java/ql/src/Security/CWE/CWE-367/TOCTOURace.ql

@@ -4,6 +4,7 @@
  *              if the state may be changed between the check and use.
  * @kind problem
  * @problem.severity warning
+ * @security-severity 5.9
  * @precision medium
  * @id java/toctou-race-condition
  * @tags security

java/ql/src/Security/CWE/CWE-421/SocketAuthRace.ql

@@ -3,6 +3,7 @@
  * @description Opening a socket after authenticating via a different channel may allow an attacker to connect to the port first.
  * @kind problem
  * @problem.severity warning
+ * @security-severity 10.0
  * @precision medium
  * @id java/socket-auth-race-condition
  * @tags security

java/ql/src/Security/CWE/CWE-502/UnsafeDeserialization.qhelp

@@ -14,8 +14,8 @@ may have unforeseen effects, such as the execution of arbitrary code.
 </p>
 <p>
 There are many different serialization frameworks.  This query currently
-supports Kryo, XmlDecoder, XStream, SnakeYaml, and Java IO serialization through
-<code>ObjectInputStream</code>/<code>ObjectOutputStream</code>.
+supports Kryo, XmlDecoder, XStream, SnakeYaml, JYaml, JsonIO, YAMLBeans, HessianBurlap, Castor, Burlap 
+and Java IO serialization through <code>ObjectInputStream</code>/<code>ObjectOutputStream</code>.
 </p>
 </overview>
 
@@ -75,6 +75,22 @@ Alvaro Muñoz &amp; Christian Schneider, RSAConference 2016:
 SnakeYaml documentation on deserialization:
 <a href="https://bitbucket.org/asomov/snakeyaml/wiki/Documentation#markdown-header-loading-yaml">SnakeYaml deserialization</a>.
 </li>
+<li>
+Hessian deserialization and related gadget chains:
+<a href="https://paper.seebug.org/1137/">Hessian deserialization</a>.
+</li>
+<li>
+Castor and Hessian java deserialization vulnerabilities:
+<a href="https://securitylab.github.com/research/hessian-java-deserialization-castor-vulnerabilities/">Castor and Hessian deserialization</a>.
+</li>
+<li>
+Remote code execution in JYaml library:
+<a href="https://www.cybersecurity-help.cz/vdb/SB2020022512">JYaml deserialization</a>.
+</li>
+<li>
+JsonIO deserialization vulnerabilities:
+<a href="https://klezvirus.github.io/Advanced-Web-Hacking/Serialisation/">JsonIO deserialization</a>.
+</li>
 </references>
 
 </qhelp>

java/ql/src/Security/CWE/CWE-502/UnsafeDeserialization.ql

@@ -4,6 +4,7 @@
  *              execute arbitrary code.
  * @kind path-problem
  * @problem.severity error
+ * @security-severity 5.9
  * @precision high
  * @id java/unsafe-deserialization
  * @tags security
@@ -21,6 +22,39 @@ class UnsafeDeserializationConfig extends TaintTracking::Configuration {
   override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
 
   override predicate isSink(DataFlow::Node sink) { sink instanceof UnsafeDeserializationSink }
+
+  override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
+    exists(ClassInstanceExpr cie |
+      cie.getArgument(0) = pred.asExpr() and
+      cie = succ.asExpr() and
+      (
+        cie.getConstructor().getDeclaringType() instanceof JsonIoJsonReader or
+        cie.getConstructor().getDeclaringType() instanceof YamlBeansReader or
+        cie.getConstructor().getDeclaringType().getASupertype*() instanceof UnsafeHessianInput or
+        cie.getConstructor().getDeclaringType() instanceof BurlapInput
+      )
+    )
+    or
+    exists(MethodAccess ma |
+      ma.getMethod() instanceof BurlapInputInitMethod and
+      ma.getArgument(0) = pred.asExpr() and
+      ma.getQualifier() = succ.asExpr()
+    )
+  }
+
+  override predicate isSanitizer(DataFlow::Node node) {
+    exists(ClassInstanceExpr cie |
+      cie.getConstructor().getDeclaringType() instanceof JsonIoJsonReader and
+      cie = node.asExpr() and
+      exists(SafeJsonIoConfig sji | sji.hasFlowToExpr(cie.getArgument(1)))
+    )
+    or
+    exists(MethodAccess ma |
+      ma.getMethod() instanceof JsonIoJsonToJavaMethod and
+      ma.getArgument(0) = node.asExpr() and
+      exists(SafeJsonIoConfig sji | sji.hasFlowToExpr(ma.getArgument(1)))
+    )
+  }
 }
 
 from DataFlow::PathNode source, DataFlow::PathNode sink, UnsafeDeserializationConfig conf

java/ql/src/Security/CWE/CWE-601/UrlRedirect.ql

@@ -4,6 +4,7 @@
  *              may cause redirection to malicious web sites.
  * @kind path-problem
  * @problem.severity error
+ * @security-severity 2.7
  * @precision high
  * @id java/unvalidated-url-redirection
  * @tags security

java/ql/src/Security/CWE/CWE-601/UrlRedirectLocal.ql

@@ -4,6 +4,7 @@
  *              may cause redirection to malicious web sites.
  * @kind path-problem
  * @problem.severity recommendation
+ * @security-severity 2.7
  * @precision medium
  * @id java/unvalidated-url-redirection-local
  * @tags security

java/ql/src/Security/CWE/CWE-611/XXE.ql

@@ -4,6 +4,7 @@
  * references may lead to disclosure of confidential data or denial of service.
  * @kind path-problem
  * @problem.severity error
+ * @security-severity 5.9
  * @precision high
  * @id java/xxe
  * @tags security

java/ql/src/Security/CWE/CWE-614/InsecureCookie.ql

@@ -4,6 +4,7 @@
  *              interception.
  * @kind problem
  * @problem.severity error
+ * @security-severity 2.9
  * @precision high
  * @id java/insecure-cookie
  * @tags security

java/ql/src/Security/CWE/CWE-643/XPathInjection.ql

@@ -4,6 +4,7 @@
  *              malicious code by the user.
  * @kind path-problem
  * @problem.severity error
+ * @security-severity 5.9
  * @precision high
  * @id java/xml/xpath-injection
  * @tags security

java/ql/src/Security/CWE/CWE-676/PotentiallyDangerousFunction.ql

@@ -3,6 +3,7 @@
  * @description Certain standard library routines are dangerous to call.
  * @kind problem
  * @problem.severity warning
+ * @security-severity 10.0
  * @precision medium
  * @id java/potentially-dangerous-function
  * @tags reliability

java/ql/src/Security/CWE/CWE-681/NumericCastTainted.ql

@@ -4,6 +4,7 @@
  *              can cause unexpected truncation.
  * @kind path-problem
  * @problem.severity error
+ * @security-severity 5.9
  * @precision high
  * @id java/tainted-numeric-cast
  * @tags security

java/ql/src/Security/CWE/CWE-681/NumericCastTaintedLocal.ql

@@ -4,6 +4,7 @@
  *              can cause unexpected truncation.
  * @kind path-problem
  * @problem.severity recommendation
+ * @security-severity 5.9
  * @precision medium
  * @id java/tainted-numeric-cast-local
  * @tags security

java/ql/src/Security/CWE/CWE-732/ReadingFromWorldWritableFile.ql

@@ -4,6 +4,7 @@
  *              the file may be modified or removed by external actors.
  * @kind problem
  * @problem.severity error
+ * @security-severity 5.9
  * @precision high
  * @id java/world-writable-file-read
  * @tags security

java/ql/src/Security/CWE/CWE-798/HardcodedCredentialsApiCall.ql

@@ -3,6 +3,7 @@
  * @description Using a hard-coded credential in a call to a sensitive Java API may compromise security.
  * @kind path-problem
  * @problem.severity error
+ * @security-severity 5.9
  * @precision medium
  * @id java/hardcoded-credential-api-call
  * @tags security

java/ql/src/Security/CWE/CWE-798/HardcodedCredentialsComparison.ql

@@ -3,6 +3,7 @@
  * @description Comparing a parameter to a hard-coded credential may compromise security.
  * @kind problem
  * @problem.severity error
+ * @security-severity 5.9
  * @precision low
  * @id java/hardcoded-credential-comparison
  * @tags security

java/ql/src/Security/CWE/CWE-798/HardcodedCredentialsSourceCall.ql

@@ -3,6 +3,7 @@
  * @description Using a hard-coded credential in a sensitive call may compromise security.
  * @kind path-problem
  * @problem.severity error
+ * @security-severity 5.9
  * @precision low
  * @id java/hardcoded-credential-sensitive-call
  * @tags security

java/ql/src/Security/CWE/CWE-798/HardcodedPasswordField.ql

@@ -3,6 +3,7 @@
  * @description Hard-coding a password string may compromise security.
  * @kind problem
  * @problem.severity error
+ * @security-severity 5.9
  * @precision low
  * @id java/hardcoded-password-field
  * @tags security

java/ql/src/Security/CWE/CWE-807/ConditionalBypass.ql

@@ -4,6 +4,7 @@
  *              passing through authentication systems.
  * @kind path-problem
  * @problem.severity error
+ * @security-severity 5.9
  * @precision medium
  * @id java/user-controlled-bypass
  * @tags security

java/ql/src/Security/CWE/CWE-807/TaintedPermissionsCheck.ql

@@ -4,6 +4,7 @@
  *              permissions being granted.
  * @kind path-problem
  * @problem.severity error
+ * @security-severity 5.9
  * @precision high
  * @id java/tainted-permissions-check
  * @tags security

java/ql/src/Security/CWE/CWE-829/InsecureDependencyResolution.ql

@@ -3,6 +3,7 @@
  * @description Non-HTTPS connections can be intercepted by third parties.
  * @kind problem
  * @problem.severity error
+ * @security-severity 5.9
  * @precision very-high
  * @id java/maven/non-https-url
  * @tags security

java/ql/src/Security/CWE/CWE-833/LockOrderInconsistency.ql

@@ -3,6 +3,7 @@
  * @description Acquiring multiple locks in a different order may cause deadlock.
  * @kind problem
  * @problem.severity recommendation
+ * @security-severity 6.9
  * @precision medium
  * @id java/lock-order-inconsistency
  * @tags security

java/ql/src/Security/CWE/CWE-835/InfiniteLoop.ql

@@ -5,6 +5,7 @@
  *              looping.
  * @kind problem
  * @problem.severity warning
+ * @security-severity 3.6
  * @precision medium
  * @id java/unreachable-exit-in-loop
  * @tags security

java/ql/src/Security/CWE/CWE-918/RequestForgery.qhelp

@@ -5,22 +5,24 @@
 
 
 <overview>
-<p>Directly incorporating user input into a HTTP request without validating the input
-can facilitate Server Side Request Forgery (SSRF) attacks. In these attacks, the server
-may be tricked into making a request and interacting with an attacker-controlled server. 
+<p>Directly incorporating user input into an HTTP request without validating the input
+can facilitate server-side request forgery (SSRF) attacks. In these attacks, the server
+may be tricked into making a request and interacting with an attacker-controlled server.
 </p>
 
 </overview>
 <recommendation>
 
-<p>To guard against SSRF attacks, it is advisable to avoid putting user input
-directly into the request URL. Instead, maintain a list of authorized
-URLs on the server; then choose from that list based on the user input provided.</p>
+<p>To guard against SSRF attacks, you should avoid putting user-provided input
+directly into a request URL. Instead, maintain a list of authorized
+URLs on the server; then choose from that list based on the input provided.
+Alternatively, ensure requests constructed from user input are limited to
+a particular host or more restrictive URL prefix.</p>
 
 </recommendation>
 <example>
 
-<p>The following example shows an HTTP request parameter being used directly in a forming a
+<p>The following example shows an HTTP request parameter being used directly to form a
 new request without validating the input, which facilitates SSRF attacks.
 It also shows how to remedy the problem by validating the user input against a known fixed string.
 </p>

java/ql/src/Security/CWE/CWE-918/RequestForgery.ql

@@ -0,0 +1,20 @@
+/**
+ * @name Server-side request forgery
+ * @description Making web requests based on unvalidated user-input
+ *              may cause the server to communicate with malicious servers.
+ * @kind path-problem
+ * @problem.severity error
+ * @precision high
+ * @id java/ssrf
+ * @tags security
+ *       external/cwe/cwe-918
+ */
+
+import java
+import semmle.code.java.security.RequestForgeryConfig
+import DataFlow::PathGraph
+
+from DataFlow::PathNode source, DataFlow::PathNode sink, RequestForgeryConfiguration conf
+where conf.hasFlowPath(source, sink)
+select sink.getNode(), source, sink, "Potential server-side request forgery due to $@.",
+  source.getNode(), "a user-provided value"

java/ql/src/experimental/Security/CWE/CWE-078/ExecCommon.qll

@@ -0,0 +1,32 @@
+import semmle.code.java.dataflow.FlowSources
+import semmle.code.java.security.ExternalProcess
+import semmle.code.java.security.CommandArguments
+
+private class RemoteUserInputToArgumentToExecFlowConfig extends TaintTracking::Configuration {
+  RemoteUserInputToArgumentToExecFlowConfig() {
+    this = "ExecCommon::RemoteUserInputToArgumentToExecFlowConfig"
+  }
+
+  override predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource }
+
+  override predicate isSink(DataFlow::Node sink) { sink.asExpr() instanceof ArgumentToExec }
+
+  override predicate isSanitizer(DataFlow::Node node) {
+    node.getType() instanceof PrimitiveType
+    or
+    node.getType() instanceof BoxedType
+    or
+    isSafeCommandArgument(node.asExpr())
+  }
+}
+
+/**
+ * Implementation of `ExecTainted.ql`. It is extracted to a QLL
+ * so that it can be excluded from `ExecUnescaped.ql` to avoid
+ * reporting overlapping results.
+ */
+predicate execTainted(DataFlow::PathNode source, DataFlow::PathNode sink, ArgumentToExec execArg) {
+  exists(RemoteUserInputToArgumentToExecFlowConfig conf |
+    conf.hasFlowPath(source, sink) and sink.getNode() = DataFlow::exprNode(execArg)
+  )
+}

java/ql/src/experimental/Security/CWE/CWE-078/ExecTainted.ql

@@ -0,0 +1,24 @@
+/**
+ * @name Uncontrolled command line
+ * @description Using externally controlled strings in a command line is vulnerable to malicious
+ *              changes in the strings.
+ * @kind path-problem
+ * @problem.severity error
+ * @precision high
+ * @id java/command-line-injection
+ * @tags security
+ *       external/cwe/cwe-078
+ *       external/cwe/cwe-088
+ */
+
+import java
+import semmle.code.java.dataflow.FlowSources
+import semmle.code.java.security.ExternalProcess
+import ExecCommon
+import JSchOSInjection
+import DataFlow::PathGraph
+
+from DataFlow::PathNode source, DataFlow::PathNode sink, ArgumentToExec execArg
+where execTainted(source, sink, execArg)
+select execArg, source, sink, "$@ flows to here and is used in a command.", source.getNode(),
+  "User-provided value"

java/ql/src/experimental/Security/CWE/CWE-078/JSchOSInjection.qll

@@ -0,0 +1,20 @@
+/**
+ * Provides classes for JSch OS command injection detection
+ */
+
+import java
+
+/** The class `com.jcraft.jsch.ChannelExec`. */
+private class JSchChannelExec extends RefType {
+  JSchChannelExec() { this.hasQualifiedName("com.jcraft.jsch", "ChannelExec") }
+}
+
+/** A method to set an OS Command for the execution. */
+private class ChannelExecSetCommandMethod extends Method, ExecCallable {
+  ChannelExecSetCommandMethod() {
+    this.hasName("setCommand") and
+    this.getDeclaringType() instanceof JSchChannelExec
+  }
+
+  override int getAnExecutedArgument() { result = 0 }
+}

java/ql/src/experimental/Security/CWE/CWE-078/JSchOSInjectionBad.java

@@ -0,0 +1,17 @@
+public class JSchOSInjectionBad {
+    void jschOsExecution(HttpServletRequest request) {
+        String command = request.getParameter("command");
+
+        JSch jsch = new JSch();
+        Session session = jsch.getSession("user", "sshHost", 22);
+        session.setPassword("password");
+        session.connect();
+
+        Channel channel = session.openChannel("exec");
+        // BAD - untrusted user data is used directly in a command
+        ((ChannelExec) channel).setCommand("ping " + command);
+        
+        channel.connect();
+    }
+}
+

java/ql/src/experimental/Security/CWE/CWE-078/JSchOSInjectionSanitized.java

@@ -0,0 +1,46 @@
+public class JSchOSInjectionSanitized {
+    void jschOsExecutionPing(HttpServletRequest request) {
+        String untrusted = request.getParameter("command");
+
+        //GOOD - Validate user the input.
+        if (!com.google.common.net.InetAddresses.isInetAddress(untrusted)) {
+            System.out.println("Invalid IP address");
+            return;
+        }
+
+        JSch jsch = new JSch();
+        Session session = jsch.getSession("user", "host", 22);
+        session.setPassword("password");
+        session.connect();
+
+        Channel channel = session.openChannel("exec");
+        ((ChannelExec) channel).setCommand("ping " + untrusted);
+
+        channel.connect();
+    }
+
+    void jschOsExecutionDig(HttpServletRequest request) {
+        String untrusted = request.getParameter("command");
+
+        //GOOD - check whether the user input doesn't contain dangerous shell characters.
+        String[] badChars = new String[] {"^", "~" ," " , "&", "|", ";", "$", ">", "<", "`", "\\", ",", "!", "{", "}", "(", ")", "@", "%", "#", "%0A", "%0a", "\n", "\r\n"};
+
+        for (String badChar : badChars) {
+            if (untrusted.contains(badChar)) {
+                System.out.println("Invalid host");
+                return;
+            }    
+        }
+
+        JSch jsch = new JSch();
+        Session session = jsch.getSession("user", "host", 22);
+        session.setPassword("password");
+        session.connect();
+
+        Channel channel = session.openChannel("exec");
+        ((ChannelExec) channel).setCommand("dig " + untrusted);
+
+        channel.connect();
+    }
+}
+

java/ql/src/experimental/Security/CWE/CWE-094/JexlInjection.ql

@@ -1,19 +0,0 @@
-/**
- * @name Expression language injection (JEXL)
- * @description Evaluation of a user-controlled JEXL expression
- *              may lead to arbitrary code execution.
- * @kind path-problem
- * @problem.severity error
- * @precision high
- * @id java/jexl-expression-injection
- * @tags security
- *       external/cwe/cwe-094
- */
-
-import java
-import JexlInjectionLib
-import DataFlow::PathGraph
-
-from DataFlow::PathNode source, DataFlow::PathNode sink, JexlInjectionConfig conf
-where conf.hasFlowPath(source, sink)
-select sink.getNode(), source, sink, "JEXL injection from $@.", source.getNode(), "this user input"

java/ql/src/experimental/Security/CWE/CWE-094/JexlInjectionLib.qll

@@ -1,277 +0,0 @@
-import java
-import FlowUtils
-import semmle.code.java.dataflow.FlowSources
-import semmle.code.java.dataflow.TaintTracking
-
-/**
- * A taint-tracking configuration for unsafe user input
- * that is used to construct and evaluate a JEXL expression.
- * It supports both JEXL 2 and 3.
- */
-class JexlInjectionConfig extends TaintTracking::Configuration {
-  JexlInjectionConfig() { this = "JexlInjectionConfig" }
-
-  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
-
-  override predicate isSink(DataFlow::Node sink) { sink instanceof JexlEvaluationSink }
-
-  override predicate isAdditionalTaintStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
-    any(TaintPropagatingJexlMethodCall c).taintFlow(fromNode, toNode) or
-    hasGetterFlow(fromNode, toNode)
-  }
-}
-
-/**
- * A sink for Expresssion Language injection vulnerabilities via Jexl,
- * i.e. method calls that run evaluation of a JEXL expression.
- *
- * Creating a `Callable` from a tainted JEXL expression or script is considered as a sink
- * although the tainted expression is not executed at this point.
- * Here we assume that it will get executed at some point,
- * maybe stored in an object field and then reached by a different flow.
- */
-private class JexlEvaluationSink extends DataFlow::ExprNode {
-  JexlEvaluationSink() {
-    exists(MethodAccess ma, Method m, Expr taintFrom |
-      ma.getMethod() = m and taintFrom = this.asExpr()
-    |
-      m instanceof DirectJexlEvaluationMethod and ma.getQualifier() = taintFrom
-      or
-      m instanceof CreateJexlCallableMethod and ma.getQualifier() = taintFrom
-      or
-      m instanceof JexlEngineGetSetPropertyMethod and
-      taintFrom.getType() instanceof TypeString and
-      ma.getAnArgument() = taintFrom
-    )
-  }
-}
-
-/**
- * Defines method calls that propagate tainted data via one of the methods
- * from JEXL library.
- */
-private class TaintPropagatingJexlMethodCall extends MethodAccess {
-  Expr taintFromExpr;
-
-  TaintPropagatingJexlMethodCall() {
-    exists(Method m, RefType taintType |
-      this.getMethod() = m and
-      taintType = taintFromExpr.getType()
-    |
-      isUnsafeEngine(this.getQualifier()) and
-      (
-        m instanceof CreateJexlScriptMethod and
-        taintFromExpr = this.getArgument(0) and
-        taintType instanceof TypeString
-        or
-        m instanceof CreateJexlExpressionMethod and
-        taintFromExpr = this.getAnArgument() and
-        taintType instanceof TypeString
-        or
-        m instanceof CreateJexlTemplateMethod and
-        (taintType instanceof TypeString or taintType instanceof Reader) and
-        taintFromExpr = this.getArgument([0, 1])
-      )
-    )
-  }
-
-  /**
-   * Holds if `fromNode` to `toNode` is a dataflow step that propagates
-   * tainted data.
-   */
-  predicate taintFlow(DataFlow::Node fromNode, DataFlow::Node toNode) {
-    fromNode.asExpr() = taintFromExpr and toNode.asExpr() = this
-  }
-}
-
-/**
- * Holds if `expr` is a JEXL engine that is not configured with a sandbox.
- */
-private predicate isUnsafeEngine(Expr expr) {
-  not exists(SandboxedJexlFlowConfig config | config.hasFlowTo(DataFlow::exprNode(expr)))
-}
-
-/**
- * A configuration for a tracking sandboxed JEXL engines.
- */
-private class SandboxedJexlFlowConfig extends DataFlow2::Configuration {
-  SandboxedJexlFlowConfig() { this = "JexlInjection::SandboxedJexlFlowConfig" }
-
-  override predicate isSource(DataFlow::Node node) { node instanceof SandboxedJexlSource }
-
-  override predicate isSink(DataFlow::Node node) {
-    exists(MethodAccess ma, Method m | ma.getMethod() = m |
-      (
-        m instanceof CreateJexlScriptMethod or
-        m instanceof CreateJexlExpressionMethod or
-        m instanceof CreateJexlTemplateMethod
-      ) and
-      ma.getQualifier() = node.asExpr()
-    )
-  }
-
-  override predicate isAdditionalFlowStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
-    createsJexlEngine(fromNode, toNode)
-  }
-}
-
-/**
- * Defines a data flow source for JEXL engines configured with a sandbox.
- */
-private class SandboxedJexlSource extends DataFlow::ExprNode {
-  SandboxedJexlSource() {
-    exists(MethodAccess ma, Method m | m = ma.getMethod() |
-      m.getDeclaringType() instanceof JexlBuilder and
-      m.hasName(["uberspect", "sandbox"]) and
-      m.getReturnType() instanceof JexlBuilder and
-      this.asExpr() = [ma, ma.getQualifier()]
-    )
-    or
-    exists(ConstructorCall cc |
-      cc.getConstructedType() instanceof JexlEngine and
-      cc.getArgument(0).getType() instanceof JexlUberspect and
-      cc = this.asExpr()
-    )
-  }
-}
-
-/**
- * Holds if `fromNode` to `toNode` is a dataflow step that creates one of the JEXL engines.
- */
-private predicate createsJexlEngine(DataFlow::Node fromNode, DataFlow::Node toNode) {
-  exists(MethodAccess ma, Method m | m = ma.getMethod() |
-    (m.getDeclaringType() instanceof JexlBuilder or m.getDeclaringType() instanceof JexlEngine) and
-    m.hasName(["create", "createJxltEngine"]) and
-    ma.getQualifier() = fromNode.asExpr() and
-    ma = toNode.asExpr()
-  )
-  or
-  exists(ConstructorCall cc |
-    cc.getConstructedType() instanceof UnifiedJexl and
-    cc.getArgument(0) = fromNode.asExpr() and
-    cc = toNode.asExpr()
-  )
-}
-
-/**
- * A methods in the `JexlEngine` class that gets or sets a property with a JEXL expression.
- */
-private class JexlEngineGetSetPropertyMethod extends Method {
-  JexlEngineGetSetPropertyMethod() {
-    getDeclaringType() instanceof JexlEngine and
-    hasName(["getProperty", "setProperty"])
-  }
-}
-
-/**
- * A method that triggers direct evaluation of JEXL expressions.
- */
-private class DirectJexlEvaluationMethod extends Method {
-  DirectJexlEvaluationMethod() {
-    getDeclaringType() instanceof JexlExpression and hasName("evaluate")
-    or
-    getDeclaringType() instanceof JexlScript and hasName("execute")
-    or
-    getDeclaringType() instanceof JxltEngineExpression and hasName(["evaluate", "prepare"])
-    or
-    getDeclaringType() instanceof JxltEngineTemplate and hasName("evaluate")
-    or
-    getDeclaringType() instanceof UnifiedJexlExpression and hasName(["evaluate", "prepare"])
-    or
-    getDeclaringType() instanceof UnifiedJexlTemplate and hasName("evaluate")
-  }
-}
-
-/**
- * A method that creates a JEXL script.
- */
-private class CreateJexlScriptMethod extends Method {
-  CreateJexlScriptMethod() { getDeclaringType() instanceof JexlEngine and hasName("createScript") }
-}
-
-/**
- * A method that creates a `Callable` for a JEXL expression or script.
- */
-private class CreateJexlCallableMethod extends Method {
-  CreateJexlCallableMethod() {
-    (getDeclaringType() instanceof JexlExpression or getDeclaringType() instanceof JexlScript) and
-    hasName("callable")
-  }
-}
-
-/**
- * A method that creates a JEXL template.
- */
-private class CreateJexlTemplateMethod extends Method {
-  CreateJexlTemplateMethod() {
-    (getDeclaringType() instanceof JxltEngine or getDeclaringType() instanceof UnifiedJexl) and
-    hasName("createTemplate")
-  }
-}
-
-/**
- * A method that creates a JEXL expression.
- */
-private class CreateJexlExpressionMethod extends Method {
-  CreateJexlExpressionMethod() {
-    (getDeclaringType() instanceof JexlEngine or getDeclaringType() instanceof JxltEngine) and
-    hasName("createExpression")
-    or
-    getDeclaringType() instanceof UnifiedJexl and hasName("parse")
-  }
-}
-
-private class JexlRefType extends RefType {
-  JexlRefType() { getPackage().hasName(["org.apache.commons.jexl2", "org.apache.commons.jexl3"]) }
-}
-
-private class JexlExpression extends JexlRefType {
-  JexlExpression() { hasName(["Expression", "JexlExpression"]) }
-}
-
-private class JexlScript extends JexlRefType {
-  JexlScript() { hasName(["Script", "JexlScript"]) }
-}
-
-private class JexlBuilder extends JexlRefType {
-  JexlBuilder() { hasName("JexlBuilder") }
-}
-
-private class JexlEngine extends JexlRefType {
-  JexlEngine() { hasName("JexlEngine") }
-}
-
-private class JxltEngine extends JexlRefType {
-  JxltEngine() { hasName("JxltEngine") }
-}
-
-private class UnifiedJexl extends JexlRefType {
-  UnifiedJexl() { hasName("UnifiedJEXL") }
-}
-
-private class JexlUberspect extends Interface {
-  JexlUberspect() {
-    hasQualifiedName("org.apache.commons.jexl2.introspection", "Uberspect") or
-    hasQualifiedName("org.apache.commons.jexl3.introspection", "JexlUberspect")
-  }
-}
-
-private class JxltEngineExpression extends NestedType {
-  JxltEngineExpression() { getEnclosingType() instanceof JxltEngine and hasName("Expression") }
-}
-
-private class JxltEngineTemplate extends NestedType {
-  JxltEngineTemplate() { getEnclosingType() instanceof JxltEngine and hasName("Template") }
-}
-
-private class UnifiedJexlExpression extends NestedType {
-  UnifiedJexlExpression() { getEnclosingType() instanceof UnifiedJexl and hasName("Expression") }
-}
-
-private class UnifiedJexlTemplate extends NestedType {
-  UnifiedJexlTemplate() { getEnclosingType() instanceof UnifiedJexl and hasName("Template") }
-}
-
-private class Reader extends RefType {
-  Reader() { hasQualifiedName("java.io", "Reader") }
-}

java/ql/src/experimental/Security/CWE/CWE-094/JythonInjection.java

@@ -0,0 +1,49 @@
+import org.python.util.PythonInterpreter;
+
+public class JythonInjection extends HttpServlet {
+    protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
+        response.setContentType("text/plain");
+        String code = request.getParameter("code");
+        PythonInterpreter interpreter = null;
+        ByteArrayOutputStream out = new ByteArrayOutputStream();
+
+        try {
+            interpreter = new PythonInterpreter();
+            interpreter.setOut(out);
+            interpreter.setErr(out);
+
+            // BAD: allow execution of arbitrary Python code
+            interpreter.exec(code);
+            out.flush();
+
+            response.getWriter().print(out.toString());
+        } catch(PyException ex) {
+            response.getWriter().println(ex.getMessage());
+        } finally {
+            if (interpreter != null) {
+                interpreter.close();
+            }
+            out.close();
+        }
+    }
+
+    protected void doPost(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
+        response.setContentType("text/plain");
+        String code = request.getParameter("code");
+        PythonInterpreter interpreter = null;
+
+        try {
+            interpreter = new PythonInterpreter();
+            // BAD: allow execution of arbitrary Python code
+            PyObject py = interpreter.eval(code);
+
+            response.getWriter().print(py.toString());
+        } catch(PyException ex) {
+            response.getWriter().println(ex.getMessage());
+        } finally {
+            if (interpreter != null) {
+                interpreter.close();
+            }
+        }
+    }
+}

java/ql/src/experimental/Security/CWE/CWE-094/JythonInjection.qhelp

@@ -0,0 +1,34 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>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.</p>
+</overview>
+
+<recommendation>
+<p>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.</p>
+</recommendation>
+
+<example>
+<p>The following code could execute arbitrary code in Jython Interpreter</p>
+<sample src="JythonInjection.java" />
+</example>
+
+<references>
+<li>
+  Jython Organization: <a href="https://jython.readthedocs.io/en/latest/JythonAndJavaIntegration/">Jython and Java Integration</a>
+</li>
+<li>
+  PortSwigger: <a href="https://portswigger.net/kb/issues/00100f10_python-code-injection">Python code injection</a>
+</li>
+</references>
+</qhelp>

java/ql/src/experimental/Security/CWE/CWE-094/JythonInjection.ql

@@ -0,0 +1,114 @@
+/**
+ * @name Injection in Jython
+ * @description Evaluation of a user-controlled malicious expression in Java Python
+ *              interpreter may lead to remote code execution.
+ * @kind path-problem
+ * @problem.severity error
+ * @precision high
+ * @id java/jython-injection
+ * @tags security
+ *       external/cwe/cwe-094
+ *       external/cwe/cwe-095
+ */
+
+import java
+import semmle.code.java.dataflow.FlowSources
+import semmle.code.java.frameworks.spring.SpringController
+import DataFlow::PathGraph
+
+/** The class `org.python.util.PythonInterpreter`. */
+class PythonInterpreter extends RefType {
+  PythonInterpreter() { this.hasQualifiedName("org.python.util", "PythonInterpreter") }
+}
+
+/** A method that evaluates, compiles or executes a Jython expression. */
+class InterpretExprMethod extends Method {
+  InterpretExprMethod() {
+    this.getDeclaringType().getAnAncestor*() instanceof PythonInterpreter and
+    getName().matches(["exec%", "run%", "eval", "compile"])
+  }
+}
+
+/** The class `org.python.core.BytecodeLoader`. */
+class BytecodeLoader extends RefType {
+  BytecodeLoader() { this.hasQualifiedName("org.python.core", "BytecodeLoader") }
+}
+
+/** Holds if a Jython expression if evaluated, compiled or executed. */
+predicate runsCode(MethodAccess ma, Expr sink) {
+  exists(Method m | m = ma.getMethod() |
+    m instanceof InterpretExprMethod and
+    sink = ma.getArgument(0)
+  )
+}
+
+/** A method that loads Java class data. */
+class LoadClassMethod extends Method {
+  LoadClassMethod() {
+    this.getDeclaringType().getAnAncestor*() instanceof BytecodeLoader and
+    hasName(["makeClass", "makeCode"])
+  }
+}
+
+/**
+ * Holds if `ma` is a call to a class-loading method, and `sink` is the byte array
+ * representing the class to be loaded.
+ */
+predicate loadsClass(MethodAccess ma, Expr sink) {
+  exists(Method m, int i | m = ma.getMethod() |
+    m instanceof LoadClassMethod and
+    m.getParameter(i).getType() instanceof Array and // makeClass(java.lang.String name, byte[] data, ...)
+    sink = ma.getArgument(i)
+  )
+}
+
+/** The class `org.python.core.Py`. */
+class Py extends RefType {
+  Py() { this.hasQualifiedName("org.python.core", "Py") }
+}
+
+/** A method declared on class `Py` or one of its descendants that compiles Python code. */
+class PyCompileMethod extends Method {
+  PyCompileMethod() {
+    this.getDeclaringType().getAnAncestor*() instanceof Py and
+    getName().matches("compile%")
+  }
+}
+
+/** Holds if source code is compiled with `PyCompileMethod`. */
+predicate compile(MethodAccess ma, Expr sink) {
+  exists(Method m | m = ma.getMethod() |
+    m instanceof PyCompileMethod and
+    sink = ma.getArgument(0)
+  )
+}
+
+/** An expression loaded by Jython. */
+class CodeInjectionSink extends DataFlow::ExprNode {
+  MethodAccess methodAccess;
+
+  CodeInjectionSink() {
+    runsCode(methodAccess, this.getExpr()) or
+    loadsClass(methodAccess, this.getExpr()) or
+    compile(methodAccess, this.getExpr())
+  }
+
+  MethodAccess getMethodAccess() { result = methodAccess }
+}
+
+/**
+ * A taint configuration for tracking flow from `RemoteFlowSource` to a Jython method call
+ * `CodeInjectionSink` that executes injected code.
+ */
+class CodeInjectionConfiguration extends TaintTracking::Configuration {
+  CodeInjectionConfiguration() { this = "CodeInjectionConfiguration" }
+
+  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
+
+  override predicate isSink(DataFlow::Node sink) { sink instanceof CodeInjectionSink }
+}
+
+from DataFlow::PathNode source, DataFlow::PathNode sink, CodeInjectionConfiguration conf
+where conf.hasFlowPath(source, sink)
+select sink.getNode().(CodeInjectionSink).getMethodAccess(), source, sink, "Jython evaluate $@.",
+  source.getNode(), "user input"