Copyedit

Author: smowton

java/ql/src/experimental/Security/CWE/CWE-200/AndroidFileIntentSink.qll

@@ -19,7 +19,7 @@ class AsyncTask extends RefType {
   AsyncTask() { this.hasQualifiedName("android.os", "AsyncTask") }
 }
 
-/** The `execute` or `executeOnExecutor` method of Android `AsyncTask`. */
+/** The `execute` or `executeOnExecutor` method of Android's `AsyncTask` class. */
 class ExecuteAsyncTaskMethod extends Method {
   int paramIndex;
 
@@ -35,23 +35,23 @@ class ExecuteAsyncTaskMethod extends Method {
   int getParamIndex() { result = paramIndex }
 }
 
-/** The `doInBackground` method of Android `AsyncTask`. */
+/** The `doInBackground` method of Android's `AsyncTask` class. */
 class AsyncTaskRunInBackgroundMethod extends Method {
   AsyncTaskRunInBackgroundMethod() {
     this.getDeclaringType().getSourceDeclaration().getASourceSupertype*() instanceof AsyncTask and
     this.getName() = "doInBackground"
   }
 }
 
-/** The service start method of Android context. */
+/** The service start method of Android's `Context` class. */
 class ContextStartServiceMethod extends Method {
   ContextStartServiceMethod() {
     this.getName() = ["startService", "startForegroundService"] and
     this.getDeclaringType().getASupertype*() instanceof TypeContext
   }
 }
 
-/** The `onStartCommand` method of Android service. */
+/** The `onStartCommand` method of Android's `Service` class. */
 class ServiceOnStartCommandMethod extends Method {
   ServiceOnStartCommandMethod() {
     this.hasName("onStartCommand") and

java/ql/src/experimental/Security/CWE/CWE-200/AndroidFileIntentSource.qll

@@ -5,15 +5,15 @@ import semmle.code.java.dataflow.FlowSources
 import semmle.code.java.dataflow.TaintTracking2
 import semmle.code.java.frameworks.android.Android
 
-/** The `startActivityForResult` method of Android `Activity`. */
+/** The `startActivityForResult` method of Android's `Activity` class. */
 class StartActivityForResultMethod extends Method {
   StartActivityForResultMethod() {
     this.getDeclaringType().getASupertype*() instanceof AndroidActivity and
     this.getName() = "startActivityForResult"
   }
 }
 
-/** Android class instance of `GET_CONTENT` intent. */
+/** An instance of `android.content.Intent` constructed passing `GET_CONTENT` to the constructor. */
 class GetContentIntent extends ClassInstanceExpr {
   GetContentIntent() {
     this.getConstructedType() instanceof TypeIntent and
@@ -28,7 +28,7 @@ class GetContentIntent extends ClassInstanceExpr {
   }
 }
 
-/** Taint configuration for getting content intent. */
+/** Taint configuration that identifies `GET_CONTENT` `Intent` instances passed to `startActivityForResult`. */
 class GetContentIntentConfig extends TaintTracking2::Configuration {
   GetContentIntentConfig() { this = "GetContentIntentConfig" }
 
@@ -56,8 +56,8 @@ class GetContentIntentConfig extends TaintTracking2::Configuration {
   }
 }
 
-/** Android `Intent` input to request file loading. */
-class AndroidFileIntentInput extends LocalUserInput {
+/** A `GET_CONTENT` `Intent` instances that is passed to `startActivityForResult`. */
+class AndroidFileIntentInput extends DataFlow::Node {
   MethodAccess ma;
 
   AndroidFileIntentInput() {
@@ -68,7 +68,7 @@ class AndroidFileIntentInput extends LocalUserInput {
     )
   }
 
-  /** The request code identifying a specific intent, which is to be matched in `onActivityResult()`. */
+  /** The request code passed to `startActivityForResult`, which is to be matched in `onActivityResult()`. */
   int getRequestCode() { result = ma.getArgument(1).(CompileTimeConstantExpr).getIntValue() }
 }
 
@@ -79,13 +79,3 @@ class OnActivityForResultMethod extends Method {
     this.getName() = "onActivityResult"
   }
 }
-
-/** Input of Android activity result from the same application or another application. */
-class AndroidActivityResultInput extends DataFlow::Node {
-  OnActivityForResultMethod m;
-
-  AndroidActivityResultInput() { this.asExpr() = m.getParameter(2).getAnAccess() }
-
-  /** The request code matching a specific intent request. */
-  VarAccess getRequestCodeVar() { result = m.getParameter(0).getAnAccess() }
-}

java/ql/src/experimental/Security/CWE/CWE-200/SensitiveAndroidFileLeak.qhelp

@@ -5,22 +5,22 @@
 <overview>
 <p>The Android API allows to start an activity in another mobile application and receive a result back.
 When starting an activity to retrieve a file from another application, missing input validation can
-lead to leaking of sensitive configuration file or user data because the intent is from the application
-itself that is allowed to access its protected data therefore bypassing the access control.
+lead to leaking of sensitive configuration file or user data because the intent could refer to paths
+which are accessible to the receiver application, but are intended to be application-private.
 </p>
 </overview>
 
 <recommendation>
 <p>
-When loading file data from an activity of another application, validate that the file path is not its own
+When loading file data from an activity of another application, validate that the file path is not the receiver's
 protected directory, which is a subdirectory of the Android application directory <code>/data/data/</code>.
 </p>
 </recommendation>
 
 <example>
 <p>
-The following examples show the bad situation and the good situation respectively. In bad situation, a
-file is loaded without path validation. In good situation, a file is loaded with path validation.
+The following examples show a bad situation and a good situation respectively. In the bad situation, a
+file is loaded without path validation. In the good situation, a file is loaded with path validation.
 </p>
 <sample src="LoadFileFromAppActivity.java" />
 </example>
@@ -33,6 +33,6 @@ Google:
 <li>
 CVE:
 <a href="https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2021-32695">CVE-2021-32695: File Sharing Flow Initiated by a Victim Leaks Sensitive Data to a Malicious App</a>.
-</li>  
+</li>
 </references>
 </qhelp>

java/ql/src/experimental/Security/CWE/CWE-200/SensitiveAndroidFileLeak.ql

@@ -1,6 +1,6 @@
 /**
  * @name Leaking sensitive Android file
- * @description Getting file intent from user input without path validation could leak arbitrary
+ * @description Using a path specified in an Android Intent without validation could leak arbitrary
  *              Android configuration file and sensitive user data.
  * @kind path-problem
  * @id java/sensitive-android-file-leak
@@ -35,18 +35,17 @@ class AndroidFileLeakConfig extends TaintTracking::Configuration {
 
   /**
    * Holds if `src` is a read of some Intent-typed variable guarded by a check like
-   * `requestCode == REQUEST_CODE__SELECT_CONTENT_FROM_APPS`, where `requestCode` is the first
-   * argument to `Activity.onActivityResult` and `REQUEST_CODE__SELECT_CONTENT_FROM_APPS` is
-   * any request code in a call to `startActivityForResult(intent, code)`.
+   * `requestCode == someCode`, where `requestCode` is the first
+   * argument to `Activity.onActivityResult` and `someCode` is
+   * any request code used in a call to `startActivityForResult(intent, someCode)`.
    */
   override predicate isSource(DataFlow::Node src) {
     exists(
-      AndroidActivityResultInput ai, AndroidFileIntentInput fi, ConditionBlock cb, EQExpr ee,
-      CompileTimeConstantExpr cc, VarAccess intentVar
+      OnActivityForResultMethod oafr, ConditionBlock cb, CompileTimeConstantExpr cc,
+      VarAccess intentVar
     |
-      cb.getCondition() = ee and
-      ee.hasOperands(ai.getRequestCodeVar(), cc) and
-      cc.getIntValue() = fi.getRequestCode() and
+      cb.getCondition().(EQExpr).hasOperands(oafr.getParameter(0).getAnAccess(), cc) and
+      cc.getIntValue() = any(AndroidFileIntentInput fi).getRequestCode() and
       intentVar.getType() instanceof TypeIntent and
       cb.controls(intentVar.getBasicBlock(), true) and
       src.asExpr() = intentVar