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

Commit f773d8ed authored by Gary Mai's avatar Gary Mai Committed by syphyr
Browse files

Patch URI vulnerability in contact photo editing 

Don't allow reading of "file://" URIs that don't point to "/storage" during the
photo saving flow.

This is to prevent malicious apps from asking us to read our own private
files which we copy into a temporary "content://" URI that we give to a
cropping app (with permission to read).

Fixing here patches both PhotoSelectionHandler.java and
AttachPhotoActivity.java.

Tested:
Manual with the fake gallery app. Confirmed that selecting an "image"
with a URI of our own shared_pref file fails without reading it.
ContactPhotoUtilsTest

Bug: 113597344
Change-Id: Iabb4f8139cedb7d7b865d69a4b95a4997f64c71d
(cherry picked from commit ccfd94b9)
parent 2ec5da1f

src/com/android/contacts/util/ContactPhotoUtils.java

+18 −3
Original line number Diff line number Diff line
@@ -18,6 +18,7 @@ 
package com.android.contacts.util;



import android.content.ClipData;

import android.content.ContentResolver;

import android.content.Context;

import android.content.Intent;

import android.graphics.Bitmap;
@@ -27,10 +28,8 @@ import android.os.Environment; 
import android.provider.MediaStore;

import android.support.v4.content.FileProvider;

import android.util.Log;



import com.android.contacts.R;

import com.google.common.io.Closeables;



import java.io.ByteArrayOutputStream;

import java.io.File;

import java.io.FileNotFoundException;
@@ -148,7 +147,7 @@ public class ContactPhotoUtils { 
     */

    public static boolean savePhotoFromUriToUri(Context context, Uri inputUri, Uri outputUri,

            boolean deleteAfterSave) {

        if (inputUri == null || outputUri == null) {

        if (inputUri == null || outputUri == null || isFilePathAndNotStorage(inputUri)) {

            return false;

        }

        try (FileOutputStream outputStream = context.getContentResolver()
@@ -173,4 +172,20 @@ public class ContactPhotoUtils { 
        }

        return true;

    }



    /**

     * Returns {@code true} if the {@code inputUri} is a FILE scheme and it does not point to

     * the storage directory.

     */

    private static boolean isFilePathAndNotStorage(Uri inputUri) {

        if (ContentResolver.SCHEME_FILE.equals(inputUri.getScheme())) {

            try {

                File file = new File(inputUri.getPath()).getCanonicalFile();

                return !file.getCanonicalPath().startsWith("/storage/");

            } catch (IOException e) {

                return false;

            }

        }

        return false;

    }

}

tests/src/com/android/contacts/util/ContactPhotoUtilsTest.java

0 → 100644
+49 −0
Original line number Diff line number Diff line 
package com.android.contacts.util;



import android.net.Uri;

import android.test.AndroidTestCase;

import android.test.suitebuilder.annotation.SmallTest;



/**

 * Test cases for {@link ContactPhotoUtils}.

 *

 * adb shell am instrument -w -e class com.android.contacts.util.ContactPhotoUtilsTest \

 *   com.android.contacts.tests/android.test.InstrumentationTestRunner

 */

@SmallTest

public class ContactPhotoUtilsTest extends AndroidTestCase {



  private Uri tempUri;



  @Override

  protected void setUp() throws Exception {

    tempUri = ContactPhotoUtils.generateTempImageUri(getContext());

  }



  protected void tearDown() throws Exception {

    getContext().getContentResolver().delete(tempUri, null, null);

  }



  public void testFileUriDataPathFails() {

    String filePath =

        "file:///data/data/com.android.contacts/shared_prefs/com.android.contacts.xml";



    assertFalse(

        ContactPhotoUtils.savePhotoFromUriToUri(getContext(), Uri.parse(filePath), tempUri, false));

  }



  public void testFileUriCanonicalDataPathFails() {

    String filePath =

        "file:///storage/../data/data/com.android.contacts/shared_prefs/com.android.contacts.xml";



    assertFalse(

        ContactPhotoUtils.savePhotoFromUriToUri(getContext(), Uri.parse(filePath), tempUri, false));

  }



  public void testContentUriInternalPasses() {

    Uri internal = ContactPhotoUtils.generateTempImageUri(getContext());



    assertTrue(

        ContactPhotoUtils.savePhotoFromUriToUri(getContext(), internal, tempUri, true));

  }

}