Merge pull request #10909 from smowton/smowton/fix/kotlin-varargs-dataflow

Kotlin: Fix varargs dataflow, and varargs default handling

Author: smowton

java/kotlin-extractor/src/main/kotlin/KotlinFileExtractor.kt

@@ -1884,7 +1884,7 @@ open class KotlinFileExtractor(
             IrConstImpl.defaultValueForType(0, 0, getDefaultsMethodLastArgType(callTarget))
         )
 
-        extractCallValueArguments(id, valueArgsWithDummies + extraArgs, enclosingStmt, enclosingCallable, nextIdx)
+        extractCallValueArguments(id, valueArgsWithDummies + extraArgs, enclosingStmt, enclosingCallable, nextIdx, extractVarargAsArray = true)
     }
 
     private fun getFunctionInvokeMethod(typeArgs: List<IrTypeArgument>): IrFunction? {
@@ -1961,8 +1961,12 @@ open class KotlinFileExtractor(
         superQualifierSymbol: IrClassSymbol? = null) {
 
         val locId = tw.getLocation(locElement)
+        val varargParam = syntacticCallTarget.valueParameters.withIndex().find { it.value.isVararg }
+        // If the vararg param is the only one not specified, and it has no default value, then we don't need to call a $default method,
+        // as omitting it already implies passing an empty vararg array.
+        val nullAllowedIdx = if (varargParam != null && varargParam.value.defaultValue == null) varargParam.index else -1
 
-        if (valueArguments.any { it == null }) {
+        if (valueArguments.withIndex().any { (index, it) -> it == null && index != nullAllowedIdx }) {
             extractsDefaultsCall(
                 syntacticCallTarget,
                 locId,
@@ -2082,11 +2086,11 @@ open class KotlinFileExtractor(
     private fun extractCallValueArguments(callId: Label<out DbExprparent>, call: IrFunctionAccessExpression, enclosingStmt: Label<out DbStmt>, enclosingCallable: Label<out DbCallable>, idxOffset: Int) =
         extractCallValueArguments(callId, (0 until call.valueArgumentsCount).map { call.getValueArgument(it) }, enclosingStmt, enclosingCallable, idxOffset)
 
-    private fun extractCallValueArguments(callId: Label<out DbExprparent>, valueArguments: List<IrExpression?>, enclosingStmt: Label<out DbStmt>, enclosingCallable: Label<out DbCallable>, idxOffset: Int) {
+    private fun extractCallValueArguments(callId: Label<out DbExprparent>, valueArguments: List<IrExpression?>, enclosingStmt: Label<out DbStmt>, enclosingCallable: Label<out DbCallable>, idxOffset: Int, extractVarargAsArray: Boolean = false) {
         var i = 0
         valueArguments.forEach { arg ->
             if(arg != null) {
-                if (arg is IrVararg) {
+                if (arg is IrVararg && !extractVarargAsArray) {
                     arg.elements.forEachIndexed { varargNo, vararg -> extractVarargElement(vararg, enclosingCallable, callId, i + idxOffset + varargNo, enclosingStmt) }
                     i += arg.elements.size
                 } else {

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

@@ -2367,14 +2367,33 @@ class Argument extends Expr {
    */
   predicate isNthVararg(int arrayindex) {
     not this.isExplicitVarargsArray() and
-    exists(Callable tgt, int varargsParamPos |
+    exists(Callable tgt |
       call.getCallee() = tgt and
-      tgt.getParameter(varargsParamPos).isVarargs() and
-      arrayindex = pos - varargsParamPos and
+      arrayindex = pos - tgt.getVaragsParameterIndex() and
       arrayindex >= 0 and
       arrayindex <= call.getNumArgument() - tgt.getNumberOfParameters()
     )
   }
+
+  /**
+   * Gets the parameter position that will receive this argument.
+   *
+   * For all vararg arguments, this is the position of the vararg array parameter.
+   */
+  int getParameterPos() {
+    exists(Callable c | c = call.getCallee() |
+      if c.isVarargs()
+      then
+        if pos < c.getVaragsParameterIndex()
+        then result = pos // Vararg method argument, before the vararg parameter
+        else (
+          if this.isVararg()
+          then result = c.getVaragsParameterIndex() // Part of the implicit vararg array
+          else result = pos - (call.getNumArgument() - c.getNumberOfParameters()) // Vararg method argument, after the vararg parameter (offset could be -1 in the zero-vararg case)
+        )
+      else result = pos // Not a vararg method
+    )
+  }
 }
 
 /**

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

@@ -283,6 +283,9 @@ class Callable extends StmtParent, Member, @callable {
   /** Holds if the last parameter of this callable is a varargs (variable arity) parameter. */
   predicate isVarargs() { this.getAParameter().isVarargs() }
 
+  /** Gets the index of this callable's varargs parameter, if any exists. */
+  int getVaragsParameterIndex() { this.getParameter(result).isVarargs() }
+
   /**
    * Gets the signature of this callable, where all types in the signature have a fully-qualified name.
    * The parameter types are only separated by a comma (without space). If this callable has

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

@@ -378,11 +378,11 @@ module Private {
      */
     predicate argumentOf(DataFlowCall call, int pos) {
       exists(Argument arg | this.asExpr() = arg |
-        call.asCall() = arg.getCall() and pos = arg.getPosition()
+        call.asCall() = arg.getCall() and pos = arg.getParameterPos()
       )
       or
       call.asCall() = this.(ImplicitVarargsArray).getCall() and
-      pos = call.asCall().getCallee().getNumberOfParameters() - 1
+      pos = call.asCall().getCallee().getVaragsParameterIndex()
       or
       pos = -1 and this = getInstanceArgument(call.asCall())
       or

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

@@ -282,13 +282,8 @@ private predicate constructorStep(Expr tracked, ConstructorCall sink) {
  * Converts an argument index to a formal parameter index.
  * This is relevant for varadic methods.
  */
-private int argToParam(Call call, int arg) {
-  exists(call.getArgument(arg)) and
-  exists(Callable c | c = call.getCallee() |
-    if c.isVarargs() and arg >= c.getNumberOfParameters()
-    then result = c.getNumberOfParameters() - 1
-    else result = arg
-  )
+private int argToParam(Call call, int argIdx) {
+  result = call.getArgument(argIdx).(Argument).getParameterPos()
 }
 
 /** Access to a method that passes taint from qualifier to argument. */