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

2 files changed, 77 insertions, 76 deletions

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<Address> 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<Address> 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<Address> 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<GeocoderParams> 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<GeocoderParams> CREATOR =
         new Parcelable.Creator<GeocoderParams>() {
-        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);
     }
 }