diff options
| -rw-r--r-- | core/tests/coretests/src/android/os/storage/StorageManagerIntegrationTest.java | 54 | ||||
| -rw-r--r-- | services/core/java/com/android/server/StorageManagerService.java | 35 |
2 files changed, 87 insertions, 2 deletions
diff --git a/core/tests/coretests/src/android/os/storage/StorageManagerIntegrationTest.java b/core/tests/coretests/src/android/os/storage/StorageManagerIntegrationTest.java index ecd2f76a5160..8685326a0173 100644 --- a/core/tests/coretests/src/android/os/storage/StorageManagerIntegrationTest.java +++ b/core/tests/coretests/src/android/os/storage/StorageManagerIntegrationTest.java @@ -16,8 +16,11 @@ package android.os.storage; +import android.content.res.ObbInfo; +import android.os.Parcel; import android.os.ParcelFileDescriptor; import android.os.ProxyFileDescriptorCallback; +import android.os.ServiceManager; import android.system.ErrnoException; import androidx.test.filters.LargeTest; @@ -104,7 +107,14 @@ public class StorageManagerIntegrationTest extends StorageManagerBaseTest { public void testMountBadPackageNameObb() throws Exception { final File file = createObbFile(OBB_FILE_3_BAD_PACKAGENAME, R.raw.obb_file3_bad_packagename); String filePath = file.getAbsolutePath(); - mountObb(filePath, OnObbStateChangeListener.ERROR_PERMISSION_DENIED); + try { + mountObb(filePath, OnObbStateChangeListener.ERROR_PERMISSION_DENIED); + fail("mountObb should have thrown a exception as package name is incorrect"); + } catch (Exception ex) { + assertEquals("Path " + filePath + + " does not contain package name " + mContext.getPackageName(), + ex.getMessage()); + } } /** @@ -154,6 +164,48 @@ public class StorageManagerIntegrationTest extends StorageManagerBaseTest { } } + @LargeTest + public void testObbInfo_withValidObbInfo_success() throws Exception { + final File file = createObbFile(OBB_FILE_1, R.raw.obb_file1); + String filePath = file.getAbsolutePath(); + try { + mountObb(filePath); + unmountObb(filePath, DONT_FORCE); + } catch (Exception ex) { + fail("No exception expected, got " + ex.getMessage()); + } + } + + @LargeTest + public void testObbInfo_withInvalidObbInfo_exception() throws Exception { + final File file = createObbFile(OBB_FILE_1, R.raw.obb_file1); + String rawPath = file.getAbsolutePath(); + String canonicalPath = file.getCanonicalPath(); + + ObbInfo obbInfo = ObbInfo.CREATOR.createFromParcel(Parcel.obtain()); + obbInfo.packageName = "com.android.obbcrash"; + obbInfo.version = 1; + obbInfo.filename = canonicalPath; + + try { + IStorageManager.Stub.asInterface(ServiceManager.getServiceOrThrow("mount")).mountObb( + rawPath, canonicalPath, new ObbActionListener(), 0, obbInfo); + fail("mountObb should have thrown a exception as package name is incorrect"); + } catch (SecurityException ex) { + assertEquals("Path " + canonicalPath + + " does not contain package name " + mContext.getPackageName(), + ex.getMessage()); + } + } + + private static class ObbActionListener extends IObbActionListener.Stub { + @SuppressWarnings("hiding") + @Override + public void onObbResult(String filename, int nonce, int status) { + + } + } + private static class MyThreadFactory implements ThreadFactory { Thread thread = null; diff --git a/services/core/java/com/android/server/StorageManagerService.java b/services/core/java/com/android/server/StorageManagerService.java index 07e5f2e34ab8..d86bae19f174 100644 --- a/services/core/java/com/android/server/StorageManagerService.java +++ b/services/core/java/com/android/server/StorageManagerService.java @@ -224,6 +224,9 @@ class StorageManagerService extends IStorageManager.Stub /** Extended timeout for the system server watchdog for vold#partition operation. */ private static final int PARTITION_OPERATION_WATCHDOG_TIMEOUT_MS = 3 * 60 * 1000; + private static final Pattern OBB_FILE_PATH = Pattern.compile( + "(?i)(^/storage/[^/]+/(?:([0-9]+)/)?Android/obb/)([^/]+)/([^/]+\\.obb)"); + @GuardedBy("mLock") private final Set<Integer> mFuseMountedUser = new ArraySet<>(); @@ -3144,7 +3147,9 @@ class StorageManagerService extends IStorageManager.Stub Objects.requireNonNull(rawPath, "rawPath cannot be null"); Objects.requireNonNull(canonicalPath, "canonicalPath cannot be null"); Objects.requireNonNull(token, "token cannot be null"); - Objects.requireNonNull(obbInfo, "obbIfno cannot be null"); + Objects.requireNonNull(obbInfo, "obbInfo cannot be null"); + + validateObbInfo(obbInfo, rawPath); final int callingUid = Binder.getCallingUid(); final ObbState obbState = new ObbState(rawPath, canonicalPath, @@ -3156,6 +3161,34 @@ class StorageManagerService extends IStorageManager.Stub Slog.i(TAG, "Send to OBB handler: " + action.toString()); } + private void validateObbInfo(ObbInfo obbInfo, String rawPath) { + String obbFilePath; + try { + obbFilePath = new File(rawPath).getCanonicalPath(); + } catch (IOException ex) { + throw new RuntimeException("Failed to resolve path" + rawPath + " : " + ex); + } + + Matcher matcher = OBB_FILE_PATH.matcher(obbFilePath); + + if (matcher.matches()) { + int userId = UserHandle.getUserId(Binder.getCallingUid()); + String pathUserId = matcher.group(2); + String pathPackageName = matcher.group(3); + if ((pathUserId != null && Integer.parseInt(pathUserId) != userId) + || (pathUserId == null && userId != mCurrentUserId)) { + throw new SecurityException( + "Path " + obbFilePath + "does not correspond to calling userId " + userId); + } + if (obbInfo != null && !obbInfo.packageName.equals(pathPackageName)) { + throw new SecurityException("Path " + obbFilePath + + " does not contain package name " + pathPackageName); + } + } else { + throw new SecurityException("Invalid path to Obb file : " + obbFilePath); + } + } + @Override public void unmountObb(String rawPath, boolean force, IObbActionListener token, int nonce) { Objects.requireNonNull(rawPath, "rawPath cannot be null"); |