Limit account id and id to longsandroid-cts_7.1_r1android-cts-7.1_r9android-cts-7.1_r8android-cts-7.1_r7android-cts-7.1_r6android-cts-7.1_r5android-cts-7.1_r4android-cts-7.1_r3android-cts-7.1_r29android-cts-7.1_r28android-cts-7.1_r27android-cts-7.1_r26android-cts-7.1_r25android-cts-7.1_r24android-cts-7.1_r23android-cts-7.1_r22android-cts-7.1_r21android-cts-7.1_r20android-cts-7.1_r2android-cts-7.1_r19android-cts-7.1_r18android-cts-7.1_r17android-cts-7.1_r16android-cts-7.1_r15android-cts-7.1_r14android-cts-7.1_r13android-cts-7.1_r12android-cts-7.1_r11android-cts-7.1_r10android-cts-7.1_r1android-7.1.1_r9android-7.1.1_r8android-7.1.1_r7android-7.1.1_r6android-7.1.1_r58android-7.1.1_r57android-7.1.1_r56android-7.1.1_r55android-7.1.1_r54android-7.1.1_r53android-7.1.1_r52android-7.1.1_r51android-7.1.1_r50android-7.1.1_r49android-7.1.1_r48android-7.1.1_r47android-7.1.1_r46android-7.1.1_r45android-7.1.1_r44android-7.1.1_r43android-7.1.1_r42android-7.1.1_r41android-7.1.1_r40android-7.1.1_r4android-7.1.1_r39android-7.1.1_r38android-7.1.1_r35android-7.1.1_r33android-7.1.1_r32android-7.1.1_r31android-7.1.1_r3android-7.1.1_r28android-7.1.1_r27android-7.1.1_r26android-7.1.1_r25android-7.1.1_r24android-7.1.1_r23android-7.1.1_r22android-7.1.1_r21android-7.1.1_r20android-7.1.1_r2android-7.1.1_r17android-7.1.1_r16android-7.1.1_r15android-7.1.1_r14android-7.1.1_r13android-7.1.1_r12android-7.1.1_r11android-7.1.1_r10android-7.1.1_r1android-7.1.0_r7android-7.1.0_r6android-7.1.0_r5android-7.1.0_r4android-7.1.0_r3android-7.1.0_r2android-7.1.0_r1nougat-mr1.7-releasenougat-mr1.6-releasenougat-mr1.5-releasenougat-mr1.4-releasenougat-mr1.3-releasenougat-mr1.2-releasenougat-mr1.1-releasenougat-mr1-volantis-releasenougat-mr1-releasenougat-mr1-flounder-releasenougat-mr1-cts-releasenougat-dr1-release

The security issue occurs because id is allowed to be an arbitrary
path instead of being limited to what it is -- a long. Both id
and account id are now parsed into longs (and if either fails, an
error will be logged and null will be returned).

Tested/verified error is logged using the reported attack.

BUG=30745403

Change-Id: Ia21418545bbaeb96fb5ab6c3f4e71858e57b8684

1 files changed, 9 insertions, 5 deletions

diff --git a/provider_src/com/android/email/provider/AttachmentProvider.java b/provider_src/com/android/email/provider/AttachmentProvider.java
index c64fb4e4c..0abed9712 100644
--- a/provider_src/com/android/email/provider/AttachmentProvider.java
+++ b/provider_src/com/android/email/provider/AttachmentProvider.java
@@ -166,8 +166,8 @@ public class AttachmentProvider extends ContentProvider {
         long callingId = Binder.clearCallingIdentity();
         try {
             List<String> segments = uri.getPathSegments();
-            String accountId = segments.get(0);
-            String id = segments.get(1);
+            final long accountId = Long.parseLong(segments.get(0));
+            final long id = Long.parseLong(segments.get(1));
             String format = segments.get(2);
             if (AttachmentUtilities.FORMAT_THUMBNAIL.equals(format)) {
                 int width = Integer.parseInt(segments.get(3));
@@ -176,8 +176,7 @@ public class AttachmentProvider extends ContentProvider {
                 File dir = getContext().getCacheDir();
                 File file = new File(dir, filename);
                 if (!file.exists()) {
-                    Uri attachmentUri = AttachmentUtilities.
-                        getAttachmentUri(Long.parseLong(accountId), Long.parseLong(id));
+                    Uri attachmentUri = AttachmentUtilities.getAttachmentUri(accountId, id);
                     Cursor c = query(attachmentUri,
                             new String[] { Columns.DATA }, null, null, null);
                     if (c != null) {
@@ -218,9 +217,14 @@ public class AttachmentProvider extends ContentProvider {
             }
             else {
                 return ParcelFileDescriptor.open(
-                        new File(getContext().getDatabasePath(accountId + ".db_att"), id),
+                        new File(getContext().getDatabasePath(accountId + ".db_att"),
+                                String.valueOf(id)),
                         ParcelFileDescriptor.MODE_READ_ONLY);
             }
+        } catch (NumberFormatException e) {
+            LogUtils.e(Logging.LOG_TAG,
+                    "AttachmentProvider.openFile: Failed to open as id is not a long");
+            return null;
         } finally {
             Binder.restoreCallingIdentity(callingId);
         }