Merge pull request #9319 from atorralba/atorralba/add-editable-valueof-step

Kotlin: Add taint step for String.valueOf(Editable)

java/ql/lib/change-notes/2022-05-25-string-valueof-editable-step.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+Added a flow step for `String.valueOf` calls on tainted `android.text.Editable` objects. 

java/ql/lib/semmle/code/java/frameworks/android/Widget.qll

@@ -18,12 +18,20 @@ private class DefaultAndroidWidgetSources extends RemoteFlowSource {
 
 private class EditableToStringStep extends AdditionalTaintStep {
   override predicate step(DataFlow::Node n1, DataFlow::Node n2) {
-    exists(MethodAccess toString |
-      toString.getMethod().hasName("toString") and
-      toString.getReceiverType().hasQualifiedName("android.text", "Editable")
-    |
-      n1.asExpr() = toString.getQualifier() and
-      n2.asExpr() = toString
+    exists(MethodAccess ma |
+      ma.getMethod().hasName("toString") and
+      ma.getReceiverType().getASourceSupertype*().hasQualifiedName("android.text", "Editable") and
+      n1.asExpr() = ma.getQualifier() and
+      n2.asExpr() = ma
+      or
+      ma.getMethod().hasQualifiedName("java.lang", "String", "valueOf") and
+      ma.getArgument(0)
+          .getType()
+          .(RefType)
+          .getASourceSupertype*()
+          .hasQualifiedName("android.text", "Editable") and
+      n1.asExpr() = ma.getArgument(0) and
+      n2.asExpr() = ma
     )
   }
 }

java/ql/test/library-tests/frameworks/android/widget/TestWidget.java

@@ -2,10 +2,14 @@ import android.widget.EditText;
 
 public class TestWidget {
 
+    private EditText source() {
+        return null;
+    }
+
     private void sink(Object sink) {}
 
-    public void test(EditText t) {
-        sink(t.getText().toString()); // $ hasTaintFlow
+    public void test() {
+        sink(source().getText().toString()); // $ hasTaintFlow
     }
 }
 

java/ql/test/library-tests/frameworks/android/widget/TestWidgetKt.kt

@@ -0,0 +1,16 @@
+ import android.text.Editable
+
+ class TestWidgetKt {
+
+    fun source() : Editable? { return null }
+    fun sink(sink : String) {}
+
+    fun test() {
+        val t = source()
+        sink(t.toString()); // $ hasTaintFlow
+
+        val t2 : Any? = source()
+        sink(t2.toString()); // $ MISSING: hasTaintFlow
+    }
+}
+

java/ql/test/library-tests/frameworks/android/widget/options

@@ -1 +1,2 @@
 //semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/google-android-9.0.0
+//codeql-extractor-kotlin-options: ${testdir}/../../../../stubs/google-android-9.0.0

java/ql/test/library-tests/frameworks/android/widget/test.expected

@@ -0,0 +1,4 @@
+failures
+valueOf
+| TestWidgetKt.kt:10:16:10:25 | valueOf(...) |
+| TestWidgetKt.kt:13:17:13:26 | valueOf(...) |

java/ql/test/library-tests/frameworks/android/widget/test.ql

@@ -2,6 +2,6 @@ import java
 import semmle.code.java.dataflow.FlowSources
 import TestUtilities.InlineFlowTest
 
-class SourceTaintFlowConf extends DefaultTaintFlowConf {
-  override predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource }
+query predicate valueOf(MethodAccess ma) {
+  ma.getMethod().hasQualifiedName("java.lang", "String", "valueOf")
 }