summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRohan Shah <shahrk@google.com>2016-08-17 11:08:33 -0700
committerSteadyQuad <SteadyQuad@gmail.com>2016-10-21 22:29:02 +0200
commitb303267c625aa39dff8c15e6c6bae9642dc7ac43 (patch)
tree6f06ce3d71c324f31f5b0a1eafa7ad5757a643a4
parentf09282b4ccc1aab8fa2eb9d83d4a86789636114b (diff)
downloadandroid_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.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 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);
}