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

Commit c4156e0e authored by Jeff Sharkey's avatar Jeff Sharkey
Browse files

Recover shady content:// paths. 

The path-permission element offers prefix or regex style matching of
paths, but most providers internally use UriMatcher to decide what
to do with an incoming Uri.

This causes trouble because UriMatcher uses Uri.getPathSegments(),
which quietly ignores "empty" paths.  Consider this example:

    <path-permission android:pathPrefix="/private" ... />

    uriMatcher.addURI("com.example", "/private", CODE_PRIVATE);

    content://com.example//private

The Uri above will pass the security check, since it's not
technically a prefix match.  But the UriMatcher will then match it
as CODE_PRIVATE, since it ignores the "//" zero-length path.

Since we can't safely change the behavior of either path-permission
or UriMatcher, we're left with recovering these shady paths by
trimming away zero-length paths.

Bug: 112555574
Test: atest android.appsecurity.cts.AppSecurityTests
Test: atest FrameworksCoreTests:android.content.ContentProviderTest
Change-Id: Ibadbfa4fc904ec54780c8102958735b03293fb9a
parent 7a96ec0e

core/java/android/content/ContentProvider.java

+33 −20
Original line number Diff line number Diff line
@@ -51,6 +51,8 @@ import android.os.storage.StorageManager; 
import android.text.TextUtils;

import android.util.Log;



import com.android.internal.annotations.VisibleForTesting;



import java.io.File;

import java.io.FileDescriptor;

import java.io.FileNotFoundException;
@@ -58,6 +60,7 @@ import java.io.IOException; 
import java.io.PrintWriter;

import java.util.ArrayList;

import java.util.Arrays;

import java.util.Objects;



/**

 * Content providers are one of the primary building blocks of Android applications, providing
@@ -220,7 +223,7 @@ public abstract class ContentProvider implements ComponentCallbacks2 { 
        @Override

        public Cursor query(String callingPkg, Uri uri, @Nullable String[] projection,

                @Nullable Bundle queryArgs, @Nullable ICancellationSignal cancellationSignal) {

            validateIncomingUri(uri);

            uri = validateIncomingUri(uri);

            uri = maybeGetUriWithoutUserId(uri);

            if (enforceReadPermission(callingPkg, uri, null) != AppOpsManager.MODE_ALLOWED) {

                // The caller has no access to the data, so return an empty cursor with
@@ -268,7 +271,7 @@ public abstract class ContentProvider implements ComponentCallbacks2 { 
        @Override

        public String getType(Uri uri) {

            // getCallingPackage() isn't available in getType(), as the javadoc states.

            validateIncomingUri(uri);

            uri = validateIncomingUri(uri);

            uri = maybeGetUriWithoutUserId(uri);

            Trace.traceBegin(TRACE_TAG_DATABASE, "getType");

            try {
@@ -280,7 +283,7 @@ public abstract class ContentProvider implements ComponentCallbacks2 { 


        @Override

        public Uri insert(String callingPkg, Uri uri, ContentValues initialValues) {

            validateIncomingUri(uri);

            uri = validateIncomingUri(uri);

            int userId = getUserIdFromUri(uri);

            uri = maybeGetUriWithoutUserId(uri);

            if (enforceWritePermission(callingPkg, uri, null) != AppOpsManager.MODE_ALLOWED) {
@@ -303,7 +306,7 @@ public abstract class ContentProvider implements ComponentCallbacks2 { 


        @Override

        public int bulkInsert(String callingPkg, Uri uri, ContentValues[] initialValues) {

            validateIncomingUri(uri);

            uri = validateIncomingUri(uri);

            uri = maybeGetUriWithoutUserId(uri);

            if (enforceWritePermission(callingPkg, uri, null) != AppOpsManager.MODE_ALLOWED) {

                return 0;
@@ -327,11 +330,11 @@ public abstract class ContentProvider implements ComponentCallbacks2 { 
            for (int i = 0; i < numOperations; i++) {

                ContentProviderOperation operation = operations.get(i);

                Uri uri = operation.getUri();

                validateIncomingUri(uri);

                userIds[i] = getUserIdFromUri(uri);

                if (userIds[i] != UserHandle.USER_CURRENT) {

                    // Removing the user id from the uri.

                    operation = new ContentProviderOperation(operation, true);

                uri = validateIncomingUri(uri);

                uri = maybeGetUriWithoutUserId(uri);

                // Rebuild operation if we changed the Uri above

                if (!Objects.equals(operation.getUri(), uri)) {

                    operation = new ContentProviderOperation(operation, uri);

                    operations.set(i, operation);

                }

                if (operation.isReadOperation()) {
@@ -368,7 +371,7 @@ public abstract class ContentProvider implements ComponentCallbacks2 { 


        @Override

        public int delete(String callingPkg, Uri uri, String selection, String[] selectionArgs) {

            validateIncomingUri(uri);

            uri = validateIncomingUri(uri);

            uri = maybeGetUriWithoutUserId(uri);

            if (enforceWritePermission(callingPkg, uri, null) != AppOpsManager.MODE_ALLOWED) {

                return 0;
@@ -386,7 +389,7 @@ public abstract class ContentProvider implements ComponentCallbacks2 { 
        @Override

        public int update(String callingPkg, Uri uri, ContentValues values, String selection,

                String[] selectionArgs) {

            validateIncomingUri(uri);

            uri = validateIncomingUri(uri);

            uri = maybeGetUriWithoutUserId(uri);

            if (enforceWritePermission(callingPkg, uri, null) != AppOpsManager.MODE_ALLOWED) {

                return 0;
@@ -405,7 +408,7 @@ public abstract class ContentProvider implements ComponentCallbacks2 { 
        public ParcelFileDescriptor openFile(

                String callingPkg, Uri uri, String mode, ICancellationSignal cancellationSignal,

                IBinder callerToken) throws FileNotFoundException {

            validateIncomingUri(uri);

            uri = validateIncomingUri(uri);

            uri = maybeGetUriWithoutUserId(uri);

            enforceFilePermission(callingPkg, uri, mode, callerToken);

            Trace.traceBegin(TRACE_TAG_DATABASE, "openFile");
@@ -423,7 +426,7 @@ public abstract class ContentProvider implements ComponentCallbacks2 { 
        public AssetFileDescriptor openAssetFile(

                String callingPkg, Uri uri, String mode, ICancellationSignal cancellationSignal)

                throws FileNotFoundException {

            validateIncomingUri(uri);

            uri = validateIncomingUri(uri);

            uri = maybeGetUriWithoutUserId(uri);

            enforceFilePermission(callingPkg, uri, mode, null);

            Trace.traceBegin(TRACE_TAG_DATABASE, "openAssetFile");
@@ -454,7 +457,7 @@ public abstract class ContentProvider implements ComponentCallbacks2 { 
        @Override

        public String[] getStreamTypes(Uri uri, String mimeTypeFilter) {

            // getCallingPackage() isn't available in getType(), as the javadoc states.

            validateIncomingUri(uri);

            uri = validateIncomingUri(uri);

            uri = maybeGetUriWithoutUserId(uri);

            Trace.traceBegin(TRACE_TAG_DATABASE, "getStreamTypes");

            try {
@@ -468,7 +471,7 @@ public abstract class ContentProvider implements ComponentCallbacks2 { 
        public AssetFileDescriptor openTypedAssetFile(String callingPkg, Uri uri, String mimeType,

                Bundle opts, ICancellationSignal cancellationSignal) throws FileNotFoundException {

            Bundle.setDefusable(opts, true);

            validateIncomingUri(uri);

            uri = validateIncomingUri(uri);

            uri = maybeGetUriWithoutUserId(uri);

            enforceFilePermission(callingPkg, uri, "r", null);

            Trace.traceBegin(TRACE_TAG_DATABASE, "openTypedAssetFile");
@@ -489,7 +492,7 @@ public abstract class ContentProvider implements ComponentCallbacks2 { 


        @Override

        public Uri canonicalize(String callingPkg, Uri uri) {

            validateIncomingUri(uri);

            uri = validateIncomingUri(uri);

            int userId = getUserIdFromUri(uri);

            uri = getUriWithoutUserId(uri);

            if (enforceReadPermission(callingPkg, uri, null) != AppOpsManager.MODE_ALLOWED) {
@@ -507,7 +510,7 @@ public abstract class ContentProvider implements ComponentCallbacks2 { 


        @Override

        public Uri uncanonicalize(String callingPkg, Uri uri) {

            validateIncomingUri(uri);

            uri = validateIncomingUri(uri);

            int userId = getUserIdFromUri(uri);

            uri = getUriWithoutUserId(uri);

            if (enforceReadPermission(callingPkg, uri, null) != AppOpsManager.MODE_ALLOWED) {
@@ -526,7 +529,7 @@ public abstract class ContentProvider implements ComponentCallbacks2 { 
        @Override

        public boolean refresh(String callingPkg, Uri uri, Bundle args,

                ICancellationSignal cancellationSignal) throws RemoteException {

            validateIncomingUri(uri);

            uri = validateIncomingUri(uri);

            uri = getUriWithoutUserId(uri);

            if (enforceReadPermission(callingPkg, uri, null) != AppOpsManager.MODE_ALLOWED) {

                return false;
@@ -1965,7 +1968,7 @@ public abstract class ContentProvider implements ComponentCallbacks2 { 
         */

        if (mContext == null) {

            mContext = context;

            if (context != null) {

            if (context != null && mTransport != null) {

                mTransport.mAppOpsManager = (AppOpsManager) context.getSystemService(

                        Context.APP_OPS_SERVICE);

            }
@@ -2074,7 +2077,8 @@ public abstract class ContentProvider implements ComponentCallbacks2 { 
    }



    /** @hide */

    private void validateIncomingUri(Uri uri) throws SecurityException {

    @VisibleForTesting

    public Uri validateIncomingUri(Uri uri) throws SecurityException {

        String auth = uri.getAuthority();

        if (!mSingleUser) {

            int userId = getUserIdFromAuthority(auth, UserHandle.USER_CURRENT);
@@ -2093,6 +2097,15 @@ public abstract class ContentProvider implements ComponentCallbacks2 { 
            }

            throw new SecurityException(message);

        }



        // Normalize the path by removing any empty path segments, which can be

        // a source of security issues.

        final String encodedPath = uri.getEncodedPath();

        if (encodedPath != null && encodedPath.indexOf("//") != -1) {

            return uri.buildUpon().encodedPath(encodedPath.replaceAll("//+", "/")).build();

        } else {

            return uri;

        }

    }



    /** @hide */

core/java/android/content/ContentProviderOperation.java

+2 −14
Original line number Diff line number Diff line
@@ -101,13 +101,9 @@ public class ContentProviderOperation implements Parcelable { 
    }



    /** @hide */

    public ContentProviderOperation(ContentProviderOperation cpo, boolean removeUserIdFromUri) {

    public ContentProviderOperation(ContentProviderOperation cpo, Uri withUri) {

        mType = cpo.mType;

        if (removeUserIdFromUri) {

            mUri = ContentProvider.getUriWithoutUserId(cpo.mUri);

        } else {

            mUri = cpo.mUri;

        }

        mUri = withUri;

        mValues = cpo.mValues;

        mSelection = cpo.mSelection;

        mSelectionArgs = cpo.mSelectionArgs;
@@ -117,14 +113,6 @@ public class ContentProviderOperation implements Parcelable { 
        mYieldAllowed = cpo.mYieldAllowed;

    }



    /** @hide */

    public ContentProviderOperation getWithoutUserIdInUri() {

        if (ContentProvider.uriHasUserId(mUri)) {

            return new ContentProviderOperation(this, true);

        }

        return this;

    }



    public void writeToParcel(Parcel dest, int flags) {

        dest.writeInt(mType);

        Uri.writeToParcel(dest, mUri);

core/tests/coretests/src/android/content/ContentProviderTest.java

0 → 100644
+88 −0
Original line number Diff line number Diff line 
/*

 * Copyright (C) 2018 The Android Open Source Project

 *

 * Licensed under the Apache License, Version 2.0 (the "License");

 * you may not use this file except in compliance with the License.

 * You may obtain a copy of the License at

 *

 *      http://www.apache.org/licenses/LICENSE-2.0

 *

 * Unless required by applicable law or agreed to in writing, software

 * distributed under the License is distributed on an "AS IS" BASIS,

 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

 * See the License for the specific language governing permissions and

 * limitations under the License

 */



package android.content;



import static org.junit.Assert.assertEquals;

import static org.mockito.Mockito.mock;

import static org.mockito.Mockito.withSettings;



import android.content.pm.ApplicationInfo;

import android.content.pm.ProviderInfo;

import android.net.Uri;

import android.support.test.runner.AndroidJUnit4;



import org.junit.Before;

import org.junit.Test;

import org.junit.runner.RunWith;

import org.mockito.Answers;



@RunWith(AndroidJUnit4.class)

public class ContentProviderTest {



    private Context mContext;

    private ContentProvider mCp;



    private ApplicationInfo mProviderApp;

    private ProviderInfo mProvider;



    @Before

    public void setUp() {

        mProviderApp = new ApplicationInfo();

        mProviderApp.uid = 10001;



        mProvider = new ProviderInfo();

        mProvider.authority = "com.example";

        mProvider.applicationInfo = mProviderApp;



        mContext = mock(Context.class);



        mCp = mock(ContentProvider.class, withSettings().defaultAnswer(Answers.CALLS_REAL_METHODS));

        mCp.attachInfo(mContext, mProvider);

    }



    @Test

    public void testValidateIncomingUri_Normal() throws Exception {

        assertEquals(Uri.parse("content://com.example/"),

                mCp.validateIncomingUri(Uri.parse("content://com.example/")));

        assertEquals(Uri.parse("content://com.example/foo/bar"),

                mCp.validateIncomingUri(Uri.parse("content://com.example/foo/bar")));

        assertEquals(Uri.parse("content://com.example/foo%2Fbar"),

                mCp.validateIncomingUri(Uri.parse("content://com.example/foo%2Fbar")));

        assertEquals(Uri.parse("content://com.example/foo%2F%2Fbar"),

                mCp.validateIncomingUri(Uri.parse("content://com.example/foo%2F%2Fbar")));

    }



    @Test

    public void testValidateIncomingUri_Shady() throws Exception {

        assertEquals(Uri.parse("content://com.example/"),

                mCp.validateIncomingUri(Uri.parse("content://com.example//")));

        assertEquals(Uri.parse("content://com.example/foo/bar/"),

                mCp.validateIncomingUri(Uri.parse("content://com.example//foo//bar//")));

        assertEquals(Uri.parse("content://com.example/foo/bar/"),

                mCp.validateIncomingUri(Uri.parse("content://com.example/foo///bar/")));

        assertEquals(Uri.parse("content://com.example/foo%2F%2Fbar/baz"),

                mCp.validateIncomingUri(Uri.parse("content://com.example/foo%2F%2Fbar//baz")));

    }



    @Test

    public void testValidateIncomingUri_NonPath() throws Exception {

        // We only touch paths; queries and fragments are left intact

        assertEquals(Uri.parse("content://com.example/foo/bar?foo=b//ar#foo=b//ar"),

                mCp.validateIncomingUri(

                        Uri.parse("content://com.example/foo/bar?foo=b//ar#foo=b//ar")));

    }

}