Add Fragment injection in PreferenceActivity query

Author: atorralba

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

@@ -5,6 +5,32 @@ private import semmle.code.java.frameworks.android.Android
 private import semmle.code.java.frameworks.android.Fragment
 private import semmle.code.java.Reflection
 
+/** The method `isValidFragment` of the class `android.preference.PreferenceActivity`. */
+class IsValidFragmentMethod extends Method {
+  IsValidFragmentMethod() {
+    this.getDeclaringType()
+        .getASupertype*()
+        .hasQualifiedName("android.preference", "PreferenceActivity") and
+    this.hasName("isValidFragment")
+  }
+
+  /**
+   * Holds if this method makes the Activity it is declared in vulnerable to Fragment injection,
+   * that is, all code paths in this method return `true` and the Activity is exported.
+   */
+  predicate isUnsafe() {
+    this.getDeclaringType().(AndroidActivity).isExported() and
+    forex(ReturnStmt retStmt, BooleanLiteral bool |
+      retStmt.getEnclosingCallable() = this and
+      // Using taint tracking to handle logical expressions, like
+      // fragmentName.equals("safe") || true
+      TaintTracking::localExprTaint(bool, retStmt.getResult())
+    |
+      bool.getBooleanValue() = true
+    )
+  }
+}
+
 /**
  * A sink for Fragment injection vulnerabilities,
  * that is, method calls that dynamically add Fragments to Activities.

java/ql/src/Security/CWE/CWE-470/FragmentInjectionInPreferenceActivity.java

@@ -0,0 +1,21 @@
+class UnsafeActivity extends PreferenceActivity {
+
+    @Override
+    protected boolean isValidFragment(String fragmentName) {
+        // BAD: any Fragment name can be provided.
+        return true;
+    }
+}
+
+
+class SafeActivity extends PreferenceActivity {
+    @Override
+    protected boolean isValidFragment(String fragmentName) {
+        // Good: only trusted Fragment names are allowed.
+        return SafeFragment1.class.getName().equals(fragmentName)
+                || SafeFragment2.class.getName().equals(fragmentName)
+                || SafeFragment3.class.getName().equals(fragmentName);
+    }
+
+}
+

java/ql/src/Security/CWE/CWE-470/FragmentInjectionInPreferenceActivity.qhelp

@@ -0,0 +1,4 @@
+<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
+<qhelp>
+<include src="FragmentInjection.inc.qhelp"></include>
+</qhelp>

java/ql/src/Security/CWE/CWE-470/FragmentInjectionInPreferenceActivity.ql

@@ -0,0 +1,21 @@
+/**
+ * @name Android Fragment injection in PreferenceActivity
+ * @description An insecure implementation of the isValidFragment method
+ *              of the PreferenceActivity class may lead to Fragment injection.
+ * @kind problem
+ * @problem.severity error
+ * @security-severity 9.8
+ * @precision high
+ * @id java/android/fragment-injection-preference-activity
+ * @tags security
+ *       external/cwe/cwe-470
+ */
+
+import java
+import semmle.code.java.security.FragmentInjection
+
+from IsValidFragmentMethod m
+where m.isUnsafe()
+select m,
+  "The 'isValidFragment' method always returns true. This makes the exported Activity $@ vulnerable to Fragment Injection.",
+  m.getDeclaringType(), m.getDeclaringType().getName()

java/ql/test/query-tests/security/CWE-470/AndroidManifest.xml

@@ -19,5 +19,8 @@
                 <category android:name="android.intent.category.LAUNCHER" />
             </intent-filter>
         </activity>
+        <activity android:name=".SafePreferenceActivity" android:exported="true" />
+        <activity android:name=".UnsafePreferenceActivity" android:exported="true" />
+        <activity android:name=".UnexportedPreferenceActivity" android:exported="false" />
     </application>
 </manifest>

java/ql/test/query-tests/security/CWE-470/FragmentInjectionInPreferenceActivityTest.expected

@@ -0,0 +1,18 @@
+import java
+import semmle.code.java.security.FragmentInjection
+import TestUtilities.InlineExpectationsTest
+
+class FragmentInjectionInPreferenceActivityTest extends InlineExpectationsTest {
+  FragmentInjectionInPreferenceActivityTest() { this = "FragmentInjectionInPreferenceActivityTest" }
+
+  override string getARelevantTag() { result = "hasPreferenceFragmentInjection" }
+
+  override predicate hasActualResult(Location location, string element, string tag, string value) {
+    tag = "hasPreferenceFragmentInjection" and
+    exists(IsValidFragmentMethod isValidFragment | isValidFragment.isUnsafe() |
+      isValidFragment.getLocation() = location and
+      element = isValidFragment.toString() and
+      value = ""
+    )
+  }
+}

java/ql/test/query-tests/security/CWE-470/FragmentInjectionInPreferenceActivityTest.ql

@@ -0,0 +1,11 @@
+package com.example.myapp;
+
+import android.preference.PreferenceActivity;
+
+public class SafePreferenceActivity extends PreferenceActivity {
+
+    @Override
+    protected boolean isValidFragment(String fragmentName) { // Safe: not all paths return true
+        return fragmentName.equals("MySafeFragment") || fragmentName.equals("MySafeFragment2");
+    }
+}

java/ql/test/query-tests/security/CWE-470/SafePreferenceActivity.java

@@ -0,0 +1,11 @@
+package com.example.myapp;
+
+import android.preference.PreferenceActivity;
+
+public class UnexportedPreferenceActivity extends PreferenceActivity {
+
+    @Override
+    protected boolean isValidFragment(String fragmentName) { // Safe: not exported
+        return true;
+    }
+}

java/ql/test/query-tests/security/CWE-470/UnexportedPreferenceActivity.java

@@ -0,0 +1,11 @@
+package com.example.myapp;
+
+import android.preference.PreferenceActivity;
+
+public class UnsafePreferenceActivity extends PreferenceActivity {
+
+    @Override
+    protected boolean isValidFragment(String fragmentName) { // $ hasPreferenceFragmentInjection
+        return fragmentName.equals("MySafeClass") || true;
+    }
+}