Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 856c4f76 authored by Grace Jia's avatar Grace Jia
Browse files

Fix security vulnerability in getSubscriberIdForSubscriber. 

Check the calling package identity before other permission check so
that user can't use thie method to figure out if a package is installed
or not.

Bug: 185513628
Test: Manually with test app
Change-Id: I0c847dbb0fa839a89f3f3a81a3e91915da6a2be2
parent 251e1a08

src/java/com/android/internal/telephony/PhoneSubInfoController.java

+24 −4
Original line number Diff line number Diff line
@@ -23,7 +23,6 @@ import static android.Manifest.permission.READ_PRIVILEGED_PHONE_STATE; 


import android.annotation.NonNull;

import android.annotation.Nullable;

import android.app.AppOpsManager;

import android.compat.annotation.UnsupportedAppUsage;

import android.content.Context;

import android.content.pm.PackageManager;
@@ -48,7 +47,7 @@ public class PhoneSubInfoController extends IPhoneSubInfo.Stub { 


    @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.R, trackingBug = 170729553)

    private final Context mContext;

    private final AppOpsManager mAppOps;

    private PackageManager mPm;



    public PhoneSubInfoController(Context context) {

        ServiceRegisterer phoneSubServiceRegisterer = TelephonyFrameworkInitializer
@@ -58,7 +57,7 @@ public class PhoneSubInfoController extends IPhoneSubInfo.Stub { 
            phoneSubServiceRegisterer.register(this);

        }

        mContext = context;

        mAppOps = (AppOpsManager) mContext.getSystemService(Context.APP_OPS_SERVICE);

        mPm = mContext.getPackageManager();

    }



    @Deprecated
@@ -143,7 +142,10 @@ public class PhoneSubInfoController extends IPhoneSubInfo.Stub { 


    public String getSubscriberIdForSubscriber(int subId, String callingPackage,

            String callingFeatureId) {

        String message = "getSubscriberId";

        String message = "getSubscriberIdForSubscriber";



        enforceCallingPackage(callingPackage, Binder.getCallingUid(), message);



        long identity = Binder.clearCallingIdentity();

        boolean isActive;

        try {
@@ -301,6 +303,24 @@ public class PhoneSubInfoController extends IPhoneSubInfo.Stub { 
                "Requires MODIFY_PHONE_STATE");

    }



    /**

     * Make sure the caller is the calling package itself

     *

     * @throws SecurityException if the caller is not the calling package

     */

    private void enforceCallingPackage(String callingPackage, int callingUid, String message) {

        int packageUid = -1;

        try {

            packageUid = mPm.getPackageUid(callingPackage, 0);

        } catch (PackageManager.NameNotFoundException e) {

            // packageUid is -1

        }

        if (packageUid != callingUid) {

            throw new SecurityException(message + ": Package " + callingPackage

                    + " does not belong to " + callingUid);

        }

    }



    @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.R, trackingBug = 170729553)

    private int getDefaultSubscription() {

        return  PhoneFactory.getDefaultSubscription();

tests/telephonytests/src/com/android/internal/telephony/PhoneSubInfoControllerTest.java

+6 −0
Original line number Diff line number Diff line
@@ -30,6 +30,7 @@ import android.app.AppOpsManager; 
import android.app.PropertyInvalidatedCache;

import android.content.Context;

import android.content.pm.PackageManager;

import android.os.Binder;

import android.os.Build;

import android.test.suitebuilder.annotation.SmallTest;
@@ -44,6 +45,7 @@ public class PhoneSubInfoControllerTest extends TelephonyTest { 


    private PhoneSubInfoController mPhoneSubInfoControllerUT;

    private AppOpsManager mAppOsMgr;

    private PackageManager mPm;



    @Mock

    GsmCdmaPhone mSecondPhone;
@@ -69,6 +71,7 @@ public class PhoneSubInfoControllerTest extends TelephonyTest { 
        doReturn(mContext).when(mSecondPhone).getContext();



        mAppOsMgr = (AppOpsManager) mContext.getSystemService(Context.APP_OPS_SERVICE);

        mPm = mContext.getPackageManager();



        replaceInstance(PhoneFactory.class, "sPhones", null, new Phone[]{mPhone, mSecondPhone});

        mPhoneSubInfoControllerUT = new PhoneSubInfoController(mContext);
@@ -81,6 +84,9 @@ public class PhoneSubInfoControllerTest extends TelephonyTest { 
        doReturn(AppOpsManager.MODE_ERRORED).when(mAppOsMgr).noteOpNoThrow(

                eq(AppOpsManager.OPSTR_READ_DEVICE_IDENTIFIERS), anyInt(), eq(TAG), eq(FEATURE_ID),

                nullable(String.class));



        // Bypass calling package check.

        doReturn(Binder.getCallingUid()).when(mPm).getPackageUid(eq(TAG), anyInt());

    }



    @After