Merge branch 'main' into java-kotlin-sensitive-logging-substring-barriers

java/ql/src/CHANGELOG.md

@@ -1,3 +1,10 @@
+## 1.10.0
+
+### Query Metadata Changes
+
+* Reduced the `security-severity` score of the `java/overly-large-range` query from 5.0 to 4.0 to better reflect its impact.
+* Reduced the `security-severity` score of the `java/insecure-cookie` query from 5.0 to 4.0 to better reflect its impact.
+
 ## 1.9.0
 
 ### New Queries

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

@@ -19,10 +19,10 @@ import semmle.code.java.dataflow.RangeUtils
 import semmle.code.java.dataflow.RangeAnalysis
 
 pragma[nomagic]
-predicate ssaArrayLengthBound(SsaVariable arr, Bound b) {
+predicate ssaArrayLengthBound(SsaDefinition arr, Bound b) {
   exists(FieldAccess len |
     len.getField() instanceof ArrayLengthField and
-    len.getQualifier() = arr.getAUse() and
+    len.getQualifier() = arr.getARead() and
     b.getExpr() = len
   )
 }
@@ -31,9 +31,9 @@ predicate ssaArrayLengthBound(SsaVariable arr, Bound b) {
  * Holds if the index expression of `aa` is less than or equal to the array length plus `k`.
  */
 predicate boundedArrayAccess(ArrayAccess aa, int k) {
-  exists(SsaVariable arr, Expr index, Bound b, int delta |
+  exists(SsaDefinition arr, Expr index, Bound b, int delta |
     aa.getIndexExpr() = index and
-    aa.getArray() = arr.getAUse() and
+    aa.getArray() = arr.getARead() and
     bounded(index, b, delta, true, _)
   |
     ssaArrayLengthBound(arr, b) and

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

@@ -127,7 +127,7 @@ Expr overFlowCand() {
     c.getIntValue() >= 0
   )
   or
-  exists(SsaExplicitUpdate x | result = x.getAUse() and x.getDefiningExpr() = overFlowCand())
+  exists(SsaExplicitWrite x | result = x.getARead() and x.getDefiningExpr() = overFlowCand())
   or
   result.(AssignExpr).getRhs() = overFlowCand()
   or
@@ -142,27 +142,27 @@ Expr overFlowCand() {
 predicate positiveOrNegative(Expr e) { positive(e) or negative(e) }
 
 /** Gets an expression that equals `v` plus a positive or negative value. */
-Expr increaseOrDecreaseOfVar(SsaVariable v) {
+Expr increaseOrDecreaseOfVar(SsaDefinition v) {
   exists(AssignAddExpr add |
     result = add and
     positiveOrNegative(add.getDest()) and
-    add.getRhs() = v.getAUse()
+    add.getRhs() = v.getARead()
   )
   or
   exists(AddExpr add, Expr e |
     result = add and
-    add.hasOperands(v.getAUse(), e) and
+    add.hasOperands(v.getARead(), e) and
     positiveOrNegative(e)
   )
   or
   exists(SubExpr sub |
     result = sub and
-    sub.getLeftOperand() = v.getAUse() and
+    sub.getLeftOperand() = v.getARead() and
     positiveOrNegative(sub.getRightOperand())
   )
   or
-  exists(SsaExplicitUpdate x |
-    result = x.getAUse() and x.getDefiningExpr() = increaseOrDecreaseOfVar(v)
+  exists(SsaExplicitWrite x |
+    result = x.getARead() and x.getDefiningExpr() = increaseOrDecreaseOfVar(v)
   )
   or
   result.(AssignExpr).getRhs() = increaseOrDecreaseOfVar(v)
@@ -172,7 +172,7 @@ Expr increaseOrDecreaseOfVar(SsaVariable v) {
 
 predicate overFlowTest(ComparisonExpr comp) {
   (
-    exists(SsaVariable v | comp.hasOperands(increaseOrDecreaseOfVar(v), v.getAUse()))
+    exists(SsaDefinition v | comp.hasOperands(increaseOrDecreaseOfVar(v), v.getARead()))
     or
     comp.getLesserOperand() = overFlowCand() and
     comp.getGreaterOperand().(IntegerLiteral).getIntValue() = 0
@@ -195,9 +195,9 @@ predicate concurrentModificationTest(BinaryExpr test) {
  */
 pragma[nomagic]
 predicate guardedTest(EqualityTest test, Guard guard, boolean isEq, int i1, int i2) {
-  exists(SsaVariable v, CompileTimeConstantExpr c1, CompileTimeConstantExpr c2 |
-    guard.isEquality(v.getAUse(), c1, isEq) and
-    test.hasOperands(v.getAUse(), c2) and
+  exists(SsaDefinition v, CompileTimeConstantExpr c1, CompileTimeConstantExpr c2 |
+    guard.isEquality(v.getARead(), c1, isEq) and
+    test.hasOperands(v.getARead(), c2) and
     i1 = c1.getIntValue() and
     i2 = c2.getIntValue() and
     v.getSourceVariable().getType() instanceof IntegralType

java/ql/src/Likely Bugs/Comparison/UselessComparisonTest.qll

@@ -27,14 +27,14 @@ class BoundKind extends string {
  */
 predicate uselessTest(ConditionNode s1, BinaryExpr test, boolean testIsTrue) {
   exists(
-    ConditionBlock cb, SsaVariable v, BinaryExpr cond, boolean condIsTrue, int k1, int k2,
+    ConditionBlock cb, SsaDefinition v, BinaryExpr cond, boolean condIsTrue, int k1, int k2,
     CompileTimeConstantExpr c1, CompileTimeConstantExpr c2
   |
     s1.getCondition() = cond and
     cb.getCondition() = cond and
-    cond.hasOperands(v.getAUse(), c1) and
+    cond.hasOperands(v.getARead(), c1) and
     c1.getIntValue() = k1 and
-    test.hasOperands(v.getAUse(), c2) and
+    test.hasOperands(v.getARead(), c2) and
     c2.getIntValue() = k2 and
     v.getSourceVariable().getVariable() instanceof LocalScopeVariable and
     cb.controls(test.getBasicBlock(), condIsTrue) and
@@ -49,7 +49,7 @@ predicate uselessTest(ConditionNode s1, BinaryExpr test, boolean testIsTrue) {
       )
       or
       exists(ComparisonExpr comp | comp = cond |
-        comp.getLesserOperand() = v.getAUse() and
+        comp.getLesserOperand() = v.getARead() and
         (
           condIsTrue = true and
           boundKind.isUpper() and
@@ -60,7 +60,7 @@ predicate uselessTest(ConditionNode s1, BinaryExpr test, boolean testIsTrue) {
           (if comp.isStrict() then bound = k1 else bound = k1 + 1)
         )
         or
-        comp.getGreaterOperand() = v.getAUse() and
+        comp.getGreaterOperand() = v.getARead() and
         (
           condIsTrue = true and
           boundKind.isLower() and
@@ -88,7 +88,7 @@ predicate uselessTest(ConditionNode s1, BinaryExpr test, boolean testIsTrue) {
       )
       or
       exists(ComparisonExpr comp | comp = test |
-        comp.getLesserOperand() = v.getAUse() and
+        comp.getLesserOperand() = v.getARead() and
         (
           boundKind.providesLowerBound() and
           testIsTrue = false and
@@ -107,7 +107,7 @@ predicate uselessTest(ConditionNode s1, BinaryExpr test, boolean testIsTrue) {
           )
         )
         or
-        comp.getGreaterOperand() = v.getAUse() and
+        comp.getGreaterOperand() = v.getARead() and
         (
           boundKind.providesLowerBound() and
           testIsTrue = true and

java/ql/src/Likely Bugs/Concurrency/DoubleCheckedLocking.qll

@@ -8,9 +8,9 @@ import semmle.code.java.dataflow.SSA
 private Expr getAFieldRead(Field f) {
   result = f.getAnAccess()
   or
-  exists(SsaExplicitUpdate v | v.getSourceVariable().getVariable() instanceof LocalScopeVariable |
-    result = v.getAUse() and
-    v.getDefiningExpr().(VariableAssign).getSource() = getAFieldRead(f)
+  exists(SsaExplicitWrite v | v.getSourceVariable().getVariable() instanceof LocalScopeVariable |
+    result = v.getARead() and
+    v.getValue() = getAFieldRead(f)
   )
   or
   result.(AssignExpr).getSource() = getAFieldRead(f)

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

@@ -52,10 +52,10 @@ predicate failedLock(LockType t, BasicBlock lockblock, BasicBlock exblock) {
     (
       lock.asExpr() = t.getLockAccess()
       or
-      exists(SsaExplicitUpdate lockbool |
+      exists(SsaExplicitWrite lockbool |
         // Using the value of `t.getLockAccess()` ensures that it is a `tryLock` call.
-        lock.asExpr() = lockbool.getAUse() and
-        lockbool.getDefiningExpr().(VariableAssign).getSource() = t.getLockAccess()
+        lock.asExpr() = lockbool.getARead() and
+        lockbool.getValue() = t.getLockAccess()
       )
     ) and
     (

java/ql/src/Likely Bugs/Likely Typos/ContradictoryTypeChecks.ql

@@ -37,11 +37,11 @@ predicate requiresInstanceOf(Expr e, VarAccess va, RefType t) {
  * `v` is not of type `sup`, which is a supertype of `t`.
  */
 predicate contradictoryTypeCheck(Expr e, Variable v, RefType t, RefType sup, Expr cond) {
-  exists(SsaVariable ssa |
+  exists(SsaDefinition ssa |
     ssa.getSourceVariable().getVariable() = v and
-    requiresInstanceOf(e, ssa.getAUse(), t) and
+    requiresInstanceOf(e, ssa.getARead(), t) and
     sup = t.getAnAncestor() and
-    instanceOfCheck(cond, ssa.getAUse(), sup) and
+    instanceOfCheck(cond, ssa.getARead(), sup) and
     cond.(Guard).controls(e.getBasicBlock(), false) and
     not t instanceof ErrorType and
     not sup instanceof ErrorType

java/ql/src/Likely Bugs/Termination/ConstantLoopCondition.ql

@@ -75,9 +75,9 @@ where
     loopWhileTrue(loop) and loopExitGuard(loop, cond)
   ) and
   // None of the ssa variables in `cond` are updated inside the loop.
-  forex(SsaVariable ssa, VarRead use | ssa.getAUse() = use and use.getParent*() = cond |
-    not ssa.getCfgNode().getEnclosingStmt().getEnclosingStmt*() = loop or
-    ssa.getCfgNode().asExpr().getParent*() = loop.(ForStmt).getAnInit()
+  forex(SsaDefinition ssa, VarRead use | ssa.getARead() = use and use.getParent*() = cond |
+    not ssa.getControlFlowNode().getEnclosingStmt().getEnclosingStmt*() = loop or
+    ssa.getControlFlowNode().asExpr().getParent*() = loop.(ForStmt).getAnInit()
   ) and
   // And `cond` does not use method calls, field reads, or array reads.
   not exists(MethodCall ma | ma.getParent*() = cond) and

java/ql/src/Violations of Best Practice/Dead Code/DeadLocals.qll

@@ -14,7 +14,7 @@ private predicate emptyDecl(LocalVariableDeclExpr decl) {
 /** A dead variable update. */
 predicate deadLocal(VariableUpdate upd) {
   upd.getDestVar() instanceof LocalScopeVariable and
-  not exists(SsaExplicitUpdate ssa | upd = ssa.getDefiningExpr()) and
+  not exists(SsaExplicitWrite ssa | upd = ssa.getDefiningExpr()) and
   not emptyDecl(upd) and
   not readImplicitly(upd, _)
 }

java/ql/src/change-notes/released/1.10.0.md

@@ -1,5 +1,6 @@
----
-category: queryMetadata
----
+## 1.10.0
+
+### Query Metadata Changes
+
 * Reduced the `security-severity` score of the `java/overly-large-range` query from 5.0 to 4.0 to better reflect its impact.
-* Reduced the `security-severity` score of the `java/insecure-cookie` query from 5.0 to 4.0 to better reflect its impact.
+* Reduced the `security-severity` score of the `java/insecure-cookie` query from 5.0 to 4.0 to better reflect its impact.

java/ql/src/codeql-pack.release.yml

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 1.9.0
+lastReleaseVersion: 1.10.0

java/ql/src/experimental/Security/CWE/CWE-470/LoadClassNoSignatureCheck.ql

@@ -42,9 +42,9 @@ class CheckSignaturesGuard extends Guard instanceof EqualityTest {
 }
 
 predicate signatureChecked(Expr safe) {
-  exists(CheckSignaturesGuard g, SsaVariable v |
-    v.getAUse() = g.getCheckedExpr() and
-    safe = v.getAUse() and
+  exists(CheckSignaturesGuard g, SsaDefinition v |
+    v.getARead() = g.getCheckedExpr() and
+    safe = v.getARead() and
     g.controls(safe.getBasicBlock(), g.(EqualityTest).polarity())
   )
 }

java/ql/src/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/java-queries
-version: 1.9.1-dev
+version: 1.10.1-dev
 groups:
   - java
   - queries

java/ql/src/utils/modelgenerator/internal/CaptureModels.qll

@@ -277,10 +277,7 @@ private module SinkModelGeneratorInput implements SinkModelGeneratorInputSig {
 
   predicate sinkModelSanitizer(DataFlow::Node node) {
     // exclude variable capture jump steps
-    exists(Ssa::SsaImplicitInit closure |
-      closure.captures(_) and
-      node.asExpr() = closure.getAFirstUse()
-    )
+    exists(Ssa::SsaCapturedDefinition closure | node.asExpr() = Ssa::ssaGetAFirstUse(closure))
   }
 
   predicate apiSource(DataFlow::Node source) {