commit 591e345fbc0ccb6815e27ccff1a0ae9d02002a90
author lucaslin <lucaslin@google.com> Thu Jul 29 09:37:48 2021 +0800
committer Lucas Lin <lucaslin@google.com> Thu Jul 29 05:33:13 2021 +0000
tree 35f075f202ca9ca279175c29b4c788af5f1937dd
parent b8a20acc472591f046f4ee95ca1d73091675330c

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

services/core/java/com/android/server/VpnManagerService.java

diff --git a/services/core/java/com/android/server/VpnManagerService.java b/services/core/java/com/android/server/VpnManagerService.java
index d483f18..a03425c 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.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 @@
         }
     }
 
+    // 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 @@
      */
     @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 @@
      */
     @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);
         }