Don't allow file attachment from file:///data.
Commit 24ed2941ab132e4156bd38f0ab734c81dae8fc2e allows file://
attachment on the /data directory if they are from the same process.
This was done to work around applications that shared their internal
data file. However, this is bad practice, and other apps should share
content:// Uri instead.
With this change, Email doesn't allow this anymore. This fixes
security issue 199888.
Also, add Analytics for these errors
compose_errors > send_intent_attachment > data_dir
https://code.google.com/p/android/issues/detail?id=199888
b/26989185
Change-Id: I7cae3389f4f7cf5f86600a58c6ccdffaf889d1c3
diff --git a/src/com/android/mail/compose/ComposeActivity.java b/src/com/android/mail/compose/ComposeActivity.java
index b7bab75..36456bf 100644
--- a/src/com/android/mail/compose/ComposeActivity.java
+++ b/src/com/android/mail/compose/ComposeActivity.java
@@ -191,6 +191,8 @@
private static final String EXTRA_CC = "cc";
private static final String EXTRA_BCC = "bcc";
+ public static final String ANALYTICS_CATEGORY_ERRORS = "compose_errors";
+
/**
* An optional extra containing a {@link ContentValues} of values to be added to
* {@link SendOrSaveMessage#mValues}.
@@ -275,9 +277,8 @@
@VisibleForTesting
public static final AtomicInteger PENDING_SEND_OR_SAVE_TASKS_NUM = new AtomicInteger(0);
- // String representing the uri of the data directory (used for attachment uri checking).
+ /* Path of the data directory (used for attachment uri checking). */
private static final String DATA_DIRECTORY_ROOT;
- private static final String ALTERNATE_DATA_DIRECTORY_ROOT;
// Static initializations
static {
@@ -286,7 +287,6 @@
SEND_SAVE_TASK_HANDLER = new Handler(handlerThread.getLooper());
DATA_DIRECTORY_ROOT = Environment.getDataDirectory().toString();
- ALTERNATE_DATA_DIRECTORY_ROOT = DATA_DIRECTORY_ROOT + DATA_DIRECTORY_ROOT;
}
private final Rect mRect = new Rect();
@@ -1143,7 +1143,8 @@
TextUtils.isEmpty(htmlInPlainText)) {
LogUtils.w(LOG_TAG, "FAILED HTML CONVERSION: from %d to %d", message.bodyText.length(),
htmlInPlainText.length());
- Analytics.getInstance().sendEvent("errors", "failed_html_conversion", null, 0);
+ Analytics.getInstance().sendEvent(ANALYTICS_CATEGORY_ERRORS,
+ "failed_html_conversion", null, 0);
message.bodyHtml = "<p>" + message.bodyText + "</p>";
}
message.embedsExternalResources = false;
@@ -1921,35 +1922,14 @@
try {
if (uri != null) {
if ("file".equals(uri.getScheme())) {
+ // We must not allow files from /data, even from our process.
final File f = new File(uri.getPath());
- // We should not be attaching any files from the data directory UNLESS
- // the data directory is part of the calling process.
final String filePath = f.getCanonicalPath();
if (filePath.startsWith(DATA_DIRECTORY_ROOT)) {
- final String callingPackage = getCallingPackage();
- if (callingPackage == null) {
- showErrorToast(getString(R.string.attachment_permission_denied));
- continue;
- }
-
- // So it looks like the data directory are usually /data/data, but
- // DATA_DIRECTORY_ROOT is only /data.. so let's check for both
- final String pathWithoutRoot;
- // We add 1 to the length for the additional / before the package name.
- if (filePath.startsWith(ALTERNATE_DATA_DIRECTORY_ROOT)) {
- pathWithoutRoot = filePath.substring(
- ALTERNATE_DATA_DIRECTORY_ROOT.length() + 1);
- } else {
- pathWithoutRoot = filePath.substring(
- DATA_DIRECTORY_ROOT.length() + 1);
- }
-
- // If we are trying to access a data package that's not part of the
- // calling package, show error toast and ignore this attachment.
- if (!pathWithoutRoot.startsWith(callingPackage)) {
- showErrorToast(getString(R.string.attachment_permission_denied));
- continue;
- }
+ showErrorToast(getString(R.string.attachment_permission_denied));
+ Analytics.getInstance().sendEvent(ANALYTICS_CATEGORY_ERRORS,
+ "send_intent_attachment", "data_dir", 0);
+ continue;
}
}
if (!handleSpecialAttachmentUri(uri)) {
