Merge tag 'android-6.0.1_r72' into HEAD

Android 6.0.1 Release 72 (M4B30X)

# gpg: Signature made Tue 04 Oct 2016 09:47:46 AM PDT using DSA key ID 9AB10E78
# gpg: Can't check signature: public key not found

1 files changed, 28 insertions, 9 deletions

diff --git a/src/com/android/providers/telephony/MmsProvider.java b/src/com/android/providers/telephony/MmsProvider.java
index 9bb94eb..ccd9ab1 100644
--- a/src/com/android/providers/telephony/MmsProvider.java
+++ b/src/com/android/providers/telephony/MmsProvider.java
@@ -16,6 +16,7 @@
 
 package com.android.providers.telephony;
 
+import android.annotation.NonNull;
 import android.app.AppOpsManager;
 import android.content.ContentProvider;
 import android.content.ContentUris;
@@ -493,7 +494,11 @@ public class MmsProvider extends ContentProvider {
 
     @Override
     public Uri insert(Uri uri, ContentValues values) {
-        // Don't let anyone insert anything with the _data column
+        // The _data column is filled internally in MmsProvider, so this check is just to avoid
+        // it from being inadvertently set. This is not supposed to be a protection against
+        // malicious attack, since sql injection could still be attempted to bypass the check. On
+        // the other hand, the MmsProvider does verify that the _data column has an allowed value
+        // before opening any uri/files.
         if (values != null && values.containsKey(Part._DATA)) {
             return null;
         }
@@ -939,9 +944,12 @@ public class MmsProvider extends ContentProvider {
     }
 
     @Override
-    public int update(Uri uri, ContentValues values,
-            String selection, String[] selectionArgs) {
-        // Don't let anyone update the _data column
+    public int update(Uri uri, ContentValues values, String selection, String[] selectionArgs) {
+        // The _data column is filled internally in MmsProvider, so this check is just to avoid
+        // it from being inadvertently set. This is not supposed to be a protection against
+        // malicious attack, since sql injection could still be attempted to bypass the check. On
+        // the other hand, the MmsProvider does verify that the _data column has an allowed value
+        // before opening any uri/files.
         if (values != null && values.containsKey(Part._DATA)) {
             return 0;
         }
@@ -1047,7 +1055,12 @@ public class MmsProvider extends ContentProvider {
             return null;
         }
 
-        // Verify that the _data path points to mms data
+        return safeOpenFileHelper(uri, mode);
+    }
+
+    @NonNull
+    private ParcelFileDescriptor safeOpenFileHelper(
+            @NonNull Uri uri, @NonNull String mode) throws FileNotFoundException {
         Cursor c = query(uri, new String[]{"_data"}, null, null, null);
         int count = (c != null) ? c.getCount() : 0;
         if (count != 1) {
@@ -1068,10 +1081,16 @@ public class MmsProvider extends ContentProvider {
         c.close();
 
         if (path == null) {
-            return null;
+            throw new FileNotFoundException("Column _data not found.");
         }
+
+        File filePath = new File(path);
         try {
-            File filePath = new File(path);
+            // The MmsProvider shouldn't open a file that isn't MMS data, so we verify that the
+            // _data path actually points to MMS data. That safeguards ourselves from callers who
+            // inserted or updated a URI (more specifically the _data column) with disallowed paths.
+            // TODO(afurtado): provide a more robust mechanism to avoid disallowed _data paths to
+            // be inserted/updated in the first place, including via SQL injection.
             if (!filePath.getCanonicalPath()
                     .startsWith(getContext().getDir(PARTS_DIR_NAME, 0).getCanonicalPath())) {
                 Log.e(TAG, "openFile: path "
@@ -1079,7 +1098,6 @@ public class MmsProvider extends ContentProvider {
                         + " does not start with "
                         + getContext().getDir(PARTS_DIR_NAME, 0).getCanonicalPath());
                 // Don't care return value
-                filePath.delete();
                 return null;
             }
         } catch (IOException e) {
@@ -1087,7 +1105,8 @@ public class MmsProvider extends ContentProvider {
             return null;
         }
 
-        return openFileHelper(uri, mode);
+        int modeBits = ParcelFileDescriptor.parseMode(mode);
+        return ParcelFileDescriptor.open(filePath, modeBits);
     }
 
     private void filterUnsupportedKeys(ContentValues values) {