Java: Fix bug involving varadic parameters

Author: joefarebrother

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

@@ -257,10 +257,23 @@ 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(MethodAccess ma, int arg) {
+  exists(ma.getArgument(arg)) and
+  exists(Method m | m = ma.getMethod() |
+    if m.isVarargs() and arg >= m.getNumberOfParameters()
+    then result = m.getNumberOfParameters() - 2
+    else result = arg
+  )
+}
+
 /** Access to a method that passes taint from qualifier to argument. */
 private predicate qualifierToArgumentStep(Expr tracked, Expr sink) {
   exists(MethodAccess ma, int arg |
-    taintPreservingQualifierToArgument(ma.getMethod(), arg) and
+    taintPreservingQualifierToArgument(ma.getMethod(), argToParam(ma, arg)) and
     tracked = ma.getQualifier() and
     sink = ma.getArgument(arg)
   )
@@ -394,7 +407,7 @@ private predicate unsafeEscape(MethodAccess ma) {
 private predicate argToMethodStep(Expr tracked, MethodAccess sink) {
   exists(Method m, int i |
     m = sink.getMethod() and
-    taintPreservingArgumentToMethod(m, i) and
+    taintPreservingArgumentToMethod(m, argToParam(sink, i)) and
     tracked = sink.getArgument(i)
   )
   or
@@ -519,7 +532,7 @@ private predicate taintPreservingArgumentToMethod(Method method, int arg) {
  */
 private predicate argToArgStep(Expr tracked, Expr sink) {
   exists(MethodAccess ma, Method method, int input, int output |
-    taintPreservingArgToArg(method, input, output) and
+    taintPreservingArgToArg(method, argToParam(ma, input), argToParam(ma, output)) and
     ma.getMethod() = method and
     ma.getArgument(input) = tracked and
     ma.getArgument(output) = sink
@@ -567,7 +580,7 @@ private predicate taintPreservingArgToArg(Method method, int input, int output)
  */
 private predicate argToQualifierStep(Expr tracked, Expr sink) {
   exists(Method m, int i, MethodAccess ma |
-    taintPreservingArgumentToQualifier(m, i) and
+    taintPreservingArgumentToQualifier(m, argToParam(ma, i)) and
     ma.getMethod() = m and
     tracked = ma.getArgument(i) and
     sink = ma.getQualifier()

java/ql/test/library-tests/dataflow/taint-format/A.java

@@ -14,6 +14,7 @@ public static void test1() {
         good.formatted("a", bad, "b", good);
         String.format("%s%s", bad, good);
         String.format("%s", good);
+        String.format("%s %s %s %s %s %s %s %s %s %s ", "a", "a", "a", "a", "a", "a", "a", "a", "a", bad);
     }
 
     public static void test2() {

java/ql/test/library-tests/dataflow/taint-format/test.expected

@@ -7,23 +7,26 @@
 | A.java:10:22:10:28 | taint(...) | A.java:15:9:15:40 | format(...) |
 | A.java:10:22:10:28 | taint(...) | A.java:15:9:15:40 | new ..[] { .. } |
 | A.java:10:22:10:28 | taint(...) | A.java:15:31:15:33 | bad |
-| A.java:20:22:20:28 | taint(...) | A.java:20:22:20:28 | taint(...) |
-| A.java:20:22:20:28 | taint(...) | A.java:24:9:24:9 | f [post update] |
-| A.java:20:22:20:28 | taint(...) | A.java:24:9:24:27 | format(...) |
-| A.java:20:22:20:28 | taint(...) | A.java:24:9:24:27 | new ..[] { .. } |
-| A.java:20:22:20:28 | taint(...) | A.java:24:24:24:26 | bad |
-| A.java:20:22:20:28 | taint(...) | A.java:25:9:25:9 | f |
-| A.java:20:22:20:28 | taint(...) | A.java:25:9:25:20 | toString(...) |
-| A.java:29:22:29:28 | taint(...) | A.java:29:22:29:28 | taint(...) |
-| A.java:29:22:29:28 | taint(...) | A.java:33:9:33:10 | sb |
-| A.java:29:22:29:28 | taint(...) | A.java:33:9:33:21 | toString(...) |
-| A.java:29:22:29:28 | taint(...) | A.java:34:9:34:9 | f [post update] |
-| A.java:29:22:29:28 | taint(...) | A.java:34:9:34:27 | format(...) |
-| A.java:29:22:29:28 | taint(...) | A.java:34:9:34:27 | new ..[] { .. } |
-| A.java:29:22:29:28 | taint(...) | A.java:34:24:34:26 | bad |
-| A.java:29:22:29:28 | taint(...) | A.java:35:9:35:10 | sb |
-| A.java:29:22:29:28 | taint(...) | A.java:35:9:35:21 | toString(...) |
-| A.java:39:22:39:28 | taint(...) | A.java:39:22:39:28 | taint(...) |
-| A.java:39:22:39:28 | taint(...) | A.java:42:18:42:20 | bad |
-| A.java:39:22:39:28 | taint(...) | A.java:43:9:43:46 | new ..[] { .. } |
-| A.java:39:22:39:28 | taint(...) | A.java:43:43:43:45 | bad |
+| A.java:10:22:10:28 | taint(...) | A.java:17:9:17:105 | format(...) |
+| A.java:10:22:10:28 | taint(...) | A.java:17:9:17:105 | new ..[] { .. } |
+| A.java:10:22:10:28 | taint(...) | A.java:17:102:17:104 | bad |
+| A.java:21:22:21:28 | taint(...) | A.java:21:22:21:28 | taint(...) |
+| A.java:21:22:21:28 | taint(...) | A.java:25:9:25:9 | f [post update] |
+| A.java:21:22:21:28 | taint(...) | A.java:25:9:25:27 | format(...) |
+| A.java:21:22:21:28 | taint(...) | A.java:25:9:25:27 | new ..[] { .. } |
+| A.java:21:22:21:28 | taint(...) | A.java:25:24:25:26 | bad |
+| A.java:21:22:21:28 | taint(...) | A.java:26:9:26:9 | f |
+| A.java:21:22:21:28 | taint(...) | A.java:26:9:26:20 | toString(...) |
+| A.java:30:22:30:28 | taint(...) | A.java:30:22:30:28 | taint(...) |
+| A.java:30:22:30:28 | taint(...) | A.java:34:9:34:10 | sb |
+| A.java:30:22:30:28 | taint(...) | A.java:34:9:34:21 | toString(...) |
+| A.java:30:22:30:28 | taint(...) | A.java:35:9:35:9 | f [post update] |
+| A.java:30:22:30:28 | taint(...) | A.java:35:9:35:27 | format(...) |
+| A.java:30:22:30:28 | taint(...) | A.java:35:9:35:27 | new ..[] { .. } |
+| A.java:30:22:30:28 | taint(...) | A.java:35:24:35:26 | bad |
+| A.java:30:22:30:28 | taint(...) | A.java:36:9:36:10 | sb |
+| A.java:30:22:30:28 | taint(...) | A.java:36:9:36:21 | toString(...) |
+| A.java:40:22:40:28 | taint(...) | A.java:40:22:40:28 | taint(...) |
+| A.java:40:22:40:28 | taint(...) | A.java:43:18:43:20 | bad |
+| A.java:40:22:40:28 | taint(...) | A.java:44:9:44:46 | new ..[] { .. } |
+| A.java:40:22:40:28 | taint(...) | A.java:44:43:44:45 | bad |