Merge pull request #9783 from tamasvajk/feature/kotlin-stdlib-mad

tamasvajk · web-flow · commit 3410dd589da8 · 2022-09-07T12:57:23.000+02:00
Kotlin: Add MaD for stdlib
diff --git a/java/ql/lib/change-notes/2022-08-31-kotlin-stdlib.md b/java/ql/lib/change-notes/2022-08-31-kotlin-stdlib.md
@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Added flow sinks, sources and summaries for the Kotlin standard library.
diff --git a/java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll b/java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll
@@ -153,7 +153,7 @@ private module Frameworks {
   private import semmle.code.java.frameworks.JMS
   private import semmle.code.java.frameworks.RabbitMQ
   private import semmle.code.java.regex.RegexFlowModels
-  private import semmle.code.java.frameworks.KotlinStdLib
+  private import semmle.code.java.frameworks.kotlin.StdLib
 }
 
 /**
diff --git a/java/ql/lib/semmle/code/java/frameworks/KotlinStdLib.qll b/java/ql/lib/semmle/code/java/frameworks/KotlinStdLib.qll
diff --git a/java/ql/lib/semmle/code/java/frameworks/generated.qll b/java/ql/lib/semmle/code/java/frameworks/generated.qll
@@ -6,4 +6,5 @@ import java
 
 private module GeneratedFrameworks {
   private import apache.IOGenerated
+  private import kotlin.StdLibGenerated
 }
diff --git a/java/ql/lib/semmle/code/java/frameworks/kotlin/NegativeStdLibGenerated.qll b/java/ql/lib/semmle/code/java/frameworks/kotlin/NegativeStdLibGenerated.qll
diff --git a/java/ql/lib/semmle/code/java/frameworks/kotlin/StdLib.qll b/java/ql/lib/semmle/code/java/frameworks/kotlin/StdLib.qll
@@ -0,0 +1,14 @@
+/** Definitions of taint steps in the KotlinStdLib framework */
+
+import java
+private import semmle.code.java.dataflow.ExternalFlow
+
+private class KotlinStdLibSummaryCsv extends SummaryModelCsv {
+  override predicate row(string row) {
+    row =
+      [
+        "kotlin.jvm.internal;ArrayIteratorKt;false;iterator;(Object[]);;Argument[0].ArrayElement;ReturnValue.Element;value;manual",
+        "kotlin.collections;ArraysKt;false;withIndex;(Object[]);;Argument[0].ArrayElement;ReturnValue;taint;manual"
+      ]
+  }
+}
diff --git a/java/ql/lib/semmle/code/java/frameworks/kotlin/StdLibGenerated.qll b/java/ql/lib/semmle/code/java/frameworks/kotlin/StdLibGenerated.qll
diff --git a/java/ql/test/kotlin/library-tests/dataflow/summaries/list.kt b/java/ql/test/kotlin/library-tests/dataflow/summaries/list.kt
@@ -4,17 +4,17 @@ class ListFlowTest {
 
     fun test(l: MutableList<String>) {
         l[0] = taint("a")
-        sink(l)
-        sink(l[0])
+        sink(l)             // $ hasTaintFlow=a
+        sink(l[0])          // $ hasValueFlow=a
         for (s in l) {
-            sink(s)
+            sink(s)         // $ hasValueFlow=a
         }
 
-        val a = arrayOf(taint("a"), "b")
-        sink(a)
-        sink(a[0])
+        val a = arrayOf(taint("b"), "c")
+        sink(a)             // $ hasTaintFlow=b
+        sink(a[0])          // $ hasValueFlow=b
         for (s in a) {
-            sink(s)
+            sink(s)         // $ hasValueFlow=b
         }
     }
 }
diff --git a/java/ql/test/kotlin/library-tests/dataflow/summaries/test.expected b/java/ql/test/kotlin/library-tests/dataflow/summaries/test.expected
@@ -1,6 +1,5 @@
-| list.kt:6:23:6:23 | a | list.kt:7:14:7:14 | l |
-| list.kt:6:23:6:23 | a | list.kt:8:14:8:17 | get(...) |
-| list.kt:6:23:6:23 | a | list.kt:10:18:10:18 | s |
-| list.kt:13:32:13:32 | a | list.kt:14:14:14:14 | a |
-| list.kt:13:32:13:32 | a | list.kt:15:14:15:17 | ...[...] |
-| list.kt:13:32:13:32 | a | list.kt:17:18:17:18 | s |
+| test.kt:28:16:28:21 | getSecond(...) | Unexpected result: hasTaintFlow=a |
+| test.kt:35:16:35:27 | component1(...) | Unexpected result: hasTaintFlow=d |
+| test.kt:41:17:41:22 | getSecond(...) | Unexpected result: hasTaintFlow=e |
+| test.kt:53:17:53:24 | getDuration(...) | Unexpected result: hasTaintFlow=f |
+| test.kt:58:18:58:29 | component2(...) | Unexpected result: hasTaintFlow=g |
diff --git a/java/ql/test/kotlin/library-tests/dataflow/summaries/test.kt b/java/ql/test/kotlin/library-tests/dataflow/summaries/test.kt
@@ -0,0 +1,75 @@
+import kotlin.time.Duration
+import kotlin.time.ExperimentalTime
+import kotlin.time.TimedValue
+
+class Test {
+    fun <T> taint(t: T) = t
+    fun sink(a: Any) {}
+
+    @OptIn(ExperimentalTime::class)
+    fun test(b: ByteArray,
+             f: kotlin.io.FileTreeWalk,
+             c1: CharArray,
+             c2: CharArray,
+             c3: CharArray,) {
+
+        sink(taint(b).copyOf())                     // $ hasTaintFlow
+        sink(taint(f).maxDepth(1))                  // $ hasTaintFlow
+
+        val sb = StringBuilder()
+        sink(sb.insertRange(0, taint(c1), 0, 0))    // $ hasTaintFlow
+        sink(sb)                                    // $ hasTaintFlow
+
+        sink(taint(c2) + c3)                        // $ hasTaintFlow
+
+        val p = Pair(taint("a"), "")
+        sink(p)                                     // $ hasTaintFlow=a
+        sink(p.component1())                        // $ hasTaintFlow=a
+        sink(p.second)
+
+        sink(taint("b").capitalize())                   // $ hasTaintFlow=b
+        sink(taint("c").replaceFirstChar { _ -> 'x' })  // $ hasTaintFlow=c
+
+        val t = Triple("", taint("d"), "")
+        sink(t)                                     // $ hasTaintFlow=d
+        sink(t.component1())
+        sink(t.second)                              // $ hasTaintFlow=d
+
+        val p1 = taint("e") to ""
+        sink(p1)                                    // $ hasTaintFlow=e
+        sink(p1.component1())                       // $ hasTaintFlow=e
+        sink(p1.second)
+
+        val l = p.toList()
+        sink(l)                                     // $ hasTaintFlow=a
+        sink(l[0])                                  // $ hasTaintFlow=a
+        for (s in l) {
+            sink(s)                                 // $ hasTaintFlow=a
+        }
+
+        val tv = TimedValue(taint("f"), Duration.parse(""))
+        sink(tv)                                    // $ hasTaintFlow=f
+        sink(tv.component1())                       // $ hasTaintFlow=f
+        sink(tv.duration)
+
+        val mg0 = MatchGroup(taint("g"), IntRange(0, 10))
+        sink(mg0)                                   // $ hasTaintFlow=g
+        sink(mg0.value)                             // $ hasTaintFlow=g
+        sink(mg0.component2())
+
+        val iv = IndexedValue<String>(5, taint("h"))
+        sink(iv)                                    // $ hasTaintFlow=h
+        sink(iv.index)
+        sink(iv.component2())                       // $ hasTaintFlow=h
+
+        val strings = arrayOf("", taint("i"))
+        sink(strings.withIndex())                   // $ hasTaintFlow=i
+        sink(strings.withIndex().toList())          // $ hasTaintFlow=i
+        sink(strings.withIndex().toList()[0].value) // $ hasTaintFlow=i
+        sink(strings.withIndex().toList()[0].index)
+        for (x in strings.withIndex()) {
+            sink(x.value)                           // $ hasTaintFlow=i
+            sink(x.index)
+        }
+    }
+}
diff --git a/java/ql/test/kotlin/library-tests/dataflow/summaries/test.ql b/java/ql/test/kotlin/library-tests/dataflow/summaries/test.ql
@@ -1,19 +1,2 @@
 import java
-import semmle.code.java.dataflow.TaintTracking
-import semmle.code.java.dataflow.ExternalFlow
-
-class Conf extends TaintTracking::Configuration {
-  Conf() { this = "qltest:mad-summaries" }
-
-  override predicate isSource(DataFlow::Node n) {
-    n.asExpr().(Argument).getCall().getCallee().hasName("taint")
-  }
-
-  override predicate isSink(DataFlow::Node n) {
-    n.asExpr().(Argument).getCall().getCallee().hasName("sink")
-  }
-}
-
-from DataFlow::Node src, DataFlow::Node sink, Conf conf
-where conf.hasFlow(src, sink)
-select src, sink
+import TestUtilities.InlineFlowTest
diff --git a/misc/scripts/models-as-data/generate_flow_model.py b/misc/scripts/models-as-data/generate_flow_model.py
@@ -20,7 +20,7 @@ def __init__ (self, language):
 
     def printHelp(self):
         print(f"""Usage:
-python3 GenerateFlowModel.py <library-database> <outputQll> [--with-sinks] [--with-sources] [--with-summaries] [--dry-run]
+python3 GenerateFlowModel.py <library-database> <outputQll> [<friendlyFrameworkName>] [--with-sinks] [--with-sources] [--with-summaries] [--dry-run]
 
 This generates summary, source and sink models for the code in the database.
 The files will be placed in `{self.language}/ql/lib/semmle/code/{self.language}/frameworks/<outputQll>` where
@@ -39,18 +39,23 @@ def printHelp(self):
 
 Example invocations:
 $ python3 GenerateFlowModel.py /tmp/dbs/my_library_db "mylibrary/Framework.qll"
+$ python3 GenerateFlowModel.py /tmp/dbs/my_library_db "mylibrary/Framework.qll" "Friendly Name of Framework"
 $ python3 GenerateFlowModel.py /tmp/dbs/my_library_db "mylibrary/FrameworkSinks.qll" --with-sinks
 
 Requirements: `codeql` should both appear on your path.
     """)
 
 
-    def setenvironment(self, target, database):
+    def setenvironment(self, target, database, friendlyName):
         self.codeQlRoot = subprocess.check_output(["git", "rev-parse", "--show-toplevel"]).decode("utf-8").strip()
         if not target.endswith(".qll"):
             target += ".qll"
         filename = os.path.basename(target)
         dirname = os.path.dirname(target)
+        if friendlyName is not None:
+            self.friendlyname = friendlyName
+        else:
+            self.friendlyname = filename[:-4]
         self.shortname = filename[:-4]
         self.database = database
         self.generatedFrameworks = os.path.join(
@@ -92,11 +97,15 @@ def make(language):
         if not generator.generateSinks and not generator.generateSources and not generator.generateSummaries and not generator.generateNegativeSummaries:
             generator.generateSinks = generator.generateSources = generator.generateSummaries = generator.generateNegativeSummaries = True
 
-        if len(sys.argv) != 3:
+        if len(sys.argv) < 3 or len(sys.argv) > 4:
             generator.printHelp()
             sys.exit(1)
-        
-        generator.setenvironment(sys.argv[2], sys.argv[1])
+
+        friendlyName = None
+        if len(sys.argv) == 4:
+            friendlyName = sys.argv[3]
+
+        generator.setenvironment(sys.argv[2], sys.argv[1], friendlyName)
         return generator
 
 
@@ -178,7 +187,7 @@ def makeContent(self):
         return f"""
 /** 
  * THIS FILE IS AN AUTO-GENERATED MODELS AS DATA FILE. DO NOT EDIT.
- * Definitions of taint steps in the {self.shortname} framework.
+ * Definitions of taint steps in the {self.friendlyname} framework.
  */
 
 import {self.language}
@@ -200,7 +209,7 @@ def makeNegativeContent(self):
         return f"""
 /** 
  * THIS FILE IS AN AUTO-GENERATED MODELS AS DATA FILE. DO NOT EDIT.
- * Definitions of negative summaries in the {self.shortname} framework.
+ * Definitions of negative summaries in the {self.friendlyname} framework.
  */
 
 import {self.language}

-Original file line number
+Diff line change
@@ @@ -0,0 +1,4 @@ @@
 +---
 +category: minorAnalysis
 +---
 +* Added flow sinks, sources and summaries for the Kotlin standard library.
Original file line number	Diff line number	Diff line change
`@@ -153,7 +153,7 @@ private module Frameworks {`
`153`	`153`	`private import semmle.code.java.frameworks.JMS`
`154`	`154`	`private import semmle.code.java.frameworks.RabbitMQ`
`155`	`155`	`private import semmle.code.java.regex.RegexFlowModels`
`156`		`- private import semmle.code.java.frameworks.KotlinStdLib`
	`156`	`+ private import semmle.code.java.frameworks.kotlin.StdLib`
`157`	`157`	`}`
`158`	`158`
`159`	`159`	`/**`
Original file line number	Diff line number	Diff line change
`@@ -6,4 +6,5 @@ import java`
`6`	`6`
`7`	`7`	`private module GeneratedFrameworks {`
`8`	`8`	`private import apache.IOGenerated`
	`9`	`+ private import kotlin.StdLibGenerated`
`9`	`10`	`}`
Original file line number	Diff line number	Diff line change
`@@ -4,17 +4,17 @@ class ListFlowTest {`
`4`	`4`
`5`	`5`	`fun test(l: MutableList<String>) {`
`6`	`6`	`l[0] = taint("a")`
`7`		`- sink(l)`
`8`		`- sink(l[0])`
	`7`	`+ sink(l) // $ hasTaintFlow=a`
	`8`	`+ sink(l[0]) // $ hasValueFlow=a`
`9`	`9`	`for (s in l) {`
`10`		`- sink(s)`
	`10`	`+ sink(s) // $ hasValueFlow=a`
`11`	`11`	`}`
`12`	`12`
`13`		`- val a = arrayOf(taint("a"), "b")`
`14`		`- sink(a)`
`15`		`- sink(a[0])`
	`13`	`+ val a = arrayOf(taint("b"), "c")`
	`14`	`+ sink(a) // $ hasTaintFlow=b`
	`15`	`+ sink(a[0]) // $ hasValueFlow=b`
`16`	`16`	`for (s in a) {`
`17`		`- sink(s)`
	`17`	`+ sink(s) // $ hasValueFlow=b`
`18`	`18`	`}`
`19`	`19`	`}`
`20`	`20`	`}`