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
diff --git a/provider_src/com/android/email/provider/AttachmentProvider.java b/provider_src/com/android/email/provider/AttachmentProvider.java
index c64fb4e..0abed97 100644
--- a/provider_src/com/android/email/provider/AttachmentProvider.java
+++ b/provider_src/com/android/email/provider/AttachmentProvider.java
@@ -166,8 +166,8 @@
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 @@
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 @@
}
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);
}
