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:
parent
cb2dfe43f2
commit
9046b84805
@ -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);
|
||||||
}
|
}
|
||||||
|
Loading…
Reference in New Issue
Block a user