Merge pull request #8872 from atorralba/atorralba/android-widget-flowstep

Java: Add Editable.toString flow step

Author: atorralba

java/ql/lib/change-notes/2022-04-26-android-editable-tostring-flow-step.md

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

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

@@ -16,6 +16,18 @@ private class DefaultAndroidWidgetSources extends RemoteFlowSource {
   override string getSourceType() { result = "Android widget source" }
 }
 
+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
+    )
+  }
+}
+
 private class AndroidWidgetSummaryModels extends SummaryModelCsv {
   override predicate row(string row) {
     row = "android.widget;EditText;true;getText;;;Argument[-1];ReturnValue;taint"

java/ql/lib/semmle/code/java/security/CleartextStorageAndroidDatabaseQuery.qll

@@ -11,20 +11,6 @@ private class LocalDatabaseCleartextStorageSink extends CleartextStorageSink {
   LocalDatabaseCleartextStorageSink() { localDatabaseInput(_, this.asExpr()) }
 }
 
-private class LocalDatabaseCleartextStorageStep extends CleartextStorageAdditionalTaintStep {
-  override predicate step(DataFlow::Node n1, DataFlow::Node n2) {
-    // EditText.getText() return type is parsed as `Object`, so we need to
-    // add a taint step for `Object.toString` to model `editText.getText().toString()`
-    exists(MethodAccess ma, Method m |
-      ma.getMethod() = m and
-      m.getDeclaringType() instanceof TypeObject and
-      m.hasName("toString")
-    |
-      n1.asExpr() = ma.getQualifier() and n2.asExpr() = ma
-    )
-  }
-}
-
 /** The creation of an object that can be used to store data in a local database. */
 class LocalDatabaseOpenMethodAccess extends Storable, Call {
   LocalDatabaseOpenMethodAccess() {

java/ql/lib/semmle/code/java/security/CleartextStorageQuery.qll

@@ -85,18 +85,3 @@ private class EncryptedValueFlowConfig extends DataFlow4::Configuration {
 
   override predicate isSink(DataFlow::Node sink) { sink.asExpr() instanceof SensitiveExpr }
 }
-
-/** A taint step for `EditText.toString` in Android. */
-private class AndroidEditTextCleartextStorageStep extends CleartextStorageAdditionalTaintStep {
-  override predicate step(DataFlow::Node n1, DataFlow::Node n2) {
-    // EditText.getText() return type is parsed as `Object`, so we need to
-    // add a taint step for `Object.toString` to model `editText.getText().toString()`
-    exists(MethodAccess ma, Method m |
-      ma.getMethod() = m and
-      m.getDeclaringType() instanceof TypeObject and
-      m.hasName("toString")
-    |
-      n1.asExpr() = ma.getQualifier() and n2.asExpr() = ma
-    )
-  }
-}

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

@@ -0,0 +1,11 @@
+import android.widget.EditText;
+
+public class TestWidget {
+
+    private void sink(Object sink) {}
+
+    public void test(EditText t) {
+        sink(t.getText().toString()); // $ hasTaintFlow
+    }
+}
+

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

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

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

@@ -0,0 +1,7 @@
+import java
+import semmle.code.java.dataflow.FlowSources
+import TestUtilities.InlineFlowTest
+
+class SourceTaintFlowConf extends DefaultTaintFlowConf {
+  override predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource }
+}