Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 017c7f8e authored by Priyanka Advani (xWF)'s avatar Priyanka Advani (xWF) Committed by Android (Google) Code Review
Browse files

Revert "Validate obbInfo in StorageManagerService" 

This reverts commit 6fe098a0.

Reason for revert: Droidmonitor created revert due to b/363234644. ACA verified the culprit.

Change-Id: Ib9b255d6074ce61acb019f8cc493dedeb0ed026b
parent 6fe098a0

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");