From 2283165c23b9162e99b817400889fe541799626f Mon Sep 17 00:00:00 2001 From: Jake Klinker Date: Thu, 21 Apr 2022 16:46:18 +0000 Subject: [PATCH 1/5] Fix url:file:// style URIs from not being detected in UriUtil. Change-Id: I81c01202306d856f6f8f8b74a5a28d7c1011fcec Tested: Was no longer able to repro b/222091734. Bug: 222091734 --- src/com/android/messaging/util/UriUtil.java | 36 ++++++++++++++++----- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/src/com/android/messaging/util/UriUtil.java b/src/com/android/messaging/util/UriUtil.java index 6e39749..f92155f 100644 --- a/src/com/android/messaging/util/UriUtil.java +++ b/src/com/android/messaging/util/UriUtil.java @@ -49,6 +49,8 @@ public class UriUtil { private static final String SCHEME_MMSTO = "smsto"; public static final HashSet SMS_MMS_SCHEMES = new HashSet( Arrays.asList(SCHEME_SMS, SCHEME_MMS, SCHEME_SMSTO, SCHEME_MMSTO)); + private static final String SCHEME_HTTP = "http"; + private static final String SCHEME_HTTPS = "https"; public static final String SCHEME_BUGLE = "bugle"; public static final HashSet SUPPORTED_SCHEME = new HashSet( @@ -98,8 +100,7 @@ public class UriUtil { public static boolean isFileUri(final Uri uri) { return uri != null && uri.getScheme() != null && - TextUtils.equals(uri.getScheme().trim().toLowerCase(), - ContentResolver.SCHEME_FILE); + uri.getScheme().trim().toLowerCase().contains(ContentResolver.SCHEME_FILE); } /** @@ -216,9 +217,10 @@ public class UriUtil { inputStream = context.getContentResolver().openInputStream(sourceUri); } else { // The content is remote. Download it. - final URL url = new URL(sourceUri.toString()); - final URLConnection ucon = url.openConnection(); - inputStream = new BufferedInputStream(ucon.getInputStream()); + inputStream = getInputStreamFromRemoteUri(sourceUri); + if (inputStream == null) { + return null; + } } return persistContentToScratchSpace(inputStream); } catch (final Exception ex) { @@ -235,6 +237,23 @@ public class UriUtil { } } + @DoesNotRunOnMainThread + private static InputStream getInputStreamFromRemoteUri(final Uri sourceUri) + throws IOException { + if (isRemoteUri(sourceUri)) { + final URL url = new URL(sourceUri.toString()); + final URLConnection ucon = url.openConnection(); + return new BufferedInputStream(ucon.getInputStream()); + } else { + return null; + } + } + + private static boolean isRemoteUri(final Uri sourceUri) { + return sourceUri.getScheme().equals(SCHEME_HTTP) + || sourceUri.getScheme().equals(SCHEME_HTTPS); + } + /** * Persist a piece of content from the given input stream, byte by byte to the specified * directory. @@ -273,9 +292,10 @@ public class UriUtil { inputStream = context.getContentResolver().openInputStream(sourceUri); } else { // The content is remote. Download it. - final URL url = new URL(sourceUri.toString()); - final URLConnection ucon = url.openConnection(); - inputStream = new BufferedInputStream(ucon.getInputStream()); + inputStream = getInputStreamFromRemoteUri(sourceUri); + if (inputStream == null) { + return null; + } } return persistContent(inputStream, outputDir, contentType); } catch (final Exception ex) { From 79481ab146f666e2bef8ee202899b68cbf7b014c Mon Sep 17 00:00:00 2001 From: Chaohui Wang Date: Fri, 26 Aug 2022 20:58:50 +0800 Subject: [PATCH 2/5] Update DrawableWrapper to DrawableWrapperCompat Api changed in aosp/2120177 Bug: 235727273 Test: TAP Change-Id: I1645dab6000dc760b4de2bb625e0af8df014c8c8 --- src/com/android/messaging/util/SwitchCompatUtils.java | 6 +++--- src/com/android/messaging/util/TintDrawableWrapper.java | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/com/android/messaging/util/SwitchCompatUtils.java b/src/com/android/messaging/util/SwitchCompatUtils.java index 8cfe2bc..9c920f6 100644 --- a/src/com/android/messaging/util/SwitchCompatUtils.java +++ b/src/com/android/messaging/util/SwitchCompatUtils.java @@ -21,7 +21,7 @@ import android.content.res.ColorStateList; import android.graphics.Color; import android.graphics.PorterDuff; import android.graphics.drawable.Drawable; -import androidx.appcompat.graphics.drawable.DrawableWrapper; +import androidx.appcompat.graphics.drawable.DrawableWrapperCompat; import androidx.appcompat.widget.SwitchCompat; import android.util.TypedValue; @@ -53,8 +53,8 @@ public class SwitchCompatUtils { private static Drawable getColorTintedDrawable(Drawable oldDrawable, final ColorStateList colorStateList, final PorterDuff.Mode mode) { final int[] thumbState = oldDrawable.isStateful() ? oldDrawable.getState() : null; - if (oldDrawable instanceof DrawableWrapper) { - oldDrawable = ((DrawableWrapper) oldDrawable).getWrappedDrawable(); + if (oldDrawable instanceof DrawableWrapperCompat) { + oldDrawable = ((DrawableWrapperCompat) oldDrawable).getDrawable(); } final Drawable newDrawable = new TintDrawableWrapper(oldDrawable, colorStateList, mode); if (thumbState != null) { diff --git a/src/com/android/messaging/util/TintDrawableWrapper.java b/src/com/android/messaging/util/TintDrawableWrapper.java index 06ee7d4..b426f8c 100644 --- a/src/com/android/messaging/util/TintDrawableWrapper.java +++ b/src/com/android/messaging/util/TintDrawableWrapper.java @@ -20,16 +20,16 @@ import android.content.res.ColorStateList; import android.graphics.Color; import android.graphics.PorterDuff; import android.graphics.drawable.Drawable; -import androidx.appcompat.graphics.drawable.DrawableWrapper; +import androidx.appcompat.graphics.drawable.DrawableWrapperCompat; /* * This is directly copied from v7/appcompat/src/androidx.appcompat.internal/widget/TintManager.java */ /** - * A {@link DrawableWrapper} which updates it's color filter using a {@link ColorStateList}. + * A {@link DrawableWrapperCompat} which updates it's color filter using a {@link ColorStateList}. */ -class TintDrawableWrapper extends DrawableWrapper { +class TintDrawableWrapper extends DrawableWrapperCompat { private final ColorStateList mTintStateList; private final PorterDuff.Mode mTintMode; private int mCurrentColor; From 4b17ed7f8045452943081b16922caf23d47f157c Mon Sep 17 00:00:00 2001 From: Michael Groover Date: Tue, 4 Oct 2022 17:18:17 -0500 Subject: [PATCH 3/5] Add unaudited exported flag to exposed runtime receivers Android T allows apps to declare a runtime receiver as not exported by invoking registerReceiver with a new RECEIVER_NOT_EXPORTED flag; receivers registered with this flag will only receive broadcasts from the platform and the app itself. However to ensure developers can properly protect their receivers, all apps targeting U or later registering a receiver for non-system broadcasts must specify either the exported or not exported flag when invoking #registerReceiver; if one of these flags is not provided, the platform will throw a SecurityException. This commit updates all the exposed receivers with a new RECEIVER_EXPORTED_UNAUDITED flag to maintain the existing behavior of exporting the receiver while also flagging the receiver for audit before the U release. Bug: 234659204 Test: Build Change-Id: I15aba10fe12dfcd2e67330ca844491341ef6d920 --- src/android/support/v7/mms/MmsNetworkManager.java | 3 ++- src/com/android/messaging/BugleApplication.java | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/android/support/v7/mms/MmsNetworkManager.java b/src/android/support/v7/mms/MmsNetworkManager.java index 059ca8f..1021b5a 100644 --- a/src/android/support/v7/mms/MmsNetworkManager.java +++ b/src/android/support/v7/mms/MmsNetworkManager.java @@ -324,7 +324,8 @@ class MmsNetworkManager { private void registerConnectivityChangeReceiverLocked() { if (!mReceiverRegistered) { - mContext.registerReceiver(mConnectivityChangeReceiver, mConnectivityIntentFilter); + mContext.registerReceiver(mConnectivityChangeReceiver, mConnectivityIntentFilter, + Context.RECEIVER_EXPORTED/*UNAUDITED*/); mReceiverRegistered = true; } } diff --git a/src/com/android/messaging/BugleApplication.java b/src/com/android/messaging/BugleApplication.java index 0ef8d91..36f062b 100644 --- a/src/com/android/messaging/BugleApplication.java +++ b/src/com/android/messaging/BugleApplication.java @@ -132,7 +132,8 @@ public class BugleApplication extends Application implements UncaughtExceptionHa LogUtil.i(TAG, "Carrier config changed. Reloading MMS config."); MmsConfig.loadAsync(); } - }, new IntentFilter(CarrierConfigManager.ACTION_CARRIER_CONFIG_CHANGED)); + }, new IntentFilter(CarrierConfigManager.ACTION_CARRIER_CONFIG_CHANGED), + Context.RECEIVER_EXPORTED/*UNAUDITED*/); } private static void initMmsLib(final Context context, final BugleGservices bugleGservices, From 0d5452146c58aa9b938daffc88f8b03eb9aee58b Mon Sep 17 00:00:00 2001 From: Jake Klinker Date: Mon, 8 May 2023 23:07:07 +0000 Subject: [PATCH 4/5] Fix exposing private messages files through attachments with a content URI. Change-Id: I30b2a06c67af4a347d03c7504d13b9b9365acafd Tested: Was no longer able to repro b/275552292. Bug: 275552292 --- src/com/android/messaging/util/FileUtil.java | 25 ++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/com/android/messaging/util/FileUtil.java b/src/com/android/messaging/util/FileUtil.java index 71fbb4b..e7d86f2 100644 --- a/src/com/android/messaging/util/FileUtil.java +++ b/src/com/android/messaging/util/FileUtil.java @@ -20,6 +20,7 @@ import android.content.ContentResolver; import android.content.Context; import android.net.Uri; import android.os.Environment; +import android.os.ParcelFileDescriptor; import android.text.TextUtils; import com.android.messaging.Factory; @@ -28,6 +29,8 @@ import com.google.common.io.Files; import java.io.File; import java.io.IOException; +import java.nio.file.Path; +import java.nio.file.Paths; import java.text.SimpleDateFormat; import java.util.Date; import java.util.Locale; @@ -121,6 +124,10 @@ public class FileUtil { // We're told it's possible to create world readable hardlinks to other apps private data // so we ban all /data file uris. public static boolean isInPrivateDir(Uri uri) { + return isFileUriInPrivateDir(uri) || isContentUriInPrivateDir(uri); + } + + private static boolean isFileUriInPrivateDir(Uri uri) { if (!UriUtil.isFileUri(uri)) { return false; } @@ -128,6 +135,24 @@ public class FileUtil { return FileUtil.isSameOrSubDirectory(Environment.getDataDirectory(), file); } + private static boolean isContentUriInPrivateDir(Uri uri) { + if (!uri.getScheme().equals(ContentResolver.SCHEME_CONTENT)) { + return false; + } + try { + Context context = Factory.get().getApplicationContext(); + ParcelFileDescriptor pfd = context.getContentResolver().openFileDescriptor(uri, "r"); + int fd = pfd.getFd(); + // Use the file descriptor to find out the read file path through symbolic link. + Path fdPath = Paths.get("/proc/self/fd/" + fd); + Path filePath = java.nio.file.Files.readSymbolicLink(fdPath); + pfd.close(); + return FileUtil.isSameOrSubDirectory(Environment.getDataDirectory(), filePath.toFile()); + } catch (Exception e) { + return false; + } + } + /** * Checks, whether the child directory is the same as, or a sub-directory of the base * directory. From e37bb58fbb38fdf6bb69f2ee379138b2c4559598 Mon Sep 17 00:00:00 2001 From: Jake Klinker Date: Thu, 11 May 2023 21:32:48 +0000 Subject: [PATCH 5/5] Trim recipient addresses that are unreasonably long. This ensures that bad input does not affect the db - the fallback is a reasonable one where we just launch the new conversation screen and have the user select the recipient. TESTED=manually confirmed that I could no longer repro b/278556945 after this change. BUG=278556945 Change-Id: I705a304a92cb46b20d916c6f5c2db81e6fa84f06 --- .../LaunchConversationActivity.java | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/com/android/messaging/ui/conversation/LaunchConversationActivity.java b/src/com/android/messaging/ui/conversation/LaunchConversationActivity.java index 5500ae8..c869839 100644 --- a/src/com/android/messaging/ui/conversation/LaunchConversationActivity.java +++ b/src/com/android/messaging/ui/conversation/LaunchConversationActivity.java @@ -37,6 +37,8 @@ import com.android.messaging.util.UriUtil; import java.io.UnsupportedEncodingException; import java.net.URLDecoder; +import java.util.ArrayList; +import java.util.List; /** * Launches ConversationActivity for sending a message to, or viewing messages from, a specific @@ -46,6 +48,7 @@ import java.net.URLDecoder; */ public class LaunchConversationActivity extends Activity implements LaunchConversationData.LaunchConversationDataListener { + private static final int MAX_RECIPIENT_LENGTH = 100; static final String SMS_BODY = "sms_body"; static final String ADDRESS = "address"; final Binding mBinding = BindingBase.createBinding(this); @@ -76,6 +79,9 @@ public class LaunchConversationActivity extends Activity implements recipients = new String[] { intent.getStringExtra(Intent.EXTRA_EMAIL) }; } } + if (recipients != null) { + recipients = trimInvalidRecipients(recipients); + } mSmsBody = intent.getStringExtra(SMS_BODY); if (TextUtils.isEmpty(mSmsBody)) { // Used by intents sent from the web YouTube (and perhaps others). @@ -103,6 +109,20 @@ public class LaunchConversationActivity extends Activity implements finish(); } + private String[] trimInvalidRecipients(String[] recipients) { + List trimmedRecipients = new ArrayList<>(); + for (String recipient : recipients) { + if (recipient.length() < MAX_RECIPIENT_LENGTH) { + trimmedRecipients.add(recipient); + } + } + if (trimmedRecipients.size() > 0) { + return trimmedRecipients.toArray(new String[0]); + } else { + return null; + } + } + private String getBody(final Uri uri) { if (uri == null) { return null;