summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRohan Shah <shahrk@google.com>2016-08-17 11:08:33 -0700
committerRohan Shah <shahrk@google.com>2016-08-17 11:22:06 -0700
commit4d43d4ae1c16b7bb90194abe27b44a21fe72d3b6 (patch)
tree1fc0f79828b445b2c383a2f3b43e0d348d572e83
parenta17c6c6dddd79883223208a921f2faf9b7966b26 (diff)
downloadandroid_packages_apps_Email-4d43d4ae1c16b7bb90194abe27b44a21fe72d3b6.tar.gz
android_packages_apps_Email-4d43d4ae1c16b7bb90194abe27b44a21fe72d3b6.tar.bz2
android_packages_apps_Email-4d43d4ae1c16b7bb90194abe27b44a21fe72d3b6.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
-rw-r--r--src/com/android/email/provider/AttachmentProvider.java14
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 c64fb4e4c..0abed9712 100644
--- a/src/com/android/email/provider/AttachmentProvider.java
+++ b/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);
}