Fix issue #4902856: Don't let apps register non-explicit PendingIntents
Location manager now checks for such intents, and logs a warning when
they are given to it. Nothing thrown yet, it needs to check the
targetSdkVersion of the caller somehow.
When sending the pending intent, we require that the recipient hold the
appropriate permission. This should pretty much close the security hole.
Includes a bunch of infrastructure in the activity manager needed to
support all this.
Change-Id: I4dba7a98a7b8bbb9e347666451aa9cb1efad1848
diff --git a/services/java/com/android/server/LocationManagerService.java b/services/java/com/android/server/LocationManagerService.java
index 656ec4d..56afe7f 100644
--- a/services/java/com/android/server/LocationManagerService.java
+++ b/services/java/com/android/server/LocationManagerService.java
@@ -195,6 +195,7 @@
final Object mKey;
final HashMap<String,UpdateRecord> mUpdateRecords = new HashMap<String,UpdateRecord>();
int mPendingBroadcasts;
+ String requiredPermissions;
Receiver(ILocationListener listener) {
mListener = listener;
@@ -284,7 +285,8 @@
synchronized (this) {
// synchronize to ensure incrementPendingBroadcastsLocked()
// is called before decrementPendingBroadcasts()
- mPendingIntent.send(mContext, 0, statusChanged, this, mLocationHandler);
+ mPendingIntent.send(mContext, 0, statusChanged, this, mLocationHandler,
+ requiredPermissions);
// call this after broadcasting so we do not increment
// if we throw an exeption.
incrementPendingBroadcastsLocked();
@@ -319,7 +321,8 @@
synchronized (this) {
// synchronize to ensure incrementPendingBroadcastsLocked()
// is called before decrementPendingBroadcasts()
- mPendingIntent.send(mContext, 0, locationChanged, this, mLocationHandler);
+ mPendingIntent.send(mContext, 0, locationChanged, this, mLocationHandler,
+ requiredPermissions);
// call this after broadcasting so we do not increment
// if we throw an exeption.
incrementPendingBroadcastsLocked();
@@ -358,7 +361,8 @@
synchronized (this) {
// synchronize to ensure incrementPendingBroadcastsLocked()
// is called before decrementPendingBroadcasts()
- mPendingIntent.send(mContext, 0, providerIntent, this, mLocationHandler);
+ mPendingIntent.send(mContext, 0, providerIntent, this, mLocationHandler,
+ requiredPermissions);
// call this after broadcasting so we do not increment
// if we throw an exeption.
incrementPendingBroadcastsLocked();
@@ -572,22 +576,30 @@
return Settings.Secure.isLocationProviderEnabled(resolver, provider);
}
- private void checkPermissionsSafe(String provider) {
- if ((LocationManager.GPS_PROVIDER.equals(provider)
- || LocationManager.PASSIVE_PROVIDER.equals(provider))
- && (mContext.checkCallingOrSelfPermission(ACCESS_FINE_LOCATION)
- != PackageManager.PERMISSION_GRANTED)) {
- throw new SecurityException("Provider " + provider
- + " requires ACCESS_FINE_LOCATION permission");
+ private String checkPermissionsSafe(String provider, String lastPermission) {
+ if (LocationManager.GPS_PROVIDER.equals(provider)
+ || LocationManager.PASSIVE_PROVIDER.equals(provider)) {
+ if (mContext.checkCallingOrSelfPermission(ACCESS_FINE_LOCATION)
+ != PackageManager.PERMISSION_GRANTED) {
+ throw new SecurityException("Provider " + provider
+ + " requires ACCESS_FINE_LOCATION permission");
+ }
+ return ACCESS_FINE_LOCATION;
}
- if (LocationManager.NETWORK_PROVIDER.equals(provider)
- && (mContext.checkCallingOrSelfPermission(ACCESS_FINE_LOCATION)
- != PackageManager.PERMISSION_GRANTED)
- && (mContext.checkCallingOrSelfPermission(ACCESS_COARSE_LOCATION)
- != PackageManager.PERMISSION_GRANTED)) {
- throw new SecurityException("Provider " + provider
- + " requires ACCESS_FINE_LOCATION or ACCESS_COARSE_LOCATION permission");
+
+ // Assume any other provider requires the coarse or fine permission.
+ if (mContext.checkCallingOrSelfPermission(ACCESS_COARSE_LOCATION)
+ == PackageManager.PERMISSION_GRANTED) {
+ return ACCESS_FINE_LOCATION.equals(lastPermission)
+ ? lastPermission : ACCESS_COARSE_LOCATION;
}
+ if (mContext.checkCallingOrSelfPermission(ACCESS_FINE_LOCATION)
+ == PackageManager.PERMISSION_GRANTED) {
+ return ACCESS_FINE_LOCATION;
+ }
+
+ throw new SecurityException("Provider " + provider
+ + " requires ACCESS_FINE_LOCATION or ACCESS_COARSE_LOCATION permission");
}
private boolean isAllowedProviderSafe(String provider) {
@@ -1099,8 +1111,21 @@
}
}
+ void validatePendingIntent(PendingIntent intent) {
+ if (intent.isTargetedToPackage()) {
+ return;
+ }
+ Slog.i(TAG, "Given Intent does not require a specific package: "
+ + intent);
+ // XXX we should really throw a security exception, if the caller's
+ // targetSdkVersion is high enough.
+ //throw new SecurityException("Given Intent does not require a specific package: "
+ // + intent);
+ }
+
public void requestLocationUpdatesPI(String provider, Criteria criteria,
long minTime, float minDistance, boolean singleShot, PendingIntent intent) {
+ validatePendingIntent(intent);
if (criteria != null) {
// FIXME - should we consider using multiple providers simultaneously
// rather than only the best one?
@@ -1132,7 +1157,8 @@
throw new IllegalArgumentException("provider=" + provider);
}
- checkPermissionsSafe(provider);
+ receiver.requiredPermissions = checkPermissionsSafe(provider,
+ receiver.requiredPermissions);
// so wakelock calls will succeed
final int callingUid = Binder.getCallingUid();
@@ -1300,7 +1326,7 @@
}
// first check for permission to the provider
- checkPermissionsSafe(provider);
+ checkPermissionsSafe(provider, null);
// and check for ACCESS_LOCATION_EXTRA_COMMANDS
if ((mContext.checkCallingOrSelfPermission(ACCESS_LOCATION_EXTRA_COMMANDS)
!= PackageManager.PERMISSION_GRANTED)) {
@@ -1432,7 +1458,8 @@
synchronized (this) {
// synchronize to ensure incrementPendingBroadcasts()
// is called before decrementPendingBroadcasts()
- intent.send(mContext, 0, enteredIntent, this, mLocationHandler);
+ intent.send(mContext, 0, enteredIntent, this, mLocationHandler,
+ ACCESS_FINE_LOCATION);
// call this after broadcasting so we do not increment
// if we throw an exeption.
incrementPendingBroadcasts();
@@ -1457,7 +1484,8 @@
synchronized (this) {
// synchronize to ensure incrementPendingBroadcasts()
// is called before decrementPendingBroadcasts()
- intent.send(mContext, 0, exitedIntent, this, mLocationHandler);
+ intent.send(mContext, 0, exitedIntent, this, mLocationHandler,
+ ACCESS_FINE_LOCATION);
// call this after broadcasting so we do not increment
// if we throw an exeption.
incrementPendingBroadcasts();
@@ -1526,6 +1554,7 @@
public void addProximityAlert(double latitude, double longitude,
float radius, long expiration, PendingIntent intent) {
+ validatePendingIntent(intent);
try {
synchronized (mLock) {
addProximityAlertLocked(latitude, longitude, radius, expiration, intent);
@@ -1626,7 +1655,7 @@
return null;
}
- checkPermissionsSafe(provider);
+ checkPermissionsSafe(provider, null);
Bundle b = new Bundle();
b.putBoolean("network", p.requiresNetwork());
@@ -1668,7 +1697,7 @@
}
private boolean _isProviderEnabledLocked(String provider) {
- checkPermissionsSafe(provider);
+ checkPermissionsSafe(provider, null);
LocationProviderInterface p = mProvidersByName.get(provider);
if (p == null) {
@@ -1694,7 +1723,7 @@
}
private Location _getLastKnownLocationLocked(String provider) {
- checkPermissionsSafe(provider);
+ checkPermissionsSafe(provider, null);
LocationProviderInterface p = mProvidersByName.get(provider);
if (p == null) {