diff --git a/java/change-notes/2021-10-20-android-fragment-injection-query.md b/java/change-notes/2021-10-20-android-fragment-injection-query.md index 14623e17854..51c757b2386 100644 --- a/java/change-notes/2021-10-20-android-fragment-injection-query.md +++ b/java/change-notes/2021-10-20-android-fragment-injection-query.md @@ -1,3 +1,3 @@ lgtm,codescanning -* Two new queries, "Android Fragment injection" (`java/android/fragment-injection`) and "Android Fragment injection in PreferenceActivity" (`java/android/fragment-injection-preference-activity`) have been added. -These queries find exported Android Activities that instantiate and host Fragments created from user-provided data, which can lead to access control bypass and exposes the application to unintended effects. \ No newline at end of file +* Two new queries, "Android fragment injection" (`java/android/fragment-injection`) and "Android fragment injection in PreferenceActivity" (`java/android/fragment-injection-preference-activity`) have been added. +These queries find exported Android activities that instantiate and host fragments created from user-provided data. Such activities are vulnerable to access control bypass and expose the Android application to unintended effects. \ No newline at end of file diff --git a/java/ql/lib/semmle/code/java/frameworks/android/Fragment.qll b/java/ql/lib/semmle/code/java/frameworks/android/Fragment.qll index b146a151d2c..528162fe4d4 100644 --- a/java/ql/lib/semmle/code/java/frameworks/android/Fragment.qll +++ b/java/ql/lib/semmle/code/java/frameworks/android/Fragment.qll @@ -1,4 +1,4 @@ -/** Provides classes and predicates related to Android Fragments. */ +/** Provides classes and predicates to track Android fragments. */ import java diff --git a/java/ql/lib/semmle/code/java/security/FragmentInjection.qll b/java/ql/lib/semmle/code/java/security/FragmentInjection.qll index e834e539ebe..e7bc7480618 100644 --- a/java/ql/lib/semmle/code/java/security/FragmentInjection.qll +++ b/java/ql/lib/semmle/code/java/security/FragmentInjection.qll @@ -35,7 +35,7 @@ class IsValidFragmentMethod extends Method { /** * A sink for Fragment injection vulnerabilities, - * that is, method calls that dynamically add Fragments to Activities. + * that is, method calls that dynamically add fragments to activities. */ abstract class FragmentInjectionSink extends DataFlow::Node { } diff --git a/java/ql/lib/semmle/code/java/security/FragmentInjectionQuery.qll b/java/ql/lib/semmle/code/java/security/FragmentInjectionQuery.qll index 2e57a02f6b7..df14504bae4 100644 --- a/java/ql/lib/semmle/code/java/security/FragmentInjectionQuery.qll +++ b/java/ql/lib/semmle/code/java/security/FragmentInjectionQuery.qll @@ -7,7 +7,7 @@ import semmle.code.java.security.FragmentInjection /** * A taint-tracking configuration for unsafe user input - * that is used to create Android Fragments dinamically. + * that is used to create Android fragments dynamically. */ class FragmentInjectionTaintConf extends TaintTracking::Configuration { FragmentInjectionTaintConf() { this = "FragmentInjectionTaintConf" } diff --git a/java/ql/src/Security/CWE/CWE-470/FragmentInjection.inc.qhelp b/java/ql/src/Security/CWE/CWE-470/FragmentInjection.inc.qhelp index 334a5999699..e4c69ea1605 100644 --- a/java/ql/src/Security/CWE/CWE-470/FragmentInjection.inc.qhelp +++ b/java/ql/src/Security/CWE/CWE-470/FragmentInjection.inc.qhelp @@ -3,43 +3,43 @@

-Fragments are reusable parts of an Android application's user interface. -Even though a Fragment controls its own lifecycle and layout and handles its input events, -it cannot exist on its own: it must be hosted either by an Activity or another fragment. -This means that a Fragment will be accessible by third-party applications (that is, exported) -only if its hosting Activity is itself exported. +When fragments are instantiated with externally provided names, this exposes any exported activity that dynamically +creates and hosts the fragment to fragment injection. A malicious application could provide the +name of an arbitrary fragment, even one not designed to be externally accessible, and inject it into the activity. +Thus, effectively bypassing access controls and exposing the application to unintended effects.

-If an exported Activity dinamically creates and hosts Fragments instantiated with externally -provided names, a malicious application could provide the name of an arbitrary Fragment, even -one not designed to be externally accessible, and inject it into the Activity, effectively -bypassing access controls and exposing the application to unintended effects. +Fragments are reusable parts of an Android application's user interface. +Even though a fragment controls its own lifecycle and layout, and handles its input events, +it cannot exist on its own: it must be hosted either by an activity or another fragment. +This means that, normally, a fragment will be accessible by third-party applications (that is, exported) +only if its hosting activity is itself exported.

-In general, do not instantiate classes (including Fragments) with user-provided names -without proper validation. +In general, do not instantiate classes (including fragments) with user-provided names +unless the name has been properly validated. -Also, if an exported Activity is extending the PreferenceActivity class, make sure that +Also, if an exported activity is extending the PreferenceActivity class, make sure that the isValidFragment method is overriden and only returns true when the provided -fragmentName points to an intended Fragment. +fragmentName points to an intended fragment.

The following example shows two cases: in the first one, untrusted data is used to instantiate and -add a Fragment to an Activity, while in the second one, a Fragment is safely added with a static name. +add a fragment to an activity, while in the second one, a fragment is safely added with a static name.

-The next example shows two Activities extending PreferenceActivity. The first one overrides -isValidFragment, but it wrongly returns true inconditionally. The second Activity -correctly overrides isValidFragment to only return true when fragmentName -is a trusted Fragment name. +The next example shows two activities that extend PreferenceActivity. The first activity overrides +isValidFragment, but it wrongly returns true unconditionally. The second activity +correctly overrides isValidFragment so that it only returns true when fragmentName +is a trusted fragment name.

diff --git a/java/ql/src/Security/CWE/CWE-470/FragmentInjection.java b/java/ql/src/Security/CWE/CWE-470/FragmentInjection.java index 4e750fe3957..7a91ce67b03 100644 --- a/java/ql/src/Security/CWE/CWE-470/FragmentInjection.java +++ b/java/ql/src/Security/CWE/CWE-470/FragmentInjection.java @@ -4,7 +4,7 @@ public class MyActivity extends FragmentActivity { protected void onCreate(Bundle savedInstance) { try { super.onCreate(savedInstance); - // BAD: Fragment instantiated from user input + // BAD: Fragment instantiated from user input without validation { String fName = getIntent().getStringExtra("fragmentName"); getFragmentManager().beginTransaction().replace(com.android.internal.R.id.prefs, diff --git a/java/ql/src/Security/CWE/CWE-470/FragmentInjection.ql b/java/ql/src/Security/CWE/CWE-470/FragmentInjection.ql index 0b4c376b013..e1636c850b1 100644 --- a/java/ql/src/Security/CWE/CWE-470/FragmentInjection.ql +++ b/java/ql/src/Security/CWE/CWE-470/FragmentInjection.ql @@ -1,7 +1,7 @@ /** - * @name Android Fragment injection - * @description Instantiating an Android Fragment from a user-provided value - * may lead to Fragment injection. + * @name Android fragment injection + * @description Instantiating an Android fragment from a user-provided value + * may allow a malicious application to bypass access controls, exposing the application to unintended effects. * @kind path-problem * @problem.severity error * @security-severity 9.8 diff --git a/java/ql/src/Security/CWE/CWE-470/FragmentInjectionInPreferenceActivity.ql b/java/ql/src/Security/CWE/CWE-470/FragmentInjectionInPreferenceActivity.ql index 5d76d14cbae..a75f0b3eca5 100644 --- a/java/ql/src/Security/CWE/CWE-470/FragmentInjectionInPreferenceActivity.ql +++ b/java/ql/src/Security/CWE/CWE-470/FragmentInjectionInPreferenceActivity.ql @@ -1,7 +1,8 @@ /** - * @name Android Fragment injection in PreferenceActivity - * @description An insecure implementation of the isValidFragment method - * of the PreferenceActivity class may lead to Fragment injection. + * @name Android fragment injection in PreferenceActivity + * @description An insecure implementation of the 'isValidFragment' method + * of the 'PreferenceActivity' class may allow a malicious application to bypass access controls, + * exposing the application to unintended effects. * @kind problem * @problem.severity error * @security-severity 9.8