Merge branch 'main' into generate-more-value-preserving-summaries-2

java/ql/integration-tests/java/query-suite/java-code-quality.qls.expected

@@ -1,4 +1,3 @@
-ql/java/ql/src/Language Abuse/TypeVariableHidesType.ql
 ql/java/ql/src/Likely Bugs/Arithmetic/IntMultToLong.ql
 ql/java/ql/src/Likely Bugs/Collections/WriteOnlyContainer.ql
 ql/java/ql/src/Likely Bugs/Comparison/IncomparableEquals.ql

java/ql/src/Language Abuse/TypeVariableHidesType.ql

@@ -9,6 +9,7 @@
  * @tags reliability
  *       readability
  *       types
+ *       quality
  */
 
 import java

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

@@ -9,6 +9,7 @@
  * @tags reliability
  *       correctness
  *       types
+ *       quality
  *       external/cwe/cwe-190
  *       external/cwe/cwe-192
  *       external/cwe/cwe-197

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

@@ -7,6 +7,7 @@
  * @id java/unused-container
  * @tags maintainability
  *       useless-code
+ *       quality
  *       external/cwe/cwe-561
  */
 

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

@@ -8,6 +8,7 @@
  * @id java/equals-on-unrelated-types
  * @tags reliability
  *       correctness
+ *       quality
  */
 
 import java

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

@@ -8,6 +8,7 @@
  * @id java/inconsistent-equals-and-hashcode
  * @tags reliability
  *       correctness
+ *       quality
  *       external/cwe/cwe-581
  */
 

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

@@ -8,6 +8,7 @@
  * @id java/unchecked-cast-in-equals
  * @tags reliability
  *       correctness
+ *       quality
  */
 
 import java

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

@@ -8,6 +8,7 @@
  * @id java/reference-equality-of-boxed-types
  * @tags reliability
  *       correctness
+ *       quality
  *       external/cwe/cwe-595
  */
 

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

@@ -9,6 +9,7 @@
  * @id java/contradictory-type-checks
  * @tags correctness
  *       logic
+ *       quality
  */
 
 import java

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

@@ -6,6 +6,7 @@
  * @precision high
  * @id java/suspicious-date-format
  * @tags correctness
+ *       quality
  */
 
 import java

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

@@ -9,6 +9,7 @@
  * @tags efficiency
  *       correctness
  *       resources
+ *       quality
  *       external/cwe/cwe-404
  *       external/cwe/cwe-772
  */

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

@@ -9,6 +9,7 @@
  * @tags efficiency
  *       correctness
  *       resources
+ *       quality
  *       external/cwe/cwe-404
  *       external/cwe/cwe-772
  */

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

@@ -5,7 +5,7 @@
  *              to it.
  * @id java/count-untrusted-data-external-api
  * @kind table
- * @tags security external/cwe/cwe-20
+ * @tags security external/cwe/cwe-020
  */
 
 import java

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

@@ -6,7 +6,7 @@
  * @precision low
  * @problem.severity error
  * @security-severity 7.8
- * @tags security external/cwe/cwe-20
+ * @tags security external/cwe/cwe-020
  */
 
 import java

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

@@ -9,7 +9,7 @@
  * @precision high
  * @id java/netty-http-request-or-response-splitting
  * @tags security
- *       external/cwe/cwe-93
+ *       external/cwe/cwe-093
  *       external/cwe/cwe-113
  */
 

java/ql/src/change-notes/2025-05-01-cwe-tag-changed.md

@@ -0,0 +1,7 @@
+---
+category: queryMetadata
+---
+
+* The tag `external/cwe/cwe-20` has been removed from `java/count-untrusted-data-external-api` and the tag `external/cwe/cwe-020` has been added.
+* The tag `external/cwe/cwe-20` has been removed from `java/untrusted-data-to-external-api` and the tag `external/cwe/cwe-020` has been added.
+* The tag `external/cwe/cwe-93` has been removed from `java/netty-http-request-or-response-splitting` and the tag `external/cwe/cwe-093` has been added.

java/ql/src/codeql-suites/java-code-quality.qls

@@ -1,17 +1,3 @@
 - queries: .
-- include:
-    id:
-      - java/contradictory-type-checks
-      - java/do-not-call-finalize
-      - java/equals-on-unrelated-types
-      - java/inconsistent-equals-and-hashcode
-      - java/input-resource-leak
-      - java/integer-multiplication-cast-to-long
-      - java/junit5-missing-nested-annotation
-      - java/output-resource-leak
-      - java/reference-equality-of-boxed-types
-      - java/string-replace-all-with-non-regex
-      - java/suspicious-date-format
-      - java/type-variable-hides-type
-      - java/unchecked-cast-in-equals
-      - java/unused-container
+- apply: code-quality-selectors.yml
+  from: codeql/suite-helpers

java/ql/src/utils/modelgenerator/CaptureContentSummaryModels.ql

@@ -7,6 +7,7 @@
  */
 
 import internal.CaptureModels
+import SummaryModels
 
 from DataFlowSummaryTargetApi api, string flow
 where flow = ContentSensitive::captureFlow(api, _, _)

java/ql/src/utils/modelgenerator/CaptureNeutralModels.ql

@@ -7,6 +7,7 @@
  */
 
 import internal.CaptureModels
+import SummaryModels
 
 from DataFlowSummaryTargetApi api, string noflow
 where noflow = captureNeutral(api)

java/ql/src/utils/modelgenerator/CaptureSinkModels.ql

@@ -7,6 +7,7 @@
  */
 
 import internal.CaptureModels
+import SinkModels
 
 from DataFlowSinkTargetApi api, string sink
 where sink = Heuristic::captureSink(api)

java/ql/src/utils/modelgenerator/CaptureSourceModels.ql

@@ -7,6 +7,7 @@
  */
 
 import internal.CaptureModels
+import SourceModels
 
 from DataFlowSourceTargetApi api, string source
 where source = Heuristic::captureSource(api)

java/ql/src/utils/modelgenerator/CaptureSummaryModels.ql

@@ -7,6 +7,7 @@
  */
 
 import internal.CaptureModels
+import SummaryModels
 
 from DataFlowSummaryTargetApi api, string flow
 where flow = captureFlow(api, _)

java/ql/src/utils/modelgenerator/debug/CaptureSummaryModelsPartialPath.ql

@@ -11,6 +11,7 @@
 import java
 import semmle.code.java.dataflow.DataFlow
 import utils.modelgenerator.internal.CaptureModels
+import SummaryModels
 import PartialFlow::PartialPathGraph
 
 int explorationLimit() { result = 3 }

java/ql/src/utils/modelgenerator/debug/CaptureSummaryModelsPath.ql

@@ -11,6 +11,7 @@
 import java
 import semmle.code.java.dataflow.DataFlow
 import utils.modelgenerator.internal.CaptureModels
+import SummaryModels
 import Heuristic
 import PropagateTaintFlow::PathGraph
 

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

@@ -25,7 +25,20 @@ predicate isPrimitiveTypeUsedForBulkData(J::Type t) {
   t.hasName(["byte", "char", "Byte", "Character"])
 }
 
-module ModelGeneratorInput implements ModelGeneratorInputSig<Location, JavaDataFlow> {
+private predicate isInfrequentlyUsed(J::CompilationUnit cu) {
+  cu.getPackage().getName().matches("javax.swing%") or
+  cu.getPackage().getName().matches("java.awt%")
+}
+
+private predicate relevant(Callable api) {
+  api.isPublic() and
+  api.getDeclaringType().isPublic() and
+  api.fromSource() and
+  not isUninterestingForModels(api) and
+  not isInfrequentlyUsed(api.getCompilationUnit())
+}
+
+module ModelGeneratorCommonInput implements ModelGeneratorCommonInputSig<Location, JavaDataFlow> {
   class Type = J::Type;
 
   class Parameter = J::Parameter;
@@ -34,96 +47,8 @@ module ModelGeneratorInput implements ModelGeneratorInputSig<Location, JavaDataF
 
   class NodeExtended = DataFlow::Node;
 
-  Callable getAsExprEnclosingCallable(NodeExtended node) {
-    result = node.asExpr().getEnclosingCallable()
-  }
-
   Callable getEnclosingCallable(NodeExtended node) { result = node.getEnclosingCallable() }
 
-  Parameter asParameter(NodeExtended node) { result = node.asParameter() }
-
-  private predicate isInfrequentlyUsed(J::CompilationUnit cu) {
-    cu.getPackage().getName().matches("javax.swing%") or
-    cu.getPackage().getName().matches("java.awt%")
-  }
-
-  private predicate relevant(Callable api) {
-    api.isPublic() and
-    api.getDeclaringType().isPublic() and
-    api.fromSource() and
-    not isUninterestingForModels(api) and
-    not isInfrequentlyUsed(api.getCompilationUnit())
-  }
-
-  private J::Method getARelevantOverride(J::Method m) {
-    result = m.getAnOverride() and
-    relevant(result) and
-    // Other exclusions for overrides.
-    not m instanceof J::ToStringMethod
-  }
-
-  /**
-   * Gets the super implementation of `m` if it is relevant.
-   * If such a super implementations does not exist, returns `m` if it is relevant.
-   */
-  private J::Callable liftedImpl(J::Callable m) {
-    (
-      result = getARelevantOverride(m)
-      or
-      result = m and relevant(m)
-    ) and
-    not exists(getARelevantOverride(result))
-  }
-
-  private predicate hasManualSummaryModel(Callable api) {
-    api = any(FlowSummaryImpl::Public::SummarizedCallable sc | sc.applyManualModel()).asCallable() or
-    api = any(FlowSummaryImpl::Public::NeutralSummaryCallable sc | sc.hasManualModel()).asCallable()
-  }
-
-  private predicate hasManualSourceModel(Callable api) {
-    api = any(ExternalFlow::SourceCallable sc | sc.hasManualModel()) or
-    api = any(FlowSummaryImpl::Public::NeutralSourceCallable sc | sc.hasManualModel()).asCallable()
-  }
-
-  private predicate hasManualSinkModel(Callable api) {
-    api = any(ExternalFlow::SinkCallable sc | sc.hasManualModel()) or
-    api = any(FlowSummaryImpl::Public::NeutralSinkCallable sc | sc.hasManualModel()).asCallable()
-  }
-
-  predicate isUninterestingForDataFlowModels(Callable api) {
-    api.getDeclaringType() instanceof J::Interface and not exists(api.getBody())
-  }
-
-  predicate isUninterestingForHeuristicDataFlowModels(Callable api) { none() }
-
-  class SourceOrSinkTargetApi extends Callable {
-    SourceOrSinkTargetApi() { relevant(this) }
-  }
-
-  class SinkTargetApi extends SourceOrSinkTargetApi {
-    SinkTargetApi() { not hasManualSinkModel(this) }
-  }
-
-  class SourceTargetApi extends SourceOrSinkTargetApi {
-    SourceTargetApi() { not hasManualSourceModel(this) }
-  }
-
-  class SummaryTargetApi extends Callable {
-    private Callable lift;
-
-    SummaryTargetApi() {
-      lift = liftedImpl(this) and
-      not hasManualSummaryModel(lift)
-    }
-
-    Callable lift() { result = lift }
-
-    predicate isRelevant() {
-      relevant(this) and
-      not hasManualSummaryModel(this)
-    }
-  }
-
   private string isExtensible(Callable c) {
     if c.getDeclaringType().isFinal() then result = "false" else result = "true"
   }
@@ -204,50 +129,88 @@ module ModelGeneratorInput implements ModelGeneratorInputSig<Location, JavaDataF
     node.asExpr().(J::ThisAccess).isOwnInstanceAccess()
   }
 
-  predicate sinkModelSanitizer(DataFlow::Node node) {
-    // exclude variable capture jump steps
-    exists(Ssa::SsaImplicitInit closure |
-      closure.captures(_) and
-      node.asExpr() = closure.getAFirstUse()
-    )
-  }
-
-  predicate apiSource(DataFlow::Node source) {
-    (
-      source.asExpr().(J::FieldAccess).isOwnFieldAccess() or
-      source instanceof DataFlow::ParameterNode
-    ) and
-    exists(J::RefType t |
-      t = source.getEnclosingCallable().getDeclaringType().getAnAncestor() and
-      not t instanceof J::TypeObject and
-      t.isPublic()
-    )
-  }
-
-  predicate irrelevantSourceSinkApi(Callable source, SourceTargetApi api) { none() }
-
-  string getInputArgument(DataFlow::Node source) {
-    exists(int pos |
-      source.(DataFlow::ParameterNode).isParameterOf(_, pos) and
-      if pos >= 0 then result = "Argument[" + pos + "]" else result = qualifierString()
-    )
-    or
-    source.asExpr() instanceof J::FieldAccess and
-    result = qualifierString()
-  }
-
-  bindingset[kind]
-  predicate isRelevantSinkKind(string kind) {
-    not kind = "log-injection" and
-    not kind.matches("regex-use%") and
-    not kind = "file-content-store"
-  }
-
-  bindingset[kind]
-  predicate isRelevantSourceKind(string kind) { any() }
-
   predicate containerContent = DataFlowPrivate::containerContent/1;
 
+  string partialModelRow(Callable api, int i) {
+    i = 0 and qualifiedName(api, result, _) // package
+    or
+    i = 1 and qualifiedName(api, _, result) // type
+    or
+    i = 2 and result = isExtensible(api) // extensible
+    or
+    i = 3 and result = api.getName() // name
+    or
+    i = 4 and result = ExternalFlow::paramsString(api) // parameters
+    or
+    i = 5 and result = "" and exists(api) // ext
+  }
+
+  string partialNeutralModelRow(Callable api, int i) {
+    i = 0 and qualifiedName(api, result, _) // package
+    or
+    i = 1 and qualifiedName(api, _, result) // type
+    or
+    i = 2 and result = api.getName() // name
+    or
+    i = 3 and result = ExternalFlow::paramsString(api) // parameters
+  }
+}
+
+private import ModelGeneratorCommonInput
+private import MakeModelGeneratorFactory<Location, JavaDataFlow, JavaTaintTracking, ModelGeneratorCommonInput>
+
+module SummaryModelGeneratorInput implements SummaryModelGeneratorInputSig {
+  Callable getAsExprEnclosingCallable(NodeExtended node) {
+    result = node.asExpr().getEnclosingCallable()
+  }
+
+  Parameter asParameter(NodeExtended node) { result = node.asParameter() }
+
+  private J::Method getARelevantOverride(J::Method m) {
+    result = m.getAnOverride() and
+    relevant(result) and
+    // Other exclusions for overrides.
+    not m instanceof J::ToStringMethod
+  }
+
+  /**
+   * Gets the super implementation of `m` if it is relevant.
+   * If such a super implementations does not exist, returns `m` if it is relevant.
+   */
+  private J::Callable liftedImpl(J::Callable m) {
+    (
+      result = getARelevantOverride(m)
+      or
+      result = m and relevant(m)
+    ) and
+    not exists(getARelevantOverride(result))
+  }
+
+  private predicate hasManualSummaryModel(Callable api) {
+    api = any(FlowSummaryImpl::Public::SummarizedCallable sc | sc.applyManualModel()).asCallable() or
+    api = any(FlowSummaryImpl::Public::NeutralSummaryCallable sc | sc.hasManualModel()).asCallable()
+  }
+
+  class SummaryTargetApi extends Callable {
+    private Callable lift;
+
+    SummaryTargetApi() {
+      lift = liftedImpl(this) and
+      not hasManualSummaryModel(lift)
+    }
+
+    Callable lift() { result = lift }
+
+    predicate isRelevant() {
+      relevant(this) and
+      not hasManualSummaryModel(this)
+    }
+  }
+
+  predicate isUninterestingForDataFlowModels(Callable api) {
+    api.getDeclaringType() instanceof J::Interface and not exists(api.getBody())
+  }
+
   predicate isAdditionalContentFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
     TaintTracking::defaultAdditionalTaintStep(node1, node2, _) and
     not exists(DataFlow::Content f |
@@ -287,34 +250,71 @@ module ModelGeneratorInput implements ModelGeneratorInputSig<Location, JavaDataF
     or
     c instanceof DataFlowUtil::MapKeyContent and result = "MapKey"
   }
+}
 
-  string partialModelRow(Callable api, int i) {
-    i = 0 and qualifiedName(api, result, _) // package
-    or
-    i = 1 and qualifiedName(api, _, result) // type
-    or
-    i = 2 and result = isExtensible(api) // extensible
-    or
-    i = 3 and result = api.getName() // name
-    or
-    i = 4 and result = ExternalFlow::paramsString(api) // parameters
-    or
-    i = 5 and result = "" and exists(api) // ext
+private module SourceModelGeneratorInput implements SourceModelGeneratorInputSig {
+  private predicate hasManualSourceModel(Callable api) {
+    api = any(ExternalFlow::SourceCallable sc | sc.hasManualModel()) or
+    api = any(FlowSummaryImpl::Public::NeutralSourceCallable sc | sc.hasManualModel()).asCallable()
   }
 
-  string partialNeutralModelRow(Callable api, int i) {
-    i = 0 and qualifiedName(api, result, _) // package
-    or
-    i = 1 and qualifiedName(api, _, result) // type
-    or
-    i = 2 and result = api.getName() // name
-    or
-    i = 3 and result = ExternalFlow::paramsString(api) // parameters
+  class SourceTargetApi extends Callable {
+    SourceTargetApi() { relevant(this) and not hasManualSourceModel(this) }
   }
 
   predicate sourceNode = ExternalFlow::sourceNode/2;
+}
+
+private module SinkModelGeneratorInput implements SinkModelGeneratorInputSig {
+  private predicate hasManualSinkModel(Callable api) {
+    api = any(ExternalFlow::SinkCallable sc | sc.hasManualModel()) or
+    api = any(FlowSummaryImpl::Public::NeutralSinkCallable sc | sc.hasManualModel()).asCallable()
+  }
+
+  class SinkTargetApi extends Callable {
+    SinkTargetApi() { relevant(this) and not hasManualSinkModel(this) }
+  }
+
+  predicate sinkModelSanitizer(DataFlow::Node node) {
+    // exclude variable capture jump steps
+    exists(Ssa::SsaImplicitInit closure |
+      closure.captures(_) and
+      node.asExpr() = closure.getAFirstUse()
+    )
+  }
+
+  predicate apiSource(DataFlow::Node source) {
+    (
+      source.asExpr().(J::FieldAccess).isOwnFieldAccess() or
+      source instanceof DataFlow::ParameterNode
+    ) and
+    exists(J::RefType t |
+      t = source.getEnclosingCallable().getDeclaringType().getAnAncestor() and
+      not t instanceof J::TypeObject and
+      t.isPublic()
+    )
+  }
+
+  string getInputArgument(DataFlow::Node source) {
+    exists(int pos |
+      source.(DataFlow::ParameterNode).isParameterOf(_, pos) and
+      if pos >= 0 then result = "Argument[" + pos + "]" else result = qualifierString()
+    )
+    or
+    source.asExpr() instanceof J::FieldAccess and
+    result = qualifierString()
+  }
+
+  bindingset[kind]
+  predicate isRelevantSinkKind(string kind) {
+    not kind = "log-injection" and
+    not kind.matches("regex-use%") and
+    not kind = "file-content-store"
+  }
 
   predicate sinkNode = ExternalFlow::sinkNode/2;
 }
 
-import MakeModelGenerator<Location, JavaDataFlow, JavaTaintTracking, ModelGeneratorInput>
+import MakeSummaryModelGenerator<SummaryModelGeneratorInput> as SummaryModels
+import MakeSourceModelGenerator<SourceModelGeneratorInput> as SourceModels
+import MakeSinkModelGenerator<SinkModelGeneratorInput> as SinkModels

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

@@ -1,6 +1,6 @@
 private import java as J
 private import codeql.mad.modelgenerator.internal.ModelPrinting
-private import CaptureModels::ModelGeneratorInput as ModelGeneratorInput
+private import CaptureModels::ModelGeneratorCommonInput as ModelGeneratorInput
 
 private module ModelPrintingLang implements ModelPrintingLangSig {
   class Callable = J::Callable;

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

@@ -2,7 +2,8 @@ private import java
 private import semmle.code.java.Collections
 private import semmle.code.java.dataflow.internal.ContainerFlow
 private import CaptureModels as CaptureModels
-private import CaptureModels::ModelGeneratorInput as ModelGeneratorInput
+private import CaptureModels::ModelGeneratorCommonInput as ModelGeneratorInput
+private import CaptureModels::SummaryModelGeneratorInput as SummaryModelGeneratorInput
 private import CaptureModelsPrinting
 
 /**
@@ -284,21 +285,19 @@ private predicate output(Callable callable, TypeVariable tv, string output) {
   functionalSink(callable, tv, output)
 }
 
-module ModelPrintingInput implements ModelPrintingSig {
+module ModelPrintingInput implements ModelPrintingSummarySig {
   class SummaryApi = TypeBasedFlowTargetApi;
 
-  class SourceOrSinkApi = ModelGeneratorInput::SourceOrSinkTargetApi;
-
   string getProvenance() { result = "tb-generated" }
 }
 
-private module Printing = ModelPrinting<ModelPrintingInput>;
+private module Printing = ModelPrintingSummary<ModelPrintingInput>;
 
 /**
  * A class of callables that are relevant generating summaries for based
  * on the Theorems for Free approach.
  */
-class TypeBasedFlowTargetApi extends ModelGeneratorInput::SummaryTargetApi {
+class TypeBasedFlowTargetApi extends SummaryModelGeneratorInput::SummaryTargetApi {
   /**
    * Gets the string representation of all type based summaries for `this`
    * inspired by the Theorems for Free approach.

java/ql/test/utils/modelgenerator/dataflow/CaptureContentSummaryModels.ql

@@ -1,5 +1,6 @@
 import java
 import utils.modelgenerator.internal.CaptureModels
+import SummaryModels
 import utils.test.InlineMadTest
 
 module InlineMadTestConfig implements InlineMadTestConfigSig {

java/ql/test/utils/modelgenerator/dataflow/CaptureHeuristicSummaryModels.ql

@@ -1,5 +1,6 @@
 import java
 import utils.modelgenerator.internal.CaptureModels
+import SummaryModels
 import utils.test.InlineMadTest
 
 module InlineMadTestConfig implements InlineMadTestConfigSig {

java/ql/test/utils/modelgenerator/dataflow/CaptureNeutralModels.ql

@@ -1,5 +1,6 @@
 import java
 import utils.modelgenerator.internal.CaptureModels
+import SummaryModels
 import utils.test.InlineMadTest
 
 module InlineMadTestConfig implements InlineMadTestConfigSig {

java/ql/test/utils/modelgenerator/dataflow/CaptureSinkModels.ql

@@ -1,5 +1,6 @@
 import java
 import utils.modelgenerator.internal.CaptureModels
+import SinkModels
 import utils.test.InlineMadTest
 
 module InlineMadTestConfig implements InlineMadTestConfigSig {

java/ql/test/utils/modelgenerator/dataflow/CaptureSourceModels.ql

@@ -1,5 +1,6 @@
 import java
 import utils.modelgenerator.internal.CaptureModels
+import SourceModels
 import utils.test.InlineMadTest
 
 module InlineMadTestConfig implements InlineMadTestConfigSig {