Add Intent URI Permission Manipulation query

java/ql/lib/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll

@@ -176,6 +176,8 @@ private predicate localAdditionalTaintExprStep(Expr src, Expr sink) {
   serializationStep(src, sink)
   or
   formatStep(src, sink)
+  or
+  bitwiseStep(src, sink)
 }
 
 /**
@@ -525,6 +527,9 @@ private class FormatterCallable extends TaintPreservingCallable {
   }
 }
 
+/** Holds if taint may flow from the operand of a bitwise expression to its result. */
+private predicate bitwiseStep(Expr src, BitwiseExpr sink) { sink.(BinaryExpr).getAnOperand() = src }
+
 private import StringBuilderVarModule
 
 module StringBuilderVarModule {

java/ql/lib/semmle/code/java/frameworks/android/Android.qll

@@ -67,6 +67,14 @@ class AndroidActivity extends ExportableAndroidComponent {
   AndroidActivity() { getASuperTypeStar(this).hasQualifiedName("android.app", "Activity") }
 }
 
+/** The method `setResult` of the class `android.app.Activity`. */
+class ActivitySetResultMethod extends Method {
+  ActivitySetResultMethod() {
+    this.getDeclaringType().hasQualifiedName("android.app", "Activity") and
+    this.hasName("setResult")
+  }
+}
+
 /** An Android service. */
 class AndroidService extends ExportableAndroidComponent {
   AndroidService() { getASuperTypeStar(this).hasQualifiedName("android.app", "Service") }

java/ql/lib/semmle/code/java/frameworks/android/Intent.qll

@@ -148,6 +148,31 @@ predicate allowIntentExtrasImplicitRead(DataFlow::Node node, DataFlow::Content c
   )
 }
 
+/**
+ * The fields to grant URI permissions of the class `android.content.Intent`:
+ *
+ * - `Intent.FLAG_GRANT_READ_URI_PERMISSION`
+ * - `Intent.FLAG_GRANT_WRITE_URI_PERMISSION`
+ * - `Intent.FLAG_GRANT_PERSISTABLE_URI_PERMISSION`
+ * - `Intent.FLAG_GRANT_PREFIX_URI_PERMISSION`
+ */
+class GrantUriPermissionFlag extends Field {
+  GrantUriPermissionFlag() {
+    this.getDeclaringType() instanceof TypeIntent and
+    this.getName().matches("FLAG_GRANT_%_URI_PERMISSION")
+  }
+}
+
+/** The field `Intent.FLAG_GRANT_READ_URI_PERMISSION`. */
+class GrantReadUriPermissionFlag extends GrantUriPermissionFlag {
+  GrantReadUriPermissionFlag() { this.hasName("FLAG_GRANT_READ_URI_PERMISSION") }
+}
+
+/** The field `Intent.FLAG_GRANT_WRITE_URI_PERMISSION`. */
+class GrantWriteUriPermissionFlag extends GrantUriPermissionFlag {
+  GrantWriteUriPermissionFlag() { this.hasName("FLAG_GRANT_WRITE_URI_PERMISSION") }
+}
+
 private class IntentBundleFlowSteps extends SummaryModelCsv {
   override predicate row(string row) {
     row =

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

@@ -0,0 +1,101 @@
+/**
+ * Provides classes and predicates to reason about Intent URI permission manipulation
+ * vulnerabilities on Android.
+ */
+
+import java
+private import semmle.code.java.dataflow.DataFlow
+private import semmle.code.java.dataflow.TaintTracking
+private import semmle.code.java.frameworks.android.Android
+private import semmle.code.java.frameworks.android.Intent
+
+/**
+ * A sink for Intent URI permission manipulation vulnerabilities in Android,
+ * that is, method calls that return an Intent as the result of an Activity.
+ */
+abstract class IntentUriPermissionManipulationSink extends DataFlow::Node { }
+
+/**
+ * A sanitizer that makes sure that an Intent is safe to be returned to another Activity.
+ *
+ * Usually, this is done by setting the Intent's data URI and/or its flags to safe values.
+ */
+abstract class IntentUriPermissionManipulationSanitizer extends DataFlow::Node { }
+
+/**
+ * A guard that makes sure that an Intent is safe to be returned to another Activity.
+ *
+ * Usually, this is done by checking that the Intent's data URI and/or its flags contain
+ * expected values.
+ */
+abstract class IntentUriPermissionManipulationGuard extends DataFlow::BarrierGuard { }
+
+private class DefaultIntentUriPermissionManipulationSink extends IntentUriPermissionManipulationSink {
+  DefaultIntentUriPermissionManipulationSink() {
+    exists(MethodAccess ma | ma.getMethod() instanceof ActivitySetResultMethod |
+      ma.getArgument(1) = this.asExpr()
+    )
+  }
+}
+
+private class IntentFlagsOrDataChangedSanitizer extends IntentUriPermissionManipulationSanitizer {
+  IntentFlagsOrDataChangedSanitizer() {
+    exists(MethodAccess ma, Method m |
+      ma.getMethod() = m and
+      m.getDeclaringType() instanceof TypeIntent and
+      this.asExpr() = ma.getQualifier()
+    |
+      m.hasName("removeFlags") and
+      TaintTracking::localExprTaint(any(GrantReadUriPermissionFlag f).getAnAccess(),
+        ma.getArgument(0)) and
+      TaintTracking::localExprTaint(any(GrantWriteUriPermissionFlag f).getAnAccess(),
+        ma.getArgument(0))
+      or
+      ma.getMethod() = m and
+      m.getDeclaringType() instanceof TypeIntent and
+      this.asExpr() = ma.getQualifier() and
+      m.hasName("setFlags") and
+      not TaintTracking::localExprTaint(any(GrantUriPermissionFlag f).getAnAccess(),
+        ma.getArgument(0))
+      or
+      m.hasName("setData")
+    )
+  }
+}
+
+private class IntentFlagsOrDataCheckedGuard extends IntentUriPermissionManipulationGuard {
+  Expr condition;
+
+  IntentFlagsOrDataCheckedGuard() {
+    this.(MethodAccess).getMethod() instanceof EqualsMethod and
+    condition = [this.(MethodAccess).getArgument(0), this.(MethodAccess).getQualifier()]
+    or
+    condition = this.(EqualityTest).getAnOperand().(AndBitwiseExpr)
+  }
+
+  override predicate checks(Expr e, boolean branch) {
+    exists(MethodAccess ma, Method m |
+      ma.getMethod() = m and
+      m.getDeclaringType() instanceof TypeIntent and
+      m.hasName(["getFlags", "getData"]) and
+      TaintTracking::localExprTaint(ma, condition) and
+      ma.getQualifier() = e
+    |
+      bitwiseCheck(branch)
+      or
+      this.(MethodAccess).getMethod() instanceof EqualsMethod and branch = true
+    )
+  }
+
+  /**
+   * Holds if `this` is a bitwise check. `branch` is `true` when the expected value is `0`
+   * and `false` otherwise.
+   */
+  private predicate bitwiseCheck(boolean branch) {
+    exists(CompileTimeConstantExpr operand | operand = this.(EqualityTest).getAnOperand() |
+      if operand.getIntValue() = 0
+      then this.(EqualityTest).polarity() = branch
+      else this.(EqualityTest).polarity().booleanNot() = branch
+    )
+  }
+}

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

@@ -0,0 +1,30 @@
+/**
+ * Provides taint tracking configurations to be used in queries related to Intent URI permission
+ * manipulation in Android.
+ */
+
+import java
+private import semmle.code.java.dataflow.FlowSources
+private import semmle.code.java.dataflow.DataFlow
+private import IntentUriPermissionManipulation
+
+/**
+ * A taint tracking configuration for user-provided Intents being returned to third party apps.
+ */
+class IntentUriPermissionManipulationConf extends TaintTracking::Configuration {
+  IntentUriPermissionManipulationConf() { this = "UriPermissionManipulationConf" }
+
+  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
+
+  override predicate isSink(DataFlow::Node sink) {
+    sink instanceof IntentUriPermissionManipulationSink
+  }
+
+  override predicate isSanitizer(DataFlow::Node barrier) {
+    barrier instanceof IntentUriPermissionManipulationSanitizer
+  }
+
+  override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
+    guard instanceof IntentUriPermissionManipulationGuard
+  }
+}

java/ql/lib/semmle/code/xml/AndroidManifest.qll

@@ -74,6 +74,13 @@ class AndroidReceiverXmlElement extends AndroidComponentXmlElement {
   AndroidReceiverXmlElement() { this.getName() = "receiver" }
 }
 
+/**
+ * An XML attribute with the `android:` prefix.
+ */
+class AndroidXmlAttribute extends XMLAttribute {
+  AndroidXmlAttribute() { this.getNamespace().getPrefix() = "android" }
+}
+
 /**
  * A `<provider>` element in an Android manifest file.
  */
@@ -91,6 +98,14 @@ class AndroidProviderXmlElement extends AndroidComponentXmlElement {
     this.getAnAttribute().(AndroidPermissionXmlAttribute).isWrite() and
     this.getAnAttribute().(AndroidPermissionXmlAttribute).isRead()
   }
+
+  predicate grantsUriPermissions() {
+    exists(AndroidXmlAttribute attr |
+      this.getAnAttribute() = attr and
+      attr.getName() = "grantUriPermissions" and
+      attr.getValue() = "true"
+    )
+  }
 }
 
 /**

java/ql/src/Security/CWE/CWE-266/IntentUriPermissionManipulation.ql

@@ -0,0 +1,24 @@
+/**
+ * @name Intent URI permission manipulation
+ * @description When an externally provided Intent is returned to an Activity via setResult,
+ *              a malicious application could use this to grant itself permissions to access
+ *              arbitrary Content Providers that are accessible by the vulnerable application.
+ * @kind path-problem
+ * @problem.severity error
+ * @precision high
+ * @id java/android/intent-uri-permission-manipulation
+ * @tags security
+ *       external/cwe/cwe-266
+ *       external/cwe/cwe-926
+ */
+
+import java
+import semmle.code.java.security.IntentUriPermissionManipulationQuery
+import semmle.code.java.dataflow.DataFlow
+import DataFlow::PathGraph
+
+from DataFlow::PathNode source, DataFlow::PathNode sink
+where any(IntentUriPermissionManipulationConf c).hasFlowPath(source, sink)
+select sink.getNode(), source, sink,
+  "This Intent can be set with arbitrary flags from $@, " +
+    "and used to give access to internal Content Providers.", source.getNode(), "this user input"

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

@@ -0,0 +1,22 @@
+<manifest xmlns:android="http://schemas.android.com/apk/res/android"
+    package="com.example.app"
+    android:installLocation="auto"
+    android:versionCode="1"
+    android:versionName="0.1" >
+
+    <application
+        android:icon="@drawable/ic_launcher"
+        android:label="@string/app_name"
+        android:theme="@style/AppTheme" >
+        <activity
+            android:name=".MainActivity"
+            android:icon="@drawable/ic_launcher"
+			android:label="@string/app_name">
+            <intent-filter>
+                <action android:name="android.intent.action.MAIN" />
+                <category android:name="android.intent.category.LAUNCHER" />
+            </intent-filter>
+        </activity>
+    </application>
+
+</manifest>

java/ql/test/query-tests/security/CWE-266/IntentUriPermissionManipulationTest.ql

@@ -0,0 +1,11 @@
+import java
+import TestUtilities.InlineFlowTest
+import semmle.code.java.security.IntentUriPermissionManipulationQuery
+
+class IntentUriPermissionManipulationTest extends InlineFlowTest {
+  override DataFlow::Configuration getValueFlowConfig() { none() }
+
+  override TaintTracking::Configuration getTaintFlowConfig() {
+    result instanceof IntentUriPermissionManipulationConf
+  }
+}

java/ql/test/query-tests/security/CWE-266/MainActivity.java

@@ -0,0 +1,85 @@
+package com.example.app;
+
+import android.app.Activity;
+import android.content.Intent;
+import android.net.Uri;
+import android.os.Bundle;
+
+public class MainActivity extends Activity {
+
+    public void onCreate(Bundle savedInstance) {
+        {
+            Intent intent = getIntent();
+            setResult(RESULT_OK, intent); // $ hasTaintFlow
+        }
+        {
+            Intent extraIntent = (Intent) getIntent().getParcelableExtra("extraIntent");
+            setResult(RESULT_OK, extraIntent); // $ hasTaintFlow
+        }
+        {
+            Intent intent = getIntent();
+            intent.setData(Uri.parse("content://safe/uri")); // Sanitizer
+            setResult(RESULT_OK, intent); // Safe
+        }
+        {
+            Intent intent = getIntent();
+            intent.setFlags(0); // Sanitizer
+            setResult(RESULT_OK, intent); // Safe
+        }
+        {
+            Intent intent = getIntent();
+            intent.setFlags( // Not properly sanitized
+                    Intent.FLAG_GRANT_WRITE_URI_PERMISSION | Intent.FLAG_ACTIVITY_CLEAR_TOP);
+            setResult(RESULT_OK, intent); // $ hasTaintFlow
+        }
+        {
+            Intent intent = getIntent();
+            intent.removeFlags( // Sanitizer
+                    Intent.FLAG_GRANT_WRITE_URI_PERMISSION | Intent.FLAG_GRANT_READ_URI_PERMISSION);
+            setResult(RESULT_OK, intent); // Safe
+        }
+        {
+            Intent intent = getIntent();
+            // Combined, the following two calls are a sanitizer
+            intent.removeFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION);
+            intent.removeFlags(Intent.FLAG_GRANT_WRITE_URI_PERMISSION);
+            setResult(RESULT_OK, intent); // $ SPURIOUS: $ hasTaintFlow
+        }
+        {
+            Intent intent = getIntent();
+            intent.removeFlags( // Not properly sanitized
+                    Intent.FLAG_GRANT_WRITE_URI_PERMISSION | Intent.FLAG_ACTIVITY_CLEAR_TOP);
+            setResult(RESULT_OK, intent); // $ hasTaintFlow
+        }
+        {
+            Intent intent = getIntent();
+            // Good check
+            if (intent.getData().equals(Uri.parse("content://safe/uri"))) {
+                setResult(RESULT_OK, intent); // Safe
+            } else {
+                setResult(RESULT_OK, intent); // $ hasTaintFlow
+            }
+        }
+        {
+            Intent intent = getIntent();
+            int flags = intent.getFlags();
+            // Good check
+            if ((flags & Intent.FLAG_GRANT_READ_URI_PERMISSION) == 0
+                    && (flags & Intent.FLAG_GRANT_WRITE_URI_PERMISSION) == 0) {
+                setResult(RESULT_OK, intent); // Safe
+            } else {
+                setResult(RESULT_OK, intent); // $ hasTaintFlow
+            }
+        }
+        {
+            Intent intent = getIntent();
+            int flags = intent.getFlags();
+            // Insufficient check
+            if ((flags & Intent.FLAG_GRANT_READ_URI_PERMISSION) == 0) {
+                setResult(RESULT_OK, intent); // $ MISSING: $ hasTaintFlow
+            } else {
+                setResult(RESULT_OK, intent); // $ hasTaintFlow
+            }
+        }
+    }
+}

java/ql/test/query-tests/security/CWE-266/options

@@ -0,0 +1 @@
+//semmle-extractor-options: --javac-args -cp ${testdir}/../../../stubs/google-android-9.0.0