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)
This commit is contained in:
Rohan Shah 2016-08-17 11:23:26 -07:00 committed by gitbuildkicker
parent e81af0322a
commit a549245b2f
1 changed files with 9 additions and 5 deletions

View File

@ -166,8 +166,8 @@ public class AttachmentProvider extends ContentProvider {
long callingId = Binder.clearCallingIdentity(); long callingId = Binder.clearCallingIdentity();
try { try {
List<String> segments = uri.getPathSegments(); List<String> segments = uri.getPathSegments();
String accountId = segments.get(0); final long accountId = Long.parseLong(segments.get(0));
String id = segments.get(1); final long id = Long.parseLong(segments.get(1));
String format = segments.get(2); String format = segments.get(2);
if (AttachmentUtilities.FORMAT_THUMBNAIL.equals(format)) { if (AttachmentUtilities.FORMAT_THUMBNAIL.equals(format)) {
int width = Integer.parseInt(segments.get(3)); int width = Integer.parseInt(segments.get(3));
@ -176,8 +176,7 @@ public class AttachmentProvider extends ContentProvider {
File dir = getContext().getCacheDir(); File dir = getContext().getCacheDir();
File file = new File(dir, filename); File file = new File(dir, filename);
if (!file.exists()) { if (!file.exists()) {
Uri attachmentUri = AttachmentUtilities. Uri attachmentUri = AttachmentUtilities.getAttachmentUri(accountId, id);
getAttachmentUri(Long.parseLong(accountId), Long.parseLong(id));
Cursor c = query(attachmentUri, Cursor c = query(attachmentUri,
new String[] { Columns.DATA }, null, null, null); new String[] { Columns.DATA }, null, null, null);
if (c != null) { if (c != null) {
@ -218,9 +217,14 @@ public class AttachmentProvider extends ContentProvider {
} }
else { else {
return ParcelFileDescriptor.open( 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); 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 { } finally {
Binder.restoreCallingIdentity(callingId); Binder.restoreCallingIdentity(callingId);
} }