Merge "Revert "Validate obbInfo in StorageManagerService"" into main (385c434c) · Commits · e / os / android_frameworks_base

core/tests/coretests/src/android/os/storage/StorageManagerIntegrationTest.java

+1 −53

Original line number	Diff line number	Diff line
		@@ -16,11 +16,8 @@

		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;
		@@ -107,14 +104,7 @@ 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();
		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());
		}
		}

		/**
		@@ -164,48 +154,6 @@ 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;

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

+1 −34

Original line number	Diff line number	Diff line
		@@ -224,9 +224,6 @@ 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<>();

		@@ -3147,9 +3144,7 @@ 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, "obbInfo cannot be null");

		validateObbInfo(obbInfo, rawPath);
		Objects.requireNonNull(obbInfo, "obbIfno cannot be null");

		final int callingUid = Binder.getCallingUid();
		final ObbState obbState = new ObbState(rawPath, canonicalPath,
		@@ -3161,34 +3156,6 @@ 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");