Implement checks for elements hidden by their xml attributes

Author: joefarebrother

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

@@ -11,15 +11,10 @@ class AndroidLayoutXmlFile extends XmlFile {
 
 /** A component declared in an Android layout file. */
 class AndroidLayoutXmlElement extends XmlElement {
-  AndroidXmlAttribute id;
+  AndroidLayoutXmlElement() { this.getFile() instanceof AndroidLayoutXmlFile }
 
-  AndroidLayoutXmlElement() {
-    this.getFile() instanceof AndroidLayoutXmlFile and
-    id = this.getAttribute("id")
-  }
-
-  /** Gets the ID of this component. */
-  string getId() { result = id.getValue() }
+  /** Gets the ID of this component, if any. */
+  string getId() { result = this.getAttribute("id").getValue() }
 
   /** Gets the class of this component. */
   Class getClass() {

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

@@ -73,6 +73,14 @@ private module TextFieldTrackingConfig implements DataFlow::ConfigSig {
 /** Holds if the given may be masked. */
 private predicate viewIsMasked(AndroidLayoutXmlElement view) {
   DataFlow::localExprFlow(getAUseOfViewWithId(view.getId()), any(MaskCall mcall).getQualifier())
+  or
+  view.getAttribute("inputType")
+      .(AndroidXmlAttribute)
+      .getValue()
+      .regexpMatch("(?i).*(text|number)(web)?password.*")
+  or
+  view.getAttribute("visibility").(AndroidXmlAttribute).getValue().toLowerCase() =
+    ["invisible", "gone"]
 }
 
 /** Holds if the qualifier of `call` is also called with a method that may mask the information displayed. */

java/ql/test/query-tests/security/CWE-200/semmle/tests/SensitiveTextView/R.java

@@ -7,6 +7,13 @@ public static final class id {
         public static final int test3 = 3;
         public static final int test4 = 4;
         public static final int test5 = 5;
+        public static final int test6 = 6;
+        public static final int test7 = 7;
+        public static final int test8 = 8;
+        public static final int test9 = 9;
+        public static final int test10 = 10;
+        public static final int test11 = 11;
+        public static final int test12 = 12;
     }
 
     public static final class string {

java/ql/test/query-tests/security/CWE-200/semmle/tests/SensitiveTextView/Test.java

@@ -10,22 +10,55 @@
 class Test extends Activity {
     void test(String password) {
         EditText test1 = findViewById(R.id.test1);
+        // BAD: Exposing sensitive data to text view
         test1.setText(password); // $sensitive-text
         test1.setHint(password); // $sensitive-text
         test1.append(password); // $sensitive-text
-        test1.setText(R.string.password_prompt);
+        // GOOD: resource constant is not sensitive info
+        test1.setText(R.string.password_prompt); 
 
+        // GOOD: Visibility is dynamically set
         TextView test2 = findViewById(R.id.test2);
         test2.setVisibility(View.INVISIBLE);
         test2.setText(password);
 
+        // GOOD: Input type is dynamically set
         EditText test3 = findViewById(R.id.test3);
         test3.setInputType(InputType.TYPE_CLASS_TEXT | InputType.TYPE_TEXT_VARIATION_PASSWORD);
         test3.setText(password);
 
+        // GOOD: Visibility of parent is dynamically set
         LinearLayout test4 = findViewById(R.id.test4);
         TextView test5 = findViewById(R.id.test5);
         test4.setVisibility(View.INVISIBLE);
         test5.setText(password);
+
+        // GOOD: Input type set to textPassword in XML
+        EditText test6 = findViewById(R.id.test6);
+        test6.setText(password);
+
+        // GOOD: Input type set to textWebPassword in XML
+        EditText test7 = findViewById(R.id.test7);
+        test7.setText(password);
+
+        // GOOD: Input type set to numberPassword in XML
+        EditText test8 = findViewById(R.id.test8);
+        test8.setText(password);
+
+        // BAD: Input type set to textVisiblePassword in XML, which is not hidden
+        EditText test9 = findViewById(R.id.test9);
+        test9.setText(password); // $sensitive-text
+
+        // GOOD: Visibility set to invisible in XML
+        EditText test10 = findViewById(R.id.test10);
+        test10.setText(password);
+
+        // GOOD: Visibility set to gone in XML
+        EditText test11 = findViewById(R.id.test11);
+        test11.setText(password);
+
+        // GOOD: Visibility of parent set to invisible in XML
+        EditText test12 = findViewById(R.id.test12);
+        test12.setText(password);
     }
 }

java/ql/test/query-tests/security/CWE-200/semmle/tests/SensitiveTextView/res/layout/Test.xml

@@ -20,4 +20,34 @@
             android:id="@+id/test5"/>
     </LinearLayout>
 
+    <EditText
+        android:id="@+id/test6"
+        android:inputType="textPassword"/>
+
+    <EditText
+        android:id="@+id/test7"
+        android:inputType="textWebPassword"/>
+
+    <EditText
+        android:id="@+id/test8"
+        android:inputType="numberPassword"/>
+
+    <EditText
+        android:id="@+id/test9"
+        android:inputType="textVisiblePassword"/>
+
+    <EditText
+        android:id="@+id/test10"
+        android:visibility="invisible"/>
+
+    <EditText
+        android:id="@+id/test11"
+        android:visibility="gone"/>
+
+    <LinearLayout
+        android:visibility="invisible">
+        <TextView
+            android:id="@+id/test12"/>
+    </LinearLayout>
+
 </LinearLayout>