Merge pull request #10876 from smowton/smowton/feature/kotlin-default-method-auto-mad

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

java/ql/integration-tests/posix-only/kotlin/default-parameter-mad-flow/lib.kt

@@ -0,0 +1,35 @@
+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
+
+  fun multiParameterTest(x: Int, y: Int, z: Int, w: Int = 0) = 0
+  fun Int.multiParameterExtensionTest(x: Int, y: Int, w: Int = 0) = 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) {}
+
+}

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")

java/ql/integration-tests/posix-only/kotlin/default-parameter-mad-flow/test.ql

@@ -0,0 +1,74 @@
+import java
+import semmle.code.java.dataflow.TaintTracking
+import TestUtilities.InlineExpectationsTest
+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",
+        ";LibClass;true;multiParameterTest;(int,int,int,int);;Argument[0..1];ReturnValue;value;manual",
+        ";LibClass;true;multiParameterExtensionTest;(int,int,int,int);;Argument[0, 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")
+  }
+}
+
+class InlineFlowTest extends InlineExpectationsTest {
+  InlineFlowTest() { this = "HasFlowTest" }
+
+  override string getARelevantTag() { result = "flow" }
+
+  override predicate hasActualResult(Location location, string element, string tag, string value) {
+    tag = "flow" and
+    exists(DataFlow::Node src, DataFlow::Node sink, Config c | c.hasFlow(src, sink) |
+      sink.getLocation() = location and
+      element = sink.toString() and
+      value = ""
+    )
+  }
+}

java/ql/integration-tests/posix-only/kotlin/default-parameter-mad-flow/user.kt

@@ -0,0 +1,60 @@
+fun source() = 1
+
+fun sink(x: Any) { }
+
+fun test(c: LibClass, sourcec: SourceClass, sinkc: SinkClass) {
+
+  sink(ConstructorWithDefaults(source(), 0)) // $ flow
+  sink(ConstructorWithDefaults(source())) // $ flow
+
+  sink(topLevelWithDefaults(source(), 0)) // $ flow
+  sink(topLevelWithDefaults(source())) // $ flow
+
+  sink("Hello world".extensionWithDefaults(source(), 0)) // $ flow
+  sink("Hello world".extensionWithDefaults(source())) // $ flow
+
+  sink(c.memberWithDefaults(source(), 0)) // $ flow
+  sink(c.memberWithDefaults(source())) // $ flow
+
+  sink(c.multiParameterTest(source(), 0, 0)) // $ flow
+  sink(c.multiParameterTest(0, source(), 0)) // $ flow
+  sink(c.multiParameterTest(0, 0, source()))
+
+  with(c) {
+    sink("Hello world".extensionMemberWithDefaults(source(), 0)) // $ flow
+    sink("Hello world".extensionMemberWithDefaults(source())) // $ flow
+  }
+
+  with(c) {
+    sink(source().multiParameterExtensionTest(0, 0)) // $ flow
+    sink(0.multiParameterExtensionTest(source(), 0)) // $ flow
+    sink(0.multiParameterExtensionTest(0, source()))
+  }
+
+  run {
+    val st = SomeToken()
+    topLevelArgSource(st)
+    sink(st) // $ flow
+  }
+
+  run {
+    val st = SomeToken()
+    "Hello world".extensionArgSource(st)
+    sink(st) // $ flow
+  }
+
+  run {
+    val st = SomeToken()
+    sourcec.memberArgSource(st)
+    sink(st) // $ flow
+  }
+
+  SinkClass(source()) // $ flow
+  topLevelSink(source()) // $ flow
+  "Hello world".extensionSink(source()) // $ flow
+  sinkc.memberSink(source()) // $ flow
+  with(sinkc) {
+    "Hello world".extensionMemberSink(source()) // $ flow
+  }
+
+}

java/ql/integration-tests/posix-only/kotlin/qlpack.yml

@@ -1,2 +1,3 @@
 libraryPathDependencies:
   - codeql-java
+  - codeql/java-tests

java/ql/lib/semmle/code/java/Member.qll

@@ -309,7 +309,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
@@ -319,16 +321,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))
       )

java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImplSpecific.qll

@@ -10,6 +10,7 @@ private import FlowSummaryImpl::Private
 private import FlowSummaryImpl::Public
 private import semmle.code.java.dataflow.ExternalFlow
 private import semmle.code.java.dataflow.FlowSummary as FlowSummary
+private import semmle.code.java.dataflow.internal.AccessPathSyntax as AccessPathSyntax
 
 class SummarizedCallableBase = FlowSummary::SummarizedCallableBase;
 
@@ -65,6 +66,66 @@ 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`.
+ *
+ * Note in the case where `originalArgSpec` uses an integer range, like `Argument[1..3]...`, this will produce multiple
+ * results for `defaultsArgSpec`, like `{Argument[2]..., Argument[3]..., Argument[4]...}`.
+ */
+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
+          exists(int oldArgParsed |
+            oldArgParsed = AccessPathSyntax::AccessPath::parseInt(oldArgNumber.splitAt(",").trim())
+          |
+            if ktExtensionFunctions(originalCallable, _, _) and oldArgParsed = 0
+            then defaultsArgSpec = "Argument[0]"
+            else defaultsArgSpec = "Argument[" + (oldArgParsed + 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 +136,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 +218,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 +239,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)
+    )
   )
 }
 

java/ql/test/kotlin/library-tests/parameter-defaults/defaults.expected

@@ -1,5 +1,7 @@
 | test.kt:5:3:7:3 | f | test.kt:5:3:7:3 | f$default |
+| test.kt:19:3:22:3 | f | test.kt:19:3:22:3 | f$default |
 | test.kt:34:14:36:3 | f | test.kt:34:14:36:3 | f$default |
+| test.kt:56:3:58:3 | test | test.kt:56:3:58:3 | test$default |
 | test.kt:68:1:80:1 | TestConstructor | test.kt:68:1:80:1 | TestConstructor |
 | test.kt:86:5:88:5 | f | test.kt:86:5:88:5 | f$default |
 | test.kt:106:7:108:7 | f | test.kt:106:7:108:7 | f$default |