Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix issue #4902856: Don't let apps register non-explicit PendingIntents

Browse Source 
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
This commit is contained in:
Dianne Hackborn
2011-06-29 14:05:33 -07:00
parent 04deb7bb7b
commit 6c418d585e
13 changed files with 272 additions and 55 deletions
Download Patch File Download Diff File Expand all files Collapse all files

77
services/java/com/android/server/LocationManagerService.java
View File

@@ -195,6 +195,7 @@ public class LocationManagerService extends ILocationManager.Stub implements Run
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 @@ public class LocationManagerService extends ILocationManager.Stub implements Run
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 @@ public class LocationManagerService extends ILocationManager.Stub implements Run
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 @@ public class LocationManagerService extends ILocationManager.Stub implements Run
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 @@ public class LocationManagerService extends ILocationManager.Stub implements Run
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 @@ public class LocationManagerService extends ILocationManager.Stub implements Run
}
}
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 @@ public class LocationManagerService extends ILocationManager.Stub implements Run
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 @@ public class LocationManagerService extends ILocationManager.Stub implements Run
}
// 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 @@ public class LocationManagerService extends ILocationManager.Stub implements Run
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 @@ public class LocationManagerService extends ILocationManager.Stub implements Run
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 class LocationManagerService extends ILocationManager.Stub implements Run
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 @@ public class LocationManagerService extends ILocationManager.Stub implements Run
return null;
}
checkPermissionsSafe(provider);
checkPermissionsSafe(provider, null);
Bundle b = new Bundle();
b.putBoolean("network", p.requiresNetwork());
@@ -1668,7 +1697,7 @@ public class LocationManagerService extends ILocationManager.Stub implements Run
}
private boolean _isProviderEnabledLocked(String provider) {
checkPermissionsSafe(provider);
checkPermissionsSafe(provider, null);
LocationProviderInterface p = mProvidersByName.get(provider);
if (p == null) {
@@ -1694,7 +1723,7 @@ public class LocationManagerService extends ILocationManager.Stub implements Run
}
private Location _getLastKnownLocationLocked(String provider) {
checkPermissionsSafe(provider);
checkPermissionsSafe(provider, null);
LocationProviderInterface p = mProvidersByName.get(provider);
if (p == null) {