From f1121dc1d35c7e8c317c278aad0dd4ad1358d870 Mon Sep 17 00:00:00 2001
From: Kenny Root <kroot@google.com>
Date: Wed, 29 Sep 2010 07:30:53 -0700
Subject: [PATCH] Move all the permissions check up for unmount

We have already scanned the state of the OBB in mountObb, so check the
caller against the stored state in the unmountObb call. This allows us
to ensure the calling binder is the same one that mounted it since we
tie the lifecycle of the OBB to the lifecycle of the binder.

Change-Id: I45d9cfbab5d3f5b37a6a9b594b10bd8b91cccc45
---
 core/java/android/content/res/ObbScanner.java |  2 +-
 .../java/com/android/server/MountService.java | 35 ++++++++++++-------
 2 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/core/java/android/content/res/ObbScanner.java b/core/java/android/content/res/ObbScanner.java
index a3f141e9b1716..572b75e8c61a6 100644
--- a/core/java/android/content/res/ObbScanner.java
+++ b/core/java/android/content/res/ObbScanner.java
@@ -42,7 +42,7 @@ public class ObbScanner {
 
         final File obbFile = new File(filePath);
         if (!obbFile.exists()) {
-            throw new IllegalArgumentException("OBB file does nto exist: " + filePath);
+            throw new IllegalArgumentException("OBB file does not exist: " + filePath);
         }
 
         final String canonicalFilePath = obbFile.getCanonicalPath();
diff --git a/services/java/com/android/server/MountService.java b/services/java/com/android/server/MountService.java
index f3625a8d2b961..265d61388f0ff 100644
--- a/services/java/com/android/server/MountService.java
+++ b/services/java/com/android/server/MountService.java
@@ -1451,11 +1451,6 @@ class MountService extends IMountService.Stub
         mHandler.sendEmptyMessage(H_UNMOUNT_PM_DONE);
     }
 
-    private boolean isCallerOwnerOfPackageOrSystem(String packageName) {
-        final int callerUid = Binder.getCallingUid();
-        return isUidOwnerOfPackageOrSystem(packageName, callerUid);
-    }
-
     private boolean isUidOwnerOfPackageOrSystem(String packageName, int callerUid) {
         if (callerUid == android.os.Process.SYSTEM_UID) {
             return true;
@@ -1507,6 +1502,12 @@ class MountService extends IMountService.Stub
         waitForReady();
         warnOnNotMounted();
 
+        if (filename == null) {
+            throw new IllegalArgumentException("filename cannot be null");
+        } else if (token == null) {
+            throw new IllegalArgumentException("token cannot be null");
+        }
+
         final ObbState obbState;
 
         synchronized (mObbMounts) {
@@ -1533,6 +1534,12 @@ class MountService extends IMountService.Stub
     }
 
     public void unmountObb(String filename, boolean force, IObbActionListener token) {
+        if (filename == null) {
+            throw new IllegalArgumentException("filename cannot be null");
+        } else if (token == null) {
+            throw new IllegalArgumentException("token cannot be null");
+        }
+
         final ObbState obbState;
 
         synchronized (mObbMounts) {
@@ -1540,6 +1547,12 @@ class MountService extends IMountService.Stub
                 throw new IllegalArgumentException("OBB is not mounted");
             }
             obbState = mObbPathToStateMap.get(filename);
+
+            if (Binder.getCallingUid() != obbState.callerUid) {
+                throw new SecurityException("caller UID does not match original mount caller UID");
+            } else if (!token.asBinder().equals(obbState.token.asBinder())) {
+                throw new SecurityException("caller does not match original mount caller");
+            }
         }
 
         UnmountObbAction action = new UnmountObbAction(obbState, force);
@@ -1758,9 +1771,9 @@ class MountService extends IMountService.Stub
         }
 
         public void handleExecute() throws RemoteException, IOException {
-            ObbInfo obbInfo = mContainerService.getObbInfo(mObbState.filename);
+            final ObbInfo obbInfo = mContainerService.getObbInfo(mObbState.filename);
             if (obbInfo == null) {
-                throw new IOException("Couldn't read OBB file");
+                throw new IOException("Couldn't read OBB file: " + mObbState.filename);
             }
 
             if (!isUidOwnerOfPackageOrSystem(obbInfo.packageName, mObbState.callerUid)) {
@@ -1833,13 +1846,9 @@ class MountService extends IMountService.Stub
         }
 
         public void handleExecute() throws RemoteException, IOException {
-            ObbInfo obbInfo = mContainerService.getObbInfo(mObbState.filename);
+            final ObbInfo obbInfo = mContainerService.getObbInfo(mObbState.filename);
             if (obbInfo == null) {
-                throw new IOException("Couldn't read OBB file");
-            }
-
-            if (!isCallerOwnerOfPackageOrSystem(obbInfo.packageName)) {
-                throw new IllegalArgumentException("Caller package does not match OBB file");
+                throw new IOException("Couldn't read OBB file: " + mObbState.filename);
             }
 
             int rc = StorageResultCode.OperationSucceeded;