Merge branch 'github:main' into LoadClassNoSignatureCheck

java/ql/src/Diagnostics/DiagnosticsReporting.qll

@@ -79,12 +79,16 @@ predicate reportableErrors(Diagnostic d, string msg, int sev) {
  */
 predicate reportableWarnings(Diagnostic d, string msg, int sev) { knownWarnings(d, msg, sev) }
 
+/**
+ * Holds if compilation unit `f` is a source file.
+ */
+predicate extracted(CompilationUnit f) { exists(f.getRelativePath()) and f.fromSource() }
+
 /**
  * Holds if compilation unit `f` is a source file that has
  * no relevant extraction diagnostics associated with it.
  */
 predicate successfullyExtracted(CompilationUnit f) {
-  not exists(Diagnostic d | reportableDiagnostics(d, _, _) and d.getLocation().getFile() = f) and
-  exists(f.getRelativePath()) and
-  f.fromSource()
+  extracted(f) and
+  not exists(Diagnostic d | reportableDiagnostics(d, _, _) and d.getLocation().getFile() = f)
 }

java/ql/src/Diagnostics/SuccessfullyExtractedFiles.ql

@@ -1,7 +1,7 @@
 /**
- * @name Successfully extracted files
+ * @name Extracted files
  * @description A list of all files in the source code directory that
- *              were extracted without encountering an error in the file.
+ *              were extracted.
  * @kind diagnostic
  * @id java/diagnostics/successfully-extracted-files
  * @tags successfully-extracted-files
@@ -11,5 +11,5 @@ import java
 import DiagnosticsReporting
 
 from CompilationUnit f
-where successfullyExtracted(f)
+where extracted(f)
 select f, ""

java/ql/src/Language Abuse/DubiousTypeTestOfThis.ql

@@ -17,7 +17,7 @@ from InstanceOfExpr ioe, RefType t, RefType ct
 where
   ioe.getExpr() instanceof ThisAccess and
   t = ioe.getExpr().getType() and
-  ct = ioe.getCheckedType() and
+  ct = ioe.getSyntacticCheckedType() and
   ct.getAnAncestor() = t
 select ioe,
   "Testing whether 'this' is an instance of $@ in $@ introduces a dependency cycle between the two types.",

java/ql/src/Likely Bugs/Collections/ReadOnlyContainer.ql

@@ -40,6 +40,6 @@ where
   ) and
   // Also, any value that `v` is initialized to is a fresh container,
   forall(Expr e | e = v.getAnAssignedValue() | e instanceof FreshContainer) and
-  // and `v` is not implicitly initialized by a for-each loop.
-  not exists(EnhancedForStmt efs | efs.getVariable().getVariable() = v)
+  // and `v` is not implicitly initialized.
+  not v.(LocalVariableDecl).getDeclExpr().hasImplicitInit()
 select v, "The contents of this container are never initialized."

java/ql/src/Likely Bugs/Collections/WriteOnlyContainer.ql

@@ -39,6 +39,6 @@ where
   ) and
   // Also, any value that `v` is initialized to is a new container,
   forall(Expr e | e = v.getAnAssignedValue() | e instanceof ClassInstanceExpr) and
-  // and `v` is not implicitly initialized by a for-each loop.
-  not exists(EnhancedForStmt efs | efs.getVariable().getVariable() = v)
+  // and `v` is not implicitly initialized
+  not v.(LocalVariableDecl).getDeclExpr().hasImplicitInit()
 select v, "The contents of this container are never accessed."

java/ql/src/Likely Bugs/Comparison/EqualsUsesInstanceOf.ql

@@ -18,7 +18,7 @@ predicate instanceofInEquals(EqualsMethod m, InstanceOfExpr e) {
   e.getEnclosingCallable() = m and
   e.getExpr().(VarAccess).getVariable() = m.getParameter() and
   exists(RefType instanceofType |
-    instanceofType = e.getCheckedType() and
+    instanceofType = e.getSyntacticCheckedType() and
     not instanceofType.isFinal()
   )
 }

java/ql/src/Security/CWE/CWE-330/InsecureRandomness.qhelp

@@ -0,0 +1,64 @@
+<!DOCTYPE qhelp PUBLIC
+"-//Semmle//qhelp//EN"
+"qhelp.dtd">
+<qhelp>
+	<overview>
+		<p>
+			If you use a cryptographically weak pseudo-random number generator to generate security-sensitive values,
+			such as passwords, attackers can more easily predict those values.
+		</p>
+		<p>
+			Pseudo-random number generators generate a sequence of numbers that only approximates the properties
+			of random numbers. The sequence is not truly random because it is completely determined by a
+			relatively small set of initial values (the seed). If the random number generator is
+			cryptographically weak, then this sequence may be easily predictable through outside observations.
+		</p>
+
+	</overview>
+	<recommendation>
+        <p>
+           The <code>java.util.Random</code> random number generator is not cryptographically secure. Use a secure random number generator such as <code>java.security.SecureRandom</code> instead.
+        </p>
+        <p>
+		Use a cryptographically secure pseudo-random number generator if the output is to be used in a
+		security-sensitive context. As a general rule, a value should be considered "security-sensitive"
+		if predicting it would allow the attacker to perform an action that they would otherwise be unable
+		to perform. For example, if an attacker could predict the random password generated for a new user,
+		they would be able to log in as that new user.
+	</p>
+	</recommendation>
+
+	<example>
+
+		<p>
+			The following examples show different ways of generating a cookie with a random value.
+		</p>
+
+        <p>
+            In the first (BAD) case, we generate a fresh cookie by appending a random integer to the end of a static
+            string. The random number generator used (<code>Random</code>) is not cryptographically secure,
+            so it may be possible for an attacker to predict the generated cookie.
+        </p>
+
+        <sample src="examples/InsecureRandomnessCookie.java" />
+
+        <p>
+            In the second (GOOD) case, we generate a fresh cookie by appending a random integer to the end of a static
+            string. The random number generator used (<code>SecureRandom</code>) is cryptographically secure,
+            so it is not possible for an attacker to predict the generated cookie.
+        </p>
+
+        <sample src="examples/SecureRandomnessCookie.java" />
+
+	</example>
+
+	<references>
+		<li>Wikipedia: <a href="http://en.wikipedia.org/wiki/Pseudorandom_number_generator">Pseudo-random number generator</a>.</li>
+        <li>
+            Java Docs: <a href="http://docs.oracle.com/javase/8/docs/api/java/util/Random.html">Random</a>.
+        </li>
+        <li>
+            Java Docs: <a href="http://docs.oracle.com/javase/8/docs/api/java/security/SecureRandom.html">SecureRandom</a>.
+        </li>
+	</references>
+</qhelp>

java/ql/src/Security/CWE/CWE-330/InsecureRandomness.ql

@@ -0,0 +1,23 @@
+/**
+ * @name Insecure randomness
+ * @description Using a cryptographically Insecure pseudo-random number generator to generate a
+ *              security-sensitive value may allow an attacker to predict what value will
+ *              be generated.
+ * @kind path-problem
+ * @problem.severity warning
+ * @security-severity 7.8
+ * @precision high
+ * @id java/insecure-randomness
+ * @tags security
+ *      external/cwe/cwe-330
+ *      external/cwe/cwe-338
+ */
+
+import java
+import semmle.code.java.security.InsecureRandomnessQuery
+import InsecureRandomnessFlow::PathGraph
+
+from InsecureRandomnessFlow::PathNode source, InsecureRandomnessFlow::PathNode sink
+where InsecureRandomnessFlow::flowPath(source, sink)
+select sink.getNode(), source, sink, "Potential Insecure randomness due to a $@.", source.getNode(),
+  "Insecure randomness source."

java/ql/src/Security/CWE/CWE-330/examples/InsecureRandomnessCookie.java

@@ -0,0 +1,9 @@
+Random r = new Random();
+
+byte[] bytes = new byte[16];
+r.nextBytes(bytes);
+
+String cookieValue = encode(bytes);
+
+Cookie cookie = new Cookie("name", cookieValue);
+response.addCookie(cookie);

java/ql/src/Security/CWE/CWE-330/examples/SecureRandomnessCookie.java

@@ -0,0 +1,9 @@
+SecureRandom r = new SecureRandom();
+
+byte[] bytes = new byte[16];
+r.nextBytes(bytes);
+
+String cookieValue = encode(bytes);
+
+Cookie cookie = new Cookie("name", cookieValue);
+response.addCookie(cookie);

java/ql/src/Violations of Best Practice/Boxed Types/BoxedVariable.ql

@@ -63,7 +63,7 @@ from LocalBoxedVar v
 where
   forall(Expr e | e = v.getAnAssignedValue() | e.getType() = v.getPrimitiveType()) and
   (
-    not v.getDeclExpr().getParent() instanceof EnhancedForStmt or
+    not v.getDeclExpr().hasImplicitInit() or
     v.getDeclExpr().getParent().(EnhancedForStmt).getExpr().getType().(Array).getComponentType() =
       v.getPrimitiveType()
   ) and

java/ql/src/Violations of Best Practice/Dead Code/UnusedLocal.ql

@@ -11,22 +11,13 @@
 import java
 import DeadLocals
 
-predicate exceptionVariable(LocalVariableDeclExpr ve) {
-  exists(CatchClause catch | catch.getVariable() = ve)
-}
-
-predicate enhancedForVariable(LocalVariableDeclExpr ve) {
-  exists(EnhancedForStmt for | for.getVariable() = ve)
-}
-
 from LocalVariableDeclExpr ve, LocalVariableDecl v
 where
   v = ve.getVariable() and
   not assigned(v) and
   not read(v) and
   (not exists(ve.getInit()) or exprHasNoEffect(ve.getInit())) and
-  // Remove contexts where Java forces a variable declaration: enhanced-for and catch clauses.
+  // Remove contexts where Java forces a variable declaration: enhanced-for, catch clauses and pattern cases.
   // Rules about catch clauses belong in an exception handling query
-  not exceptionVariable(ve) and
-  not enhancedForVariable(ve)
+  not ve.hasImplicitInit()
 select v, "Variable " + v.getName() + " is not used."

java/ql/src/Violations of Best Practice/Declarations/BreakInSwitchCase.ql

@@ -17,8 +17,7 @@ import Common
 from SwitchStmt s, Stmt c
 where
   c = s.getACase() and
-  not c.(ControlFlowNode).getASuccessor() instanceof ConstCase and
-  not c.(ControlFlowNode).getASuccessor() instanceof DefaultCase and
+  not c.(ControlFlowNode).getASuccessor() instanceof SwitchCase and
   not s.(Annotatable).suppressesWarningsAbout("fallthrough") and
   mayDropThroughWithoutComment(s, c)
 select c,

java/ql/src/change-notes/2023-11-08-weak-randomness-query.md

@@ -0,0 +1,5 @@
+---
+category: newQuery
+---
+* Added the `java/insecure-randomness` query to detect uses of weakly random values which an attacker may be able to predict. Also added the `crypto-parameter` sink kind for sinks which represent the parameters and keys of cryptographic operations. 
+

java/ql/src/change-notes/2023-12-12-ognl-invalid-sinks.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Removed some spurious sinks related to `com.opensymphony.xwork2.TextProvider.getText` from the query `java/ognl-injection`.

java/ql/src/experimental/Security/CWE/CWE-073/FilePathInjection.ql

@@ -9,7 +9,7 @@
  * @id java/file-path-injection
  * @tags security
  *       experimental
- *       external/cwe-073
+ *       external/cwe/cwe-073
  */
 
 import java

java/ql/src/experimental/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql

@@ -24,7 +24,6 @@ import java
 import semmle.code.java.dataflow.FlowSteps
 import semmle.code.java.frameworks.Servlets
 import semmle.code.java.dataflow.TaintTracking
-import semmle.code.java.dataflow.TaintTracking2
 import MissingHttpOnlyFlow::PathGraph
 
 /** Gets a regular expression for matching common names of sensitive cookies. */

java/ql/src/experimental/Security/CWE/CWE-552/UnsafeUrlForward.ql

@@ -8,7 +8,7 @@
  * @id java/unsafe-url-forward-dispatch-load
  * @tags security
  *       experimental
- *       external/cwe-552
+ *       external/cwe/cwe-552
  */
 
 import java

java/ql/src/experimental/Security/CWE/CWE-625/PermissiveDotRegex.ql

@@ -8,8 +8,8 @@
  * @id java/permissive-dot-regex
  * @tags security
  *       experimental
- *       external/cwe-625
- *       external/cwe-863
+ *       external/cwe/cwe-625
+ *       external/cwe/cwe-863
  */
 
 import java