Optimize the query

2025-12-21 19:26:31 +01:00 · 2021-09-10 15:04:29 +00:00
parent 0621e65827
commit 02bfa1ca57
4 changed files with 39 additions and 32 deletions
--- a/java/ql/src/experimental/Security/CWE/CWE-200/AndroidFileIntentSink.qll
+++ b/java/ql/src/experimental/Security/CWE/CWE-200/AndroidFileIntentSink.qll
@@ -19,24 +19,30 @@ class AsyncTask extends RefType {
  AsyncTask() { this.hasQualifiedName("android.os", "AsyncTask") }
 }

+/** The method that executes `AsyncTask` of Android. */
+abstract class ExecuteAsyncTaskMethod extends Method {
+  /** Returns index of the parameter that is tainted. */
+  abstract int getParamIndex();
+}
+
 /** The `execute` method of Android `AsyncTask`. */
-class AsyncTaskExecuteMethod extends Method {
+class AsyncTaskExecuteMethod extends ExecuteAsyncTaskMethod {
  AsyncTaskExecuteMethod() {
    this.getDeclaringType().getSourceDeclaration().getASourceSupertype*() instanceof AsyncTask and
    this.getName() = "execute"
  }

-  int getParamIndex() { result = 0 }
+  override int getParamIndex() { result = 0 }
 }

 /** The `executeOnExecutor` method of Android `AsyncTask`. */
-class AsyncTaskExecuteOnExecutorMethod extends Method {
+class AsyncTaskExecuteOnExecutorMethod extends ExecuteAsyncTaskMethod {
  AsyncTaskExecuteOnExecutorMethod() {
    this.getDeclaringType().getSourceDeclaration().getASourceSupertype*() instanceof AsyncTask and
    this.getName() = "executeOnExecutor"
  }

-  int getParamIndex() { result = 1 }
+  override int getParamIndex() { result = 1 }
 }

 /** The `doInBackground` method of Android `AsyncTask`. */
--- a/java/ql/src/experimental/Security/CWE/CWE-200/AndroidFileIntentSource.qll
+++ b/java/ql/src/experimental/Security/CWE/CWE-200/AndroidFileIntentSource.qll
@@ -15,7 +15,7 @@ class StartActivityForResultMethod extends Method {
 /** Android class instance of `GET_CONTENT` intent. */
 class GetContentIntent extends ClassInstanceExpr {
  GetContentIntent() {
-    this.getConstructedType().getASupertype*() instanceof TypeIntent and
+    this.getConstructedType() instanceof TypeIntent and
    this.getArgument(0).(CompileTimeConstantExpr).getStringValue() =
      "android.intent.action.GET_CONTENT"
    or
--- a/java/ql/src/experimental/Security/CWE/CWE-200/SensitiveAndroidFileLeak.ql
+++ b/java/ql/src/experimental/Security/CWE/CWE-200/SensitiveAndroidFileLeak.ql
@@ -3,7 +3,7 @@
 * @description Getting file intent from user input without path validation could leak arbitrary
 *              Android configuration file and sensitive user data.
 * @kind path-problem
- * @id java/sensitive_android_file_leak
+ * @id java/sensitive-android-file-leak
 * @tags security
 *       external/cwe/cwe-200
 */
@@ -14,6 +14,19 @@ import AndroidFileIntentSource
 import DataFlow2::PathGraph
 import semmle.code.java.dataflow.TaintTracking2

+private class StartsWithSanitizer extends DataFlow2::BarrierGuard {
+  StartsWithSanitizer() { this.(MethodAccess).getMethod().hasName("startsWith") }
+
+  override predicate checks(Expr e, boolean branch) {
+    e =
+      [
+        this.(MethodAccess).getQualifier(),
+        this.(MethodAccess).getQualifier().(MethodAccess).getQualifier()
+      ] and
+    branch = false
+  }
+}
+
 class AndroidFileLeakConfig extends TaintTracking2::Configuration {
  AndroidFileLeakConfig() { this = "AndroidFileLeakConfig" }

@@ -38,37 +51,23 @@ class AndroidFileLeakConfig extends TaintTracking2::Configuration {
    exists(MethodAccess aema, AsyncTaskRunInBackgroundMethod arm |
      // fileAsyncTask.execute(params) will invoke doInBackground(params) of FileAsyncTask
      aema.getQualifier().getType() = arm.getDeclaringType() and
-      (
-        aema.getMethod() instanceof AsyncTaskExecuteMethod and
-        prev.asExpr() = aema.getArgument(0)
-        or
-        aema.getMethod() instanceof AsyncTaskExecuteOnExecutorMethod and
-        prev.asExpr() = aema.getArgument(1)
-      ) and
-      succ.asExpr() = arm.getParameter(0).getAnAccess()
+      aema.getMethod() instanceof ExecuteAsyncTaskMethod and
+      prev.asExpr() = aema.getArgument(aema.getMethod().(ExecuteAsyncTaskMethod).getParamIndex()) and
+      succ.asParameter() = arm.getParameter(0)
    )
    or
    exists(MethodAccess csma, ServiceOnStartCommandMethod ssm, ClassInstanceExpr ce |
      csma.getMethod() instanceof ContextStartServiceMethod and
      ce.getConstructedType() instanceof TypeIntent and // Intent intent = new Intent(context, FileUploader.class);
-      ce.getArgument(1).getType().(ParameterizedType).getTypeArgument(0) = ssm.getDeclaringType() and
+      ce.getArgument(1).(TypeLiteral).getReferencedType() = ssm.getDeclaringType() and
      DataFlow2::localExprFlow(ce, csma.getArgument(0)) and // context.startService(intent);
      prev.asExpr() = csma.getArgument(0) and
-      succ.asExpr() = ssm.getParameter(0).getAnAccess() // public int onStartCommand(Intent intent, int flags, int startId) {...} in FileUploader
+      succ.asParameter() = ssm.getParameter(0) // public int onStartCommand(Intent intent, int flags, int startId) {...} in FileUploader
    )
  }

-  override predicate isSanitizer(DataFlow2::Node node) {
-    exists(
-      MethodAccess startsWith // "startsWith" path check
-    |
-      startsWith.getMethod().hasName("startsWith") and
-      (
-        DataFlow2::localExprFlow(node.asExpr(), startsWith.getQualifier()) or
-        DataFlow2::localExprFlow(node.asExpr(),
-          startsWith.getQualifier().(MethodAccess).getQualifier())
-      )
-    )
+  override predicate isSanitizerGuard(DataFlow2::BarrierGuard guard) {
+    guard instanceof StartsWithSanitizer
  }
 }

--- a/java/ql/test/experimental/query-tests/security/CWE-200/SensitiveAndroidFileLeak.expected
+++ b/java/ql/test/experimental/query-tests/security/CWE-200/SensitiveAndroidFileLeak.expected
@@ -1,26 +1,28 @@
 edges
+| FileService.java:20:31:20:43 | intent : Intent | FileService.java:21:28:21:33 | intent : Intent |
+| FileService.java:20:31:20:43 | intent : Intent | FileService.java:25:42:25:50 | localPath : String |
 | FileService.java:21:28:21:33 | intent : Intent | FileService.java:21:28:21:64 | getStringExtra(...) : String |
-| FileService.java:21:28:21:33 | intent : Intent | FileService.java:25:42:25:50 | localPath : String |
 | FileService.java:21:28:21:64 | getStringExtra(...) : String | FileService.java:25:42:25:50 | localPath : String |
-| FileService.java:25:13:25:51 | makeParamsToExecute(...) : Object[] | FileService.java:44:44:44:49 | params : Object[] |
+| FileService.java:25:13:25:51 | makeParamsToExecute(...) : Object[] | FileService.java:40:41:40:55 | params : Object[] |
 | FileService.java:25:13:25:51 | makeParamsToExecute(...) [[]] : String | FileService.java:25:13:25:51 | makeParamsToExecute(...) : Object[] |
 | FileService.java:25:42:25:50 | localPath : String | FileService.java:25:13:25:51 | makeParamsToExecute(...) [[]] : String |
+| FileService.java:40:41:40:55 | params : Object[] | FileService.java:44:33:44:52 | (...)... : Object |
 | FileService.java:44:33:44:52 | (...)... : Object | FileService.java:45:53:45:59 | ...[...] |
-| FileService.java:44:44:44:49 | params : Object[] | FileService.java:44:33:44:52 | (...)... : Object |
-| LeakFileActivity2.java:16:26:16:31 | intent : Intent | FileService.java:21:28:21:33 | intent : Intent |
+| LeakFileActivity2.java:16:26:16:31 | intent : Intent | FileService.java:20:31:20:43 | intent : Intent |
 | LeakFileActivity.java:14:35:14:38 | data : Intent | LeakFileActivity.java:18:40:18:59 | contentIntent : Intent |
 | LeakFileActivity.java:18:40:18:59 | contentIntent : Intent | LeakFileActivity.java:19:31:19:43 | contentIntent : Intent |
 | LeakFileActivity.java:19:31:19:43 | contentIntent : Intent | LeakFileActivity.java:19:31:19:53 | getData(...) : Uri |
 | LeakFileActivity.java:19:31:19:53 | getData(...) : Uri | LeakFileActivity.java:21:58:21:72 | streamsToUpload : Uri |
 | LeakFileActivity.java:21:58:21:72 | streamsToUpload : Uri | LeakFileActivity.java:21:58:21:82 | getPath(...) |
 nodes
+| FileService.java:20:31:20:43 | intent : Intent | semmle.label | intent : Intent |
 | FileService.java:21:28:21:33 | intent : Intent | semmle.label | intent : Intent |
 | FileService.java:21:28:21:64 | getStringExtra(...) : String | semmle.label | getStringExtra(...) : String |
 | FileService.java:25:13:25:51 | makeParamsToExecute(...) : Object[] | semmle.label | makeParamsToExecute(...) : Object[] |
 | FileService.java:25:13:25:51 | makeParamsToExecute(...) [[]] : String | semmle.label | makeParamsToExecute(...) [[]] : String |
 | FileService.java:25:42:25:50 | localPath : String | semmle.label | localPath : String |
+| FileService.java:40:41:40:55 | params : Object[] | semmle.label | params : Object[] |
 | FileService.java:44:33:44:52 | (...)... : Object | semmle.label | (...)... : Object |
-| FileService.java:44:44:44:49 | params : Object[] | semmle.label | params : Object[] |
 | FileService.java:45:53:45:59 | ...[...] | semmle.label | ...[...] |
 | LeakFileActivity2.java:16:26:16:31 | intent : Intent | semmle.label | intent : Intent |
 | LeakFileActivity.java:14:35:14:38 | data : Intent | semmle.label | data : Intent |