From de38982c30afe7c0264e0e0cd0dccfba15494e5d Mon Sep 17 00:00:00 2001 From: Soonil Nagarkar Date: Wed, 4 Nov 2020 11:17:57 -0800 Subject: Fix security hole in Geocoder APIs Geocoder APIs are a mess, but this will at least validate the caller properly, and pass the relevant information along to clients. Bug: 158318462 Test: presubmits Change-Id: I47b09962598a2346bd79caa643ba1d83069c0c19 --- location/java/android/location/Geocoder.java | 61 ++++---------- location/java/android/location/GeocoderParams.java | 92 ++++++++++++++-------- 2 files changed, 77 insertions(+), 76 deletions(-) (limited to 'location/java') diff --git a/location/java/android/location/Geocoder.java b/location/java/android/location/Geocoder.java index 704cdbfc77b1..307fb8783c51 100644 --- a/location/java/android/location/Geocoder.java +++ b/location/java/android/location/Geocoder.java @@ -21,6 +21,8 @@ import android.os.IBinder; import android.os.RemoteException; import android.os.ServiceManager; +import com.android.internal.util.Preconditions; + import java.io.IOException; import java.util.Collections; import java.util.List; @@ -77,12 +79,9 @@ public final class Geocoder { * @throws NullPointerException if Locale is null */ public Geocoder(Context context, Locale locale) { - if (locale == null) { - throw new NullPointerException("locale == null"); - } mParams = new GeocoderParams(context, locale); - IBinder b = ServiceManager.getService(Context.LOCATION_SERVICE); - mService = ILocationManager.Stub.asInterface(b); + mService = ILocationManager.Stub.asInterface( + ServiceManager.getService(Context.LOCATION_SERVICE)); } /** @@ -121,13 +120,10 @@ public final class Geocoder { * I/O problem occurs */ public List
getFromLocation(double latitude, double longitude, int maxResults) - throws IOException { - if (latitude < -90.0 || latitude > 90.0) { - throw new IllegalArgumentException("latitude == " + latitude); - } - if (longitude < -180.0 || longitude > 180.0) { - throw new IllegalArgumentException("longitude == " + longitude); - } + throws IOException { + Preconditions.checkArgumentInRange(latitude, -90.0, 90.0, "latitude"); + Preconditions.checkArgumentInRange(longitude, -180.0, 180.0, "longitude"); + try { GeocodeListener listener = new GeocodeListener(); mService.getFromLocation(latitude, longitude, maxResults, mParams, listener); @@ -161,17 +157,7 @@ public final class Geocoder { * I/O problem occurs */ public List
getFromLocationName(String locationName, int maxResults) throws IOException { - if (locationName == null) { - throw new IllegalArgumentException("locationName == null"); - } - - try { - GeocodeListener listener = new GeocodeListener(); - mService.getFromLocationName(locationName, 0, 0, 0, 0, maxResults, mParams, listener); - return listener.getResults(); - } catch (RemoteException e) { - throw e.rethrowFromSystemServer(); - } + return getFromLocationName(locationName, maxResults, 0, 0, 0, 0); } /** @@ -210,27 +196,14 @@ public final class Geocoder { * I/O problem occurs */ public List
getFromLocationName(String locationName, int maxResults, - double lowerLeftLatitude, double lowerLeftLongitude, - double upperRightLatitude, double upperRightLongitude) throws IOException { - if (locationName == null) { - throw new IllegalArgumentException("locationName == null"); - } - if (lowerLeftLatitude < -90.0 || lowerLeftLatitude > 90.0) { - throw new IllegalArgumentException("lowerLeftLatitude == " - + lowerLeftLatitude); - } - if (lowerLeftLongitude < -180.0 || lowerLeftLongitude > 180.0) { - throw new IllegalArgumentException("lowerLeftLongitude == " - + lowerLeftLongitude); - } - if (upperRightLatitude < -90.0 || upperRightLatitude > 90.0) { - throw new IllegalArgumentException("upperRightLatitude == " - + upperRightLatitude); - } - if (upperRightLongitude < -180.0 || upperRightLongitude > 180.0) { - throw new IllegalArgumentException("upperRightLongitude == " - + upperRightLongitude); - } + double lowerLeftLatitude, double lowerLeftLongitude, double upperRightLatitude, + double upperRightLongitude) throws IOException { + Preconditions.checkArgument(locationName != null); + Preconditions.checkArgumentInRange(lowerLeftLatitude, -90.0, 90.0, "lowerLeftLatitude"); + Preconditions.checkArgumentInRange(lowerLeftLongitude, -180.0, 180.0, "lowerLeftLongitude"); + Preconditions.checkArgumentInRange(upperRightLatitude, -90.0, 90.0, "upperRightLatitude"); + Preconditions.checkArgumentInRange(upperRightLongitude, -180.0, 180.0, + "upperRightLongitude"); try { GeocodeListener listener = new GeocodeListener(); diff --git a/location/java/android/location/GeocoderParams.java b/location/java/android/location/GeocoderParams.java index 1c6e9b6e1836..b00a9a99e75d 100644 --- a/location/java/android/location/GeocoderParams.java +++ b/location/java/android/location/GeocoderParams.java @@ -16,12 +16,16 @@ package android.location; +import android.annotation.NonNull; +import android.annotation.Nullable; import android.compat.annotation.UnsupportedAppUsage; import android.content.Context; import android.os.Parcel; import android.os.Parcelable; +import android.os.Process; import java.util.Locale; +import java.util.Objects; /** * This class contains extra parameters to pass to an IGeocodeProvider @@ -34,64 +38,88 @@ import java.util.Locale; * @hide */ public class GeocoderParams implements Parcelable { - private Locale mLocale; - private String mPackageName; - // used only for parcelling - private GeocoderParams() { + private final int mUid; + private final String mPackageName; + private final @Nullable String mAttributionTag; + private final Locale mLocale; + + public GeocoderParams(Context context) { + this(context, Locale.getDefault()); } - /** - * This object is only constructed by the Geocoder class - * - * @hide - */ public GeocoderParams(Context context, Locale locale) { - mLocale = locale; - mPackageName = context.getPackageName(); + this(Process.myUid(), context.getPackageName(), context.getAttributionTag(), locale); + } + + private GeocoderParams(int uid, String packageName, String attributionTag, Locale locale) { + mUid = uid; + mPackageName = Objects.requireNonNull(packageName); + mAttributionTag = attributionTag; + mLocale = Objects.requireNonNull(locale); } /** - * returns the Geocoder's locale + * Returns the client UID. */ @UnsupportedAppUsage - public Locale getLocale() { - return mLocale; + public int getClientUid() { + return mUid; } /** - * returns the package name of the Geocoder's client + * Returns the client package name. */ @UnsupportedAppUsage - public String getClientPackage() { + public @NonNull String getClientPackage() { return mPackageName; } - public static final @android.annotation.NonNull Parcelable.Creator CREATOR = + /** + * Returns the client attribution tag. + */ + @UnsupportedAppUsage + public @Nullable String getClientAttributionTag() { + return mAttributionTag; + } + + /** + * Returns the locale. + */ + @UnsupportedAppUsage + public @NonNull Locale getLocale() { + return mLocale; + } + + public static final @NonNull Parcelable.Creator CREATOR = new Parcelable.Creator() { - public GeocoderParams createFromParcel(Parcel in) { - GeocoderParams gp = new GeocoderParams(); - String language = in.readString(); - String country = in.readString(); - String variant = in.readString(); - gp.mLocale = new Locale(language, country, variant); - gp.mPackageName = in.readString(); - return gp; - } - - public GeocoderParams[] newArray(int size) { - return new GeocoderParams[size]; - } - }; + public GeocoderParams createFromParcel(Parcel in) { + int uid = in.readInt(); + String packageName = in.readString(); + String attributionTag = in.readString(); + String language = in.readString(); + String country = in.readString(); + String variant = in.readString(); + + return new GeocoderParams(uid, packageName, attributionTag, + new Locale(language, country, variant)); + } + + public GeocoderParams[] newArray(int size) { + return new GeocoderParams[size]; + } + }; public int describeContents() { return 0; } public void writeToParcel(Parcel parcel, int flags) { + parcel.writeInt(mUid); + parcel.writeString(mPackageName); + parcel.writeString(mAttributionTag); parcel.writeString(mLocale.getLanguage()); parcel.writeString(mLocale.getCountry()); parcel.writeString(mLocale.getVariant()); - parcel.writeString(mPackageName); } } -- cgit v1.2.3-59-g8ed1b