From 591e345fbc0ccb6815e27ccff1a0ae9d02002a90 Mon Sep 17 00:00:00 2001 From: lucaslin Date: Thu, 29 Jul 2021 09:37:48 +0800 Subject: [PATCH] Make sure that only the owner can call [stop|start]VpnProfile() In stopVpnProfile() & startVpnProfile, it doesn't check if the caller's package name is the same as the given one, so any app has chance to stop/start the VPN profile of other apps. Bug: 191382886 Test: atest FrameworksNetTests CtsNetTestCases \ CtsHostsideNetworkTests:HostsideVpnTests Change-Id: Ib0a6e9ed191ff8c8bd55ce9902d894b6a339ace2 --- .../com/android/server/VpnManagerService.java | 30 +++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/services/core/java/com/android/server/VpnManagerService.java b/services/core/java/com/android/server/VpnManagerService.java index d483f1863258f..a03425c0bb75e 100644 --- a/services/core/java/com/android/server/VpnManagerService.java +++ b/services/core/java/com/android/server/VpnManagerService.java @@ -26,6 +26,8 @@ import android.content.BroadcastReceiver; import android.content.Context; import android.content.Intent; import android.content.IntentFilter; +import android.content.pm.PackageManager; +import android.content.pm.PackageManager.NameNotFoundException; import android.net.ConnectivityManager; import android.net.INetd; import android.net.IVpnManager; @@ -312,6 +314,26 @@ public class VpnManagerService extends IVpnManager.Stub { } } + // TODO : Move to a static lib to factorize with Vpn.java + private int getAppUid(final String app, final int userId) { + final PackageManager pm = mContext.getPackageManager(); + final long token = Binder.clearCallingIdentity(); + try { + return pm.getPackageUidAsUser(app, userId); + } catch (NameNotFoundException e) { + return -1; + } finally { + Binder.restoreCallingIdentity(token); + } + } + + private void verifyCallingUidAndPackage(String packageName, int callingUid) { + final int userId = UserHandle.getUserId(callingUid); + if (getAppUid(packageName, userId) != callingUid) { + throw new SecurityException(packageName + " does not belong to uid " + callingUid); + } + } + /** * Starts the VPN based on the stored profile for the given package * @@ -323,7 +345,9 @@ public class VpnManagerService extends IVpnManager.Stub { */ @Override public void startVpnProfile(@NonNull String packageName) { - final int user = UserHandle.getUserId(mDeps.getCallingUid()); + final int callingUid = Binder.getCallingUid(); + verifyCallingUidAndPackage(packageName, callingUid); + final int user = UserHandle.getUserId(callingUid); synchronized (mVpns) { throwIfLockdownEnabled(); mVpns.get(user).startVpnProfile(packageName); @@ -340,7 +364,9 @@ public class VpnManagerService extends IVpnManager.Stub { */ @Override public void stopVpnProfile(@NonNull String packageName) { - final int user = UserHandle.getUserId(mDeps.getCallingUid()); + final int callingUid = Binder.getCallingUid(); + verifyCallingUidAndPackage(packageName, callingUid); + final int user = UserHandle.getUserId(callingUid); synchronized (mVpns) { mVpns.get(user).stopVpnProfile(packageName); }