Validate obbInfo in StorageManagerService
Validate that the obbInfo passed for mounting belongs to the calling app's specified path
Bug: 236122058
Test: atest android.os.storage.cts.StorageManagerTest
Flag: EXEMPT bugfix
Change-Id: I74e945ce4922b59e3777b66a7ebd901395193e39
diff --git a/core/tests/coretests/src/android/os/storage/StorageManagerIntegrationTest.java b/core/tests/coretests/src/android/os/storage/StorageManagerIntegrationTest.java
index ecd2f76..b157c95 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 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 throw an 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 @@
}
}
+ @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 throw an 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 07e5f2e..d86bae1 100644
--- a/services/core/java/com/android/server/StorageManagerService.java
+++ b/services/core/java/com/android/server/StorageManagerService.java
@@ -224,6 +224,9 @@
/** 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 @@
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 @@
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");