From e81af0322a9dc5515e293bc8bd8826f34575b5fa Mon Sep 17 00:00:00 2001 From: Andy Huang Date: Fri, 1 Jul 2016 16:56:19 -0700 Subject: [PATCH 1/2] stop exporting EmailAccountCacheProvider Bug: 29767043 Change-Id: Ib9f16385a5e63557b6f293d148e49c9ad044c9b4 --- AndroidManifest.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AndroidManifest.xml b/AndroidManifest.xml index c3e33ab37..bbe02d4ae 100644 --- a/AndroidManifest.xml +++ b/AndroidManifest.xml @@ -393,7 +393,7 @@ From a549245b2f1e6510c6a87a1f381437bae30ee51b Mon Sep 17 00:00:00 2001 From: Rohan Shah Date: Wed, 17 Aug 2016 11:23:26 -0700 Subject: [PATCH 2/2] Limit account id and id to longs 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 (cherry picked from commit 9794d7e8216138adf143a3b6faf3d5683316a662) --- .../android/email/provider/AttachmentProvider.java | 14 +++++++++----- 1 file 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 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); }