Java models-as-data: infer Kotlin $default models from that of its parent function

2025-12-24 04:36:35 +01:00 · 2022-10-18 18:15:35 +01:00
parent 73f977c98c
commit b148e3168f
7 changed files with 314 additions and 12 deletions
--- a/java/ql/integration-tests/posix-only/kotlin/default-parameter-mad-flow/lib.kt
+++ b/java/ql/integration-tests/posix-only/kotlin/default-parameter-mad-flow/lib.kt
@@ -0,0 +1,33 @@
+class ConstructorWithDefaults(x: Int, y: Int = 1) { }
+
+fun topLevelWithDefaults(x: Int, y: Int = 1) = 0
+fun String.extensionWithDefaults(x: Int, y: Int = 1) = 0
+
+class LibClass {
+
+  fun memberWithDefaults(x: Int, y: Int = 1) = 0
+  fun String.extensionMemberWithDefaults(x: Int, y: Int = 1) = 0
+
+}
+
+class SomeToken {}
+
+fun topLevelArgSource(st: SomeToken, x: Int = 0) {}
+fun String.extensionArgSource(st: SomeToken, x: Int = 0) {}
+
+class SourceClass {
+
+  fun memberArgSource(st: SomeToken, x: Int = 0) {}
+
+}
+
+fun topLevelSink(x: Int, y: Int = 1) {}
+fun String.extensionSink(x: Int, y: Int = 1) {}
+
+class SinkClass(x: Int, y: Int = 1) {
+
+  fun memberSink(x: Int, y: Int = 1) {}
+  fun String.extensionMemberSink(x: Int, y: Int = 1) {}
+
+}
+
--- a/java/ql/integration-tests/posix-only/kotlin/default-parameter-mad-flow/test.expected
+++ b/java/ql/integration-tests/posix-only/kotlin/default-parameter-mad-flow/test.expected
@@ -0,0 +1,66 @@
+edges
+| user.kt:7:32:7:39 | source(...) : Number | user.kt:7:8:7:43 | new ConstructorWithDefaults(...) |
+| user.kt:8:32:8:39 | source(...) : Number | user.kt:8:8:8:40 | new ConstructorWithDefaults(...) |
+| user.kt:10:29:10:36 | source(...) : Number | user.kt:10:8:10:40 | topLevelWithDefaults(...) |
+| user.kt:11:29:11:36 | source(...) : Number | user.kt:11:8:11:37 | topLevelWithDefaults$default(...) |
+| user.kt:13:44:13:51 | source(...) : Number | user.kt:13:22:13:55 | extensionWithDefaults(...) |
+| user.kt:14:44:14:51 | source(...) : Number | user.kt:14:22:14:52 | extensionWithDefaults$default(...) |
+| user.kt:16:29:16:36 | source(...) : Number | user.kt:16:10:16:40 | memberWithDefaults(...) |
+| user.kt:17:29:17:36 | source(...) : Number | user.kt:17:10:17:37 | memberWithDefaults$default(...) |
+| user.kt:20:52:20:59 | source(...) : Number | user.kt:20:24:20:63 | extensionMemberWithDefaults(...) |
+| user.kt:21:52:21:59 | source(...) : Number | user.kt:21:24:21:60 | extensionMemberWithDefaults$default(...) |
+| user.kt:26:23:26:24 | st [post update] : SomeToken | user.kt:27:10:27:11 | st |
+| user.kt:32:38:32:39 | st [post update] : SomeToken | user.kt:33:10:33:11 | st |
+| user.kt:38:29:38:30 | st [post update] : SomeToken | user.kt:39:10:39:11 | st |
+nodes
+| user.kt:7:8:7:43 | new ConstructorWithDefaults(...) | semmle.label | new ConstructorWithDefaults(...) |
+| user.kt:7:32:7:39 | source(...) : Number | semmle.label | source(...) : Number |
+| user.kt:8:8:8:40 | new ConstructorWithDefaults(...) | semmle.label | new ConstructorWithDefaults(...) |
+| user.kt:8:32:8:39 | source(...) : Number | semmle.label | source(...) : Number |
+| user.kt:10:8:10:40 | topLevelWithDefaults(...) | semmle.label | topLevelWithDefaults(...) |
+| user.kt:10:29:10:36 | source(...) : Number | semmle.label | source(...) : Number |
+| user.kt:11:8:11:37 | topLevelWithDefaults$default(...) | semmle.label | topLevelWithDefaults$default(...) |
+| user.kt:11:29:11:36 | source(...) : Number | semmle.label | source(...) : Number |
+| user.kt:13:22:13:55 | extensionWithDefaults(...) | semmle.label | extensionWithDefaults(...) |
+| user.kt:13:44:13:51 | source(...) : Number | semmle.label | source(...) : Number |
+| user.kt:14:22:14:52 | extensionWithDefaults$default(...) | semmle.label | extensionWithDefaults$default(...) |
+| user.kt:14:44:14:51 | source(...) : Number | semmle.label | source(...) : Number |
+| user.kt:16:10:16:40 | memberWithDefaults(...) | semmle.label | memberWithDefaults(...) |
+| user.kt:16:29:16:36 | source(...) : Number | semmle.label | source(...) : Number |
+| user.kt:17:10:17:37 | memberWithDefaults$default(...) | semmle.label | memberWithDefaults$default(...) |
+| user.kt:17:29:17:36 | source(...) : Number | semmle.label | source(...) : Number |
+| user.kt:20:24:20:63 | extensionMemberWithDefaults(...) | semmle.label | extensionMemberWithDefaults(...) |
+| user.kt:20:52:20:59 | source(...) : Number | semmle.label | source(...) : Number |
+| user.kt:21:24:21:60 | extensionMemberWithDefaults$default(...) | semmle.label | extensionMemberWithDefaults$default(...) |
+| user.kt:21:52:21:59 | source(...) : Number | semmle.label | source(...) : Number |
+| user.kt:26:23:26:24 | st [post update] : SomeToken | semmle.label | st [post update] : SomeToken |
+| user.kt:27:10:27:11 | st | semmle.label | st |
+| user.kt:32:38:32:39 | st [post update] : SomeToken | semmle.label | st [post update] : SomeToken |
+| user.kt:33:10:33:11 | st | semmle.label | st |
+| user.kt:38:29:38:30 | st [post update] : SomeToken | semmle.label | st [post update] : SomeToken |
+| user.kt:39:10:39:11 | st | semmle.label | st |
+| user.kt:42:13:42:20 | source(...) | semmle.label | source(...) |
+| user.kt:43:16:43:23 | source(...) | semmle.label | source(...) |
+| user.kt:44:31:44:38 | source(...) | semmle.label | source(...) |
+| user.kt:45:20:45:27 | source(...) | semmle.label | source(...) |
+| user.kt:47:39:47:46 | source(...) | semmle.label | source(...) |
+subpaths
+#select
+| user.kt:7:32:7:39 | source(...) : Number | user.kt:7:32:7:39 | source(...) : Number | user.kt:7:8:7:43 | new ConstructorWithDefaults(...) | flow path |
+| user.kt:8:32:8:39 | source(...) : Number | user.kt:8:32:8:39 | source(...) : Number | user.kt:8:8:8:40 | new ConstructorWithDefaults(...) | flow path |
+| user.kt:10:29:10:36 | source(...) : Number | user.kt:10:29:10:36 | source(...) : Number | user.kt:10:8:10:40 | topLevelWithDefaults(...) | flow path |
+| user.kt:11:29:11:36 | source(...) : Number | user.kt:11:29:11:36 | source(...) : Number | user.kt:11:8:11:37 | topLevelWithDefaults$default(...) | flow path |
+| user.kt:13:44:13:51 | source(...) : Number | user.kt:13:44:13:51 | source(...) : Number | user.kt:13:22:13:55 | extensionWithDefaults(...) | flow path |
+| user.kt:14:44:14:51 | source(...) : Number | user.kt:14:44:14:51 | source(...) : Number | user.kt:14:22:14:52 | extensionWithDefaults$default(...) | flow path |
+| user.kt:16:29:16:36 | source(...) : Number | user.kt:16:29:16:36 | source(...) : Number | user.kt:16:10:16:40 | memberWithDefaults(...) | flow path |
+| user.kt:17:29:17:36 | source(...) : Number | user.kt:17:29:17:36 | source(...) : Number | user.kt:17:10:17:37 | memberWithDefaults$default(...) | flow path |
+| user.kt:20:52:20:59 | source(...) : Number | user.kt:20:52:20:59 | source(...) : Number | user.kt:20:24:20:63 | extensionMemberWithDefaults(...) | flow path |
+| user.kt:21:52:21:59 | source(...) : Number | user.kt:21:52:21:59 | source(...) : Number | user.kt:21:24:21:60 | extensionMemberWithDefaults$default(...) | flow path |
+| user.kt:26:23:26:24 | st [post update] : SomeToken | user.kt:26:23:26:24 | st [post update] : SomeToken | user.kt:27:10:27:11 | st | flow path |
+| user.kt:32:38:32:39 | st [post update] : SomeToken | user.kt:32:38:32:39 | st [post update] : SomeToken | user.kt:33:10:33:11 | st | flow path |
+| user.kt:38:29:38:30 | st [post update] : SomeToken | user.kt:38:29:38:30 | st [post update] : SomeToken | user.kt:39:10:39:11 | st | flow path |
+| user.kt:42:13:42:20 | source(...) | user.kt:42:13:42:20 | source(...) | user.kt:42:13:42:20 | source(...) | flow path |
+| user.kt:43:16:43:23 | source(...) | user.kt:43:16:43:23 | source(...) | user.kt:43:16:43:23 | source(...) | flow path |
+| user.kt:44:31:44:38 | source(...) | user.kt:44:31:44:38 | source(...) | user.kt:44:31:44:38 | source(...) | flow path |
+| user.kt:45:20:45:27 | source(...) | user.kt:45:20:45:27 | source(...) | user.kt:45:20:45:27 | source(...) | flow path |
+| user.kt:47:39:47:46 | source(...) | user.kt:47:39:47:46 | source(...) | user.kt:47:39:47:46 | source(...) | flow path |
--- a/java/ql/integration-tests/posix-only/kotlin/default-parameter-mad-flow/test.py
+++ b/java/ql/integration-tests/posix-only/kotlin/default-parameter-mad-flow/test.py
@@ -0,0 +1,5 @@
+from create_database_utils import *
+import subprocess
+
+subprocess.check_call(["kotlinc", "lib.kt", "-d", "lib"])
+run_codeql_database_create(["kotlinc user.kt -cp lib"], lang="java")
--- a/java/ql/integration-tests/posix-only/kotlin/default-parameter-mad-flow/test.ql
+++ b/java/ql/integration-tests/posix-only/kotlin/default-parameter-mad-flow/test.ql
@@ -0,0 +1,61 @@
+import java
+import semmle.code.java.dataflow.TaintTracking
+import DataFlow::PathGraph
+private import semmle.code.java.dataflow.ExternalFlow
+
+private class Models extends SummaryModelCsv {
+  override predicate row(string row) {
+    row =
+      [
+        ";ConstructorWithDefaults;true;ConstructorWithDefaults;(int,int);;Argument[0];Argument[-1];taint;manual",
+        ";LibKt;true;topLevelWithDefaults;(int,int);;Argument[0];ReturnValue;value;manual",
+        ";LibKt;true;extensionWithDefaults;(String,int,int);;Argument[1];ReturnValue;value;manual",
+        ";LibClass;true;memberWithDefaults;(int,int);;Argument[0];ReturnValue;value;manual",
+        ";LibClass;true;extensionMemberWithDefaults;(String,int,int);;Argument[1];ReturnValue;value;manual"
+      ]
+  }
+}
+
+private class SourceModels extends SourceModelCsv {
+  override predicate row(string row) {
+    row =
+      [
+        ";LibKt;true;topLevelArgSource;(SomeToken,int);;Argument[0];kotlinMadFlowTest;manual",
+        ";LibKt;true;extensionArgSource;(String,SomeToken,int);;Argument[1];kotlinMadFlowTest;manual",
+        ";SourceClass;true;memberArgSource;(SomeToken,int);;Argument[0];kotlinMadFlowTest;manual"
+      ]
+  }
+}
+
+private class SinkModels extends SinkModelCsv {
+  override predicate row(string row) {
+    row =
+      [
+        ";SinkClass;true;SinkClass;(int,int);;Argument[0];kotlinMadFlowTest;manual",
+        ";LibKt;true;topLevelSink;(int,int);;Argument[0];kotlinMadFlowTest;manual",
+        ";LibKt;true;extensionSink;(String,int,int);;Argument[1];kotlinMadFlowTest;manual",
+        ";SinkClass;true;memberSink;(int,int);;Argument[0];kotlinMadFlowTest;manual",
+        ";SinkClass;true;extensionMemberSink;(String,int,int);;Argument[1];kotlinMadFlowTest;manual"
+      ]
+  }
+}
+
+class Config extends TaintTracking::Configuration {
+  Config() { this = "Config" }
+
+  override predicate isSource(DataFlow::Node n) {
+    n.asExpr().(MethodAccess).getCallee().getName() = "source"
+    or
+    sourceNode(n, "kotlinMadFlowTest")
+  }
+
+  override predicate isSink(DataFlow::Node n) {
+    n.asExpr().(Argument).getCall().getCallee().getName() = "sink"
+    or
+    sinkNode(n, "kotlinMadFlowTest")
+  }
+}
+
+from DataFlow::PathNode source, DataFlow::PathNode sink, Config c
+where c.hasFlowPath(source, sink)
+select source, source, sink, "flow path"
--- a/java/ql/integration-tests/posix-only/kotlin/default-parameter-mad-flow/user.kt
+++ b/java/ql/integration-tests/posix-only/kotlin/default-parameter-mad-flow/user.kt
@@ -0,0 +1,50 @@
+fun source() = 1
+
+fun sink(x: Any) { }
+
+fun test(c: LibClass, sourcec: SourceClass, sinkc: SinkClass) {
+
+  sink(ConstructorWithDefaults(source(), 0))
+  sink(ConstructorWithDefaults(source()))
+
+  sink(topLevelWithDefaults(source(), 0))
+  sink(topLevelWithDefaults(source()))
+
+  sink("Hello world".extensionWithDefaults(source(), 0))
+  sink("Hello world".extensionWithDefaults(source()))
+
+  sink(c.memberWithDefaults(source(), 0))
+  sink(c.memberWithDefaults(source()))
+
+  with(c) {
+    sink("Hello world".extensionMemberWithDefaults(source(), 0))
+    sink("Hello world".extensionMemberWithDefaults(source()))
+  };
+
+  run {
+    val st = SomeToken()
+    topLevelArgSource(st)
+    sink(st)
+  }
+
+  run {
+    val st = SomeToken()
+    "Hello world".extensionArgSource(st)
+    sink(st)
+  }
+
+  run {
+    val st = SomeToken()
+    sourcec.memberArgSource(st)
+    sink(st)
+  }
+
+  SinkClass(source())
+  topLevelSink(source())
+  "Hello world".extensionSink(source())
+  sinkc.memberSink(source())
+  with(sinkc) {
+    "Hello world".extensionMemberSink(source())
+  }
+
+}
--- a/java/ql/lib/semmle/code/java/Member.qll
+++ b/java/ql/lib/semmle/code/java/Member.qll
@@ -306,7 +306,9 @@ class Callable extends StmtParent, Member, @callable {
   */
  Callable getKotlinParameterDefaultsProxy() {
    this.getDeclaringType() = result.getDeclaringType() and
-    exists(int proxyNParams, int extraLeadingParams, RefType lastParamType |
+    exists(
+      int proxyNParams, int extraLeadingParams, int regularParamsStartIdx, RefType lastParamType
+    |
      proxyNParams = result.getNumberOfParameters() and
      extraLeadingParams = (proxyNParams - this.getNumberOfParameters()) - 2 and
      extraLeadingParams >= 0 and
@@ -316,16 +318,28 @@ class Callable extends StmtParent, Member, @callable {
        this instanceof Constructor and
        result instanceof Constructor and
        extraLeadingParams = 0 and
+        regularParamsStartIdx = 0 and
        lastParamType.hasQualifiedName("kotlin.jvm.internal", "DefaultConstructorMarker")
        or
        this instanceof Method and
        result instanceof Method and
        this.getName() + "$default" = result.getName() and
-        extraLeadingParams <= 2 and
+        extraLeadingParams <= 1 and
+        (
+          if ktExtensionFunctions(this, _, _)
+          then
+            // Both extension receivers are expected to occur at arg0, with any
+            // dispatch receiver inserted afterwards in the $default proxy's parameter list.
+            // Check the extension receiver matches here, and note regular args
+            // are bumped one position to the right.
+            regularParamsStartIdx = extraLeadingParams + 1 and
+            this.getParameterType(0).getErasure() = eraseRaw(result.getParameterType(0))
+          else regularParamsStartIdx = extraLeadingParams
+        ) and
        lastParamType instanceof TypeObject
      )
    |
-      forall(int paramIdx | paramIdx in [extraLeadingParams .. proxyNParams - 3] |
+      forall(int paramIdx | paramIdx in [regularParamsStartIdx .. proxyNParams - 3] |
        this.getParameterType(paramIdx - extraLeadingParams).getErasure() =
          eraseRaw(result.getParameterType(paramIdx))
      )
--- a/java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImplSpecific.qll
+++ b/java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImplSpecific.qll
@@ -65,6 +65,61 @@ private boolean isGenerated(string provenance) {
  provenance != "generated" and result = false
 }

+private predicate relatedArgSpec(Callable c, string spec) {
+  exists(
+    string namespace, string type, boolean subtypes, string name, string signature, string ext
+  |
+    summaryModel(namespace, type, subtypes, name, signature, ext, spec, _, _, _) or
+    summaryModel(namespace, type, subtypes, name, signature, ext, _, spec, _, _) or
+    sourceModel(namespace, type, subtypes, name, signature, ext, spec, _, _) or
+    sinkModel(namespace, type, subtypes, name, signature, ext, spec, _, _)
+  |
+    c = interpretElement(namespace, type, subtypes, name, signature, ext)
+  )
+}
+
+/**
+ * Holds if `defaultsCallable` is a Kotlin default-parameter proxy for `originalCallable`, and
+ * `originalCallable` has a model, and `defaultsArgSpec` is `originalArgSpec` adjusted to account
+ * for the additional dispatch receiver parameter that occurs in the default-parameter proxy's argument
+ * list. When no adjustment is required (e.g. for constructors, or non-argument-based specs), `defaultArgsSpec`
+ * equals `originalArgSpec`.
+ */
+private predicate correspondingKotlinParameterDefaultsArgSpec(
+  Callable originalCallable, Callable defaultsCallable, string originalArgSpec,
+  string defaultsArgSpec
+) {
+  relatedArgSpec(originalCallable, originalArgSpec) and
+  defaultsCallable = originalCallable.getKotlinParameterDefaultsProxy() and
+  (
+    originalCallable instanceof Constructor and originalArgSpec = defaultsArgSpec
+    or
+    originalCallable instanceof Method and
+    exists(string regex |
+      // Note I use a regex and not AccessPathToken because this feeds summaryElement et al,
+      // which would introduce mutual recursion with the definition of AccessPathToken.
+      regex = "Argument\\[([0-9]+)\\](.*)" and
+      (
+        exists(string oldArgNumber, string rest, int paramOffset |
+          oldArgNumber = originalArgSpec.regexpCapture(regex, 1) and
+          rest = originalArgSpec.regexpCapture(regex, 2) and
+          paramOffset =
+            (
+              defaultsCallable.getNumberOfParameters() -
+                (originalCallable.getNumberOfParameters() + 2)
+            ) and
+          if ktExtensionFunctions(originalCallable, _, _) and oldArgNumber = "0"
+          then defaultsArgSpec = originalArgSpec
+          else defaultsArgSpec = "Argument[" + (oldArgNumber.toInt() + paramOffset) + "]" + rest
+        )
+        or
+        not originalArgSpec.regexpMatch(regex) and
+        defaultsArgSpec = originalArgSpec
+      )
+    )
+  )
+}
+
 /**
 * Holds if an external flow summary exists for `c` with input specification
 * `input`, output specification `output`, kind `kind`, and a flag `generated`
@@ -75,11 +130,19 @@ predicate summaryElement(
 ) {
  exists(
    string namespace, string type, boolean subtypes, string name, string signature, string ext,
-    string provenance
+    string provenance, string originalInput, string originalOutput, Callable baseCallable
  |
-    summaryModel(namespace, type, subtypes, name, signature, ext, input, output, kind, provenance) and
+    summaryModel(namespace, type, subtypes, name, signature, ext, originalInput, originalOutput,
+      kind, provenance) and
    generated = isGenerated(provenance) and
-    c.asCallable() = interpretElement(namespace, type, subtypes, name, signature, ext)
+    baseCallable = interpretElement(namespace, type, subtypes, name, signature, ext) and
+    (
+      c.asCallable() = baseCallable and input = originalInput and output = originalOutput
+      or
+      correspondingKotlinParameterDefaultsArgSpec(baseCallable, c.asCallable(), originalInput, input) and
+      correspondingKotlinParameterDefaultsArgSpec(baseCallable, c.asCallable(), originalOutput,
+        output)
+    )
  )
 }

@@ -149,11 +212,16 @@ class SourceOrSinkElement = Top;
 predicate sourceElement(SourceOrSinkElement e, string output, string kind, boolean generated) {
  exists(
    string namespace, string type, boolean subtypes, string name, string signature, string ext,
-    string provenance
+    string provenance, SourceOrSinkElement baseSource, string originalOutput
  |
-    sourceModel(namespace, type, subtypes, name, signature, ext, output, kind, provenance) and
+    sourceModel(namespace, type, subtypes, name, signature, ext, originalOutput, kind, provenance) and
    generated = isGenerated(provenance) and
-    e = interpretElement(namespace, type, subtypes, name, signature, ext)
+    baseSource = interpretElement(namespace, type, subtypes, name, signature, ext) and
+    (
+      e = baseSource and output = originalOutput
+      or
+      correspondingKotlinParameterDefaultsArgSpec(baseSource, e, originalOutput, output)
+    )
  )
 }

@@ -165,11 +233,16 @@ predicate sourceElement(SourceOrSinkElement e, string output, string kind, boole
 predicate sinkElement(SourceOrSinkElement e, string input, string kind, boolean generated) {
  exists(
    string namespace, string type, boolean subtypes, string name, string signature, string ext,
-    string provenance
+    string provenance, SourceOrSinkElement baseSink, string originalInput
  |
-    sinkModel(namespace, type, subtypes, name, signature, ext, input, kind, provenance) and
+    sinkModel(namespace, type, subtypes, name, signature, ext, originalInput, kind, provenance) and
    generated = isGenerated(provenance) and
-    e = interpretElement(namespace, type, subtypes, name, signature, ext)
+    baseSink = interpretElement(namespace, type, subtypes, name, signature, ext) and
+    (
+      e = baseSink and originalInput = input
+      or
+      correspondingKotlinParameterDefaultsArgSpec(baseSink, e, originalInput, input)
+    )
  )
 }