clean up android query and tests

Author: Jami Cogswell

java/ql/src/Security/CWE/CWE-489/DebuggableAttributeTrue.qhelp

@@ -4,35 +4,31 @@
 <qhelp>
 
 <overview>
-<p>The <code>debuggable</code> attribute in the application section of the AndroidManifest.xml file should
-never be enabled in production builds.</p>
-
-<p>ADD MORE/EDIT?</p>
+<p>When a debugger is enabled it could allow for entry points in the application or reveal sensitive information.</p>
 
 </overview>
 <recommendation>
 
-<p>Make sure that the <code>debuggable</code> attribute is set to false in production builds.</p>
+<p>In Android applications either set the <code>android:debuggable</code> attribute to <code>false</code>
+or do not include it in the manifest. The default value when not included is <code>false</code>.</p>
 
 </recommendation>
 <example>
 
-<p>In the example below, the <code>debuggable</code> attribute is set to <code>true</code>.</p>
+<p>In the example below, the <code>android:debuggable</code> attribute is set to <code>true</code>.</p>
 
+<sample src="DebuggableTrue.xml" />
 
-<p>The corrected version sets the <code>debuggable</code> attribute to <code>false</code>.</p>
+<p>The corrected version sets the <code>android:debuggable</code> attribute to <code>false</code>.</p>
 
+<sample src="DebuggableFalse.xml" />
 
 </example>
 <references>
 
 <li>
-  Java SE Documentation:
-  <a href="https://www.oracle.com/java/technologies/javase/codeconventions-statements.html#15395">Compound Statements</a>.
-</li>
-<li>
-  Wikipedia:
-  <a href="https://en.wikipedia.org/wiki/Indentation_style">Indentation style</a>.
+  Android Developers:
+  <a href="https://developer.android.com/guide/topics/manifest/application-element#debug">The android:debuggable attribute</a>.
 </li>
 
 </references>

java/ql/src/Security/CWE/CWE-489/DebuggableAttributeTrue.ql

@@ -1,13 +1,13 @@
 /**
- * @name Debuggable attribute enabled
+ * @name Android debuggable attribute enabled
  * @description An enabled debugger can allow for entry points in the application or reveal sensitive information.
  * @kind problem
  * @problem.severity warning
  * @id java/android/debuggable-attribute-enabled
  * @tags security
  *       external/cwe/cwe-489
  * @precision very-high
- * @security-severity 0.1
+ * @security-severity
  */
 
 import java

java/ql/src/Security/CWE/CWE-489/DebuggableFalse.xml

@@ -0,0 +1,8 @@
+<manifest ... >
+    <!-- GOOD: 'android:debuggable' set to false -->
+    <application
+        android:debuggable="false">
+        <activity ... >
+        </activity>
+    </application>
+</manifest>

java/ql/src/Security/CWE/CWE-489/DebuggableTrue.xml

@@ -0,0 +1,8 @@
+<manifest ... >
+    <!-- BAD: 'android:debuggable' set to true -->
+    <application
+        android:debuggable="true">
+        <activity ... >
+        </activity>
+    </application>
+</manifest>

java/ql/test/query-tests/security/CWE-489/DebuggableAttributeTrue.expected

@@ -1 +1 @@
-| TestTrue.xml:7:5:17:30 | debuggable=true | Warning: debuggable attribute enabled |
+| TestTrue.xml:7:5:17:30 | debuggable=true | The 'debuggable' attribute is enabled. |

java/ql/test/query-tests/security/CWE-489/Test.java

@@ -1,2 +1,39 @@
-// No need for Java code since only testing XML files
-public class Test { }
+package com.example.myapp;
+
+import android.app.Fragment;
+import android.os.Bundle;
+import android.view.LayoutInflater;
+import android.view.View;
+import android.view.ViewGroup;
+import android.widget.Button;
+import androidx.fragment.app.FragmentActivity;
+import androidx.fragment.app.FragmentTransaction;
+
+public class Test extends FragmentActivity {
+
+    @Override
+    public void onCreate(Bundle savedInstance) {
+        try {
+            super.onCreate(savedInstance);
+            final String fname = getIntent().getStringExtra("fname");
+            FragmentTransaction ft = getSupportFragmentManager().beginTransaction();
+            Class<Fragment> fClass = (Class<Fragment>) Class.forName(fname);
+            ft.add(fClass.newInstance(), ""); // hasTaintFlow
+            ft.add(0, Fragment.instantiate(this, fname), null); // hasTaintFlow
+            ft.add(0, Fragment.instantiate(this, fname, null)); // hasTaintFlow
+            ft.add(0, fClass, null, ""); // hasTaintFlow
+            ft.add(0, fClass.newInstance(), ""); // hasTaintFlow
+            ft.attach(fClass.newInstance()); // hasTaintFlow
+            ft.replace(0, fClass, null); // hasTaintFlow
+            ft.replace(0, fClass.newInstance()); // hasTaintFlow
+            ft.replace(0, fClass, null, ""); // hasTaintFlow
+            ft.replace(0, fClass.newInstance(), ""); // hasTaintFlow
+
+            ft.add(Fragment.class.newInstance(), ""); // Safe
+            ft.attach(Fragment.class.newInstance()); // Safe
+            ft.replace(0, Fragment.class.newInstance(), ""); // Safe
+        } catch (Exception e) {
+        }
+    }
+
+}

java/ql/test/query-tests/security/CWE-489/TestFalse.xml

@@ -3,7 +3,7 @@
     xmlns:tools="http://schemas.android.com/tools"
     package="com.example.happybirthday">
 
-    <!-- Safe: 'debuggable' set to false -->
+    <!-- Safe: 'android:debuggable' set to false -->
     <application
         android:debuggable="false"
         android:allowBackup="true"

java/ql/test/query-tests/security/CWE-489/TestNotSet.xml

@@ -3,7 +3,7 @@
     xmlns:tools="http://schemas.android.com/tools"
     package="com.example.happybirthday">
 
-    <!-- Safe: 'debuggable' not set at all -->
+    <!-- Safe: 'android:debuggable' not set at all -->
     <application
         android:allowBackup="true"
         android:dataExtractionRules="@xml/data_extraction_rules"

java/ql/test/query-tests/security/CWE-489/TestTrue.xml

@@ -3,7 +3,7 @@
     xmlns:tools="http://schemas.android.com/tools"
     package="com.example.happybirthday">
 
-    <!-- Not Safe: 'debuggable' set to true -->
+    <!-- Not Safe: 'android:debuggable' set to true -->
     <application
         android:debuggable="true"
         android:allowBackup="true"