diff options
author | Rohan Shah <shahrk@google.com> | 2016-08-17 11:08:33 -0700 |
---|---|---|
committer | SteadyQuad <SteadyQuad@gmail.com> | 2016-10-21 22:29:02 +0200 |
commit | b303267c625aa39dff8c15e6c6bae9642dc7ac43 (patch) | |
tree | 6f06ce3d71c324f31f5b0a1eafa7ad5757a643a4 | |
parent | f09282b4ccc1aab8fa2eb9d83d4a86789636114b (diff) | |
download | android_packages_apps_Email-b303267c625aa39dff8c15e6c6bae9642dc7ac43.tar.gz android_packages_apps_Email-b303267c625aa39dff8c15e6c6bae9642dc7ac43.tar.bz2 android_packages_apps_Email-b303267c625aa39dff8c15e6c6bae9642dc7ac43.zip |
DO NOT MERGE 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: Ibe87479fd798da7da0e8809e37a39a4dfc708658
(cherry picked from commit 4d43d4ae1c16b7bb90194abe27b44a21fe72d3b6)
-rw-r--r-- | src/com/android/email/provider/AttachmentProvider.java | 14 |
1 files changed, 9 insertions, 5 deletions
diff --git a/src/com/android/email/provider/AttachmentProvider.java b/src/com/android/email/provider/AttachmentProvider.java index 3b5b38ef7..1a9f16abf 100644 --- a/src/com/android/email/provider/AttachmentProvider.java +++ b/src/com/android/email/provider/AttachmentProvider.java @@ -184,8 +184,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)); @@ -194,8 +194,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) { @@ -236,9 +235,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); } |