summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJeff Sharkey <jsharkey@android.com>2016-11-16 17:22:48 -0700
committergitbuildkicker <android-build@google.com>2017-01-24 16:41:14 -0800
commitcefaefa4f447a51a717e7cca750461d3f19b68fc (patch)
tree8a69ed4e23c890e6a7f025348154ac43075b2e2c
parentdb66afe61e52f81f185593a2b6dd63c436ce9014 (diff)
downloadbase-cefaefa4f447a51a717e7cca750461d3f19b68fc.tar.gz
DO NOT MERGE: Check provider access for content changes.
For an app to either send or receive content change notifications, require that they have some level of access to the underlying provider. Without these checks, a malicious app could sniff sensitive user data from the notifications of otherwise private providers. Test: builds, boots, PoC app now fails Bug: 32555637 Change-Id: If2dcd45cb0a9f1fb3b93e39fc7b8ae9c34c2fdef (cherry picked from commit ff2fede0ddd9464d4c65e6218a032036adb73099)
-rw-r--r--core/java/android/app/ActivityManagerInternal.java5
-rw-r--r--services/core/java/com/android/server/am/ActivityManagerService.java42
-rw-r--r--services/core/java/com/android/server/content/ContentService.java79
3 files changed, 95 insertions, 31 deletions
diff --git a/core/java/android/app/ActivityManagerInternal.java b/core/java/android/app/ActivityManagerInternal.java
index 40eb799f26b9..579b307efac6 100644
--- a/core/java/android/app/ActivityManagerInternal.java
+++ b/core/java/android/app/ActivityManagerInternal.java
@@ -25,6 +25,11 @@ import android.content.ComponentName;
* @hide Only for use within the system server.
*/
public abstract class ActivityManagerInternal {
+ /**
+ * Verify that calling app has access to the given provider.
+ */
+ public abstract String checkContentProviderAccess(String authority, int userId);
+
// Called by the power manager.
public abstract void onWakefulnessChanged(int wakefulness);
diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java
index 3307418645ea..a093bf56e2ce 100644
--- a/services/core/java/com/android/server/am/ActivityManagerService.java
+++ b/services/core/java/com/android/server/am/ActivityManagerService.java
@@ -9303,6 +9303,43 @@ public final class ActivityManagerService extends ActivityManagerNative
}
/**
+ * Check if the calling UID has a possible chance at accessing the provider
+ * at the given authority and user.
+ */
+ public String checkContentProviderAccess(String authority, int userId) {
+ if (userId == UserHandle.USER_ALL) {
+ mContext.enforceCallingOrSelfPermission(
+ Manifest.permission.INTERACT_ACROSS_USERS_FULL, TAG);
+ userId = UserHandle.getCallingUserId();
+ }
+
+ ProviderInfo cpi = null;
+ try {
+ cpi = AppGlobals.getPackageManager().resolveContentProvider(authority,
+ STOCK_PM_FLAGS | PackageManager.GET_URI_PERMISSION_PATTERNS, userId);
+ } catch (RemoteException ignored) {
+ }
+ if (cpi == null) {
+ // TODO: make this an outright failure in a future platform release;
+ // until then anonymous content notifications are unprotected
+ //return "Failed to find provider " + authority + " for user " + userId;
+ return null;
+ }
+
+ ProcessRecord r = null;
+ synchronized (mPidsSelfLocked) {
+ r = mPidsSelfLocked.get(Binder.getCallingPid());
+ }
+ if (r == null) {
+ return "Failed to find PID " + Binder.getCallingPid();
+ }
+
+ synchronized (this) {
+ return checkContentProviderPermissionLocked(cpi, r, userId, true);
+ }
+ }
+
+ /**
* Check if {@link ProcessRecord} has a possible chance at accessing the
* given {@link ProviderInfo}. Final permission checking is always done
* in {@link ContentProvider}.
@@ -20624,6 +20661,11 @@ public final class ActivityManagerService extends ActivityManagerNative
private final class LocalService extends ActivityManagerInternal {
@Override
+ public String checkContentProviderAccess(String authority, int userId) {
+ return ActivityManagerService.this.checkContentProviderAccess(authority, userId);
+ }
+
+ @Override
public void onWakefulnessChanged(int wakefulness) {
ActivityManagerService.this.onWakefulnessChanged(wakefulness);
}
diff --git a/services/core/java/com/android/server/content/ContentService.java b/services/core/java/com/android/server/content/ContentService.java
index f581a7f8c4b7..1db31c9abbec 100644
--- a/services/core/java/com/android/server/content/ContentService.java
+++ b/services/core/java/com/android/server/content/ContentService.java
@@ -19,6 +19,8 @@ package com.android.server.content;
import android.Manifest;
import android.accounts.Account;
import android.app.ActivityManager;
+import android.app.ActivityManagerInternal;
+import android.app.ActivityManagerNative;
import android.content.ComponentName;
import android.content.ContentResolver;
import android.content.Context;
@@ -51,7 +53,6 @@ import com.android.server.LocalServices;
import java.io.FileDescriptor;
import java.io.PrintWriter;
-import java.security.InvalidParameterException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
@@ -190,23 +191,15 @@ public final class ContentService extends IContentService.Stub {
final int uid = Binder.getCallingUid();
final int pid = Binder.getCallingPid();
- final int callingUserHandle = UserHandle.getCallingUserId();
- // Registering an observer for any user other than the calling user requires uri grant or
- // cross user permission
- if (callingUserHandle != userHandle &&
- mContext.checkUriPermission(uri, pid, uid, Intent.FLAG_GRANT_READ_URI_PERMISSION)
- != PackageManager.PERMISSION_GRANTED) {
- enforceCrossUserPermission(userHandle,
- "no permission to observe other users' provider view");
- }
- if (userHandle < 0) {
- if (userHandle == UserHandle.USER_CURRENT) {
- userHandle = ActivityManager.getCurrentUser();
- } else if (userHandle != UserHandle.USER_ALL) {
- throw new InvalidParameterException("Bad user handle for registerContentObserver: "
- + userHandle);
- }
+ userHandle = handleIncomingUser(uri, pid, uid,
+ Intent.FLAG_GRANT_READ_URI_PERMISSION, userHandle);
+
+ final String msg = LocalServices.getService(ActivityManagerInternal.class)
+ .checkContentProviderAccess(uri.getAuthority(), userHandle);
+ if (msg != null) {
+ Log.w(TAG, "Ignoring content changes for " + uri + " from " + uid + ": " + msg);
+ return;
}
synchronized (mRootNode) {
@@ -253,21 +246,15 @@ public final class ContentService extends IContentService.Stub {
final int uid = Binder.getCallingUid();
final int pid = Binder.getCallingPid();
final int callingUserHandle = UserHandle.getCallingUserId();
- // Notify for any user other than the caller requires uri grant or cross user permission
- if (callingUserHandle != userHandle &&
- mContext.checkUriPermission(uri, pid, uid, Intent.FLAG_GRANT_WRITE_URI_PERMISSION)
- != PackageManager.PERMISSION_GRANTED) {
- enforceCrossUserPermission(userHandle, "no permission to notify other users");
- }
- // We passed the permission check; resolve pseudouser targets as appropriate
- if (userHandle < 0) {
- if (userHandle == UserHandle.USER_CURRENT) {
- userHandle = ActivityManager.getCurrentUser();
- } else if (userHandle != UserHandle.USER_ALL) {
- throw new InvalidParameterException("Bad user handle for notifyChange: "
- + userHandle);
- }
+ userHandle = handleIncomingUser(uri, pid, uid,
+ Intent.FLAG_GRANT_WRITE_URI_PERMISSION, userHandle);
+
+ final String msg = LocalServices.getService(ActivityManagerInternal.class)
+ .checkContentProviderAccess(uri.getAuthority(), userHandle);
+ if (msg != null) {
+ Log.w(TAG, "Ignoring notify for " + uri + " from " + uid + ": " + msg);
+ return;
}
// This makes it so that future permission checks will be in the context of this
@@ -317,6 +304,15 @@ public final class ContentService extends IContentService.Stub {
}
}
+ private int checkUriPermission(Uri uri, int pid, int uid, int modeFlags, int userHandle) {
+ try {
+ return ActivityManagerNative.getDefault().checkUriPermission(
+ uri, pid, uid, modeFlags, userHandle, null);
+ } catch (RemoteException e) {
+ return PackageManager.PERMISSION_DENIED;
+ }
+ }
+
public void notifyChange(Uri uri, IContentObserver observer,
boolean observerWantsSelfNotifications, boolean syncToNetwork) {
notifyChange(uri, observer, observerWantsSelfNotifications, syncToNetwork,
@@ -924,6 +920,27 @@ public final class ContentService extends IContentService.Stub {
return service;
}
+ private int handleIncomingUser(Uri uri, int pid, int uid, int modeFlags, int userId) {
+ if (userId == UserHandle.USER_CURRENT) {
+ userId = ActivityManager.getCurrentUser();
+ }
+
+ if (userId == UserHandle.USER_ALL) {
+ mContext.enforceCallingOrSelfPermission(
+ Manifest.permission.INTERACT_ACROSS_USERS_FULL, TAG);
+ } else if (userId < 0) {
+ throw new IllegalArgumentException("Invalid user: " + userId);
+ } else if (userId != UserHandle.getCallingUserId()) {
+ if (checkUriPermission(uri, pid, uid, modeFlags,
+ userId) != PackageManager.PERMISSION_GRANTED) {
+ mContext.enforceCallingOrSelfPermission(
+ Manifest.permission.INTERACT_ACROSS_USERS_FULL, TAG);
+ }
+ }
+
+ return userId;
+ }
+
/**
* Checks if the request is from the system or an app that has INTERACT_ACROSS_USERS_FULL
* permission, if the userHandle is not for the caller.