From 69ed579fb8092395c4ffeb64ff5147622def3d4a Mon Sep 17 00:00:00 2001 From: Tom Taylor Date: Mon, 5 Dec 2016 13:57:45 -0800 Subject: [PATCH] 32161610 Security Vulnerability - Information disclosure vulnerability in AOSP Messaging * Check to make sure the returned uri from the gallery picker does not point to bugle's data directory (or any subdir). * Test: Manual- * I created the test app in the bug, the one that injects the bad uri into Bugle. I verified the bad behavior before the fix and the good behavior after. * I tested the gallery to make sure picking photos, from the photos app and drive, still work. * I verified the behavior in the debugger to be sure the code is catching the bad uri from the test app. Change-Id: I3393f3b886c837a49758b91945cf1e17ec9bee41 Fixes: 32161610 --- .../ui/mediapicker/DocumentImagePicker.java | 17 +++++++++++++++++ src/com/android/messaging/util/FileUtil.java | 8 ++++++++ 2 files changed, 25 insertions(+) diff --git a/src/com/android/messaging/ui/mediapicker/DocumentImagePicker.java b/src/com/android/messaging/ui/mediapicker/DocumentImagePicker.java index 2c36752..dff59cf 100644 --- a/src/com/android/messaging/ui/mediapicker/DocumentImagePicker.java +++ b/src/com/android/messaging/ui/mediapicker/DocumentImagePicker.java @@ -24,8 +24,13 @@ import android.os.Bundle; import com.android.messaging.Factory; import com.android.messaging.datamodel.data.PendingAttachmentData; import com.android.messaging.ui.UIIntents; +import com.android.messaging.util.LogUtil; +import com.android.messaging.util.FileUtil; import com.android.messaging.util.ImageUtils; import com.android.messaging.util.SafeAsyncTask; +import com.android.messaging.util.UriUtil; + +import java.io.File; /** * Wraps around the functionalities to allow the user to pick images from the document @@ -111,12 +116,24 @@ public class DocumentImagePicker { new SafeAsyncTask() { @Override protected String doInBackgroundTimed(final Void... params) { + if (UriUtil.isFileUri(documentUri) && + FileUtil.isInDataDir(new File(documentUri.getPath()))) { + // hacker sending private app data. Bail out + if (LogUtil.isLoggable(LogUtil.BUGLE_TAG, LogUtil.ERROR)) { + LogUtil.e(LogUtil.BUGLE_TAG, "Aborting attach of private app data (" + + documentUri + ")"); + } + return null; + } return ImageUtils.getContentType( Factory.get().getApplicationContext().getContentResolver(), documentUri); } @Override protected void onPostExecute(final String contentType) { + if (contentType == null) { + return; // bad uri on input + } // Ask the listener to create a temporary placeholder item to show the progress. final PendingAttachmentData pendingItem = PendingAttachmentData.createPendingAttachmentData(contentType, diff --git a/src/com/android/messaging/util/FileUtil.java b/src/com/android/messaging/util/FileUtil.java index 7c47ae9..b147b25 100644 --- a/src/com/android/messaging/util/FileUtil.java +++ b/src/com/android/messaging/util/FileUtil.java @@ -17,6 +17,7 @@ package com.android.messaging.util; import android.content.Context; +import android.os.Environment; import android.webkit.MimeTypeMap; import com.android.messaging.Factory; @@ -116,6 +117,13 @@ public class FileUtil { } } + // Checks if the file is in /data, and don't allow any app to send personal information. + // We're told it's possible to create world readable hardlinks to other apps private data + // so we ban all /data file uris. b/28793303 + public static boolean isInDataDir(File file) { + return isSameOrSubDirectory(Environment.getDataDirectory(), file); + } + /** * Checks, whether the child directory is the same as, or a sub-directory of the base * directory.