From 86b733e159831cc26f0b8fd561eff011305a0edf Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Fri, 1 Mar 2024 14:39:54 +0100 Subject: [PATCH] prevent receiving (as share with target) file URIs as Element (Matrix client) demonstrated again file URIs are unnecessarily dangerous. On Android 7+ there is no good reason to process them anymore --- .../persistance/FileBackend.java | 38 +++++++++++-------- .../ui/ConversationFragment.java | 4 +- 2 files changed, 24 insertions(+), 18 deletions(-) diff --git a/src/main/java/eu/siacs/conversations/persistance/FileBackend.java b/src/main/java/eu/siacs/conversations/persistance/FileBackend.java index d8050646a..eea7486ab 100644 --- a/src/main/java/eu/siacs/conversations/persistance/FileBackend.java +++ b/src/main/java/eu/siacs/conversations/persistance/FileBackend.java @@ -370,29 +370,35 @@ public class FileBackend { } } - public static boolean weOwnFile(final Uri uri) { - if (uri == null || !ContentResolver.SCHEME_FILE.equals(uri.getScheme())) { - return false; - } else { - return weOwnFileLollipop(uri); + public static boolean dangerousFile(final Uri uri) { + if (uri == null || Strings.isNullOrEmpty(uri.getScheme())) { + return true; } + if (ContentResolver.SCHEME_FILE.equals(uri.getScheme())) { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { + // On Android 7 (and apps that target 7) it is now longer possible to share files + // with a file scheme. By now you should probably not be running apps that target + // anything less than 7 any more + return true; + } else { + return isFileOwnedByProcess(uri); + } + } + return false; } - private static boolean weOwnFileLollipop(final Uri uri) { + private static boolean isFileOwnedByProcess(final Uri uri) { final String path = uri.getPath(); if (path == null) { - return false; + return true; } - try { - File file = new File(path); - FileDescriptor fd = - ParcelFileDescriptor.open(file, ParcelFileDescriptor.MODE_READ_ONLY) - .getFileDescriptor(); - StructStat st = Os.fstat(fd); + try (final var pfd = + ParcelFileDescriptor.open(new File(path), ParcelFileDescriptor.MODE_READ_ONLY)) { + final FileDescriptor fd = pfd.getFileDescriptor(); + final StructStat st = Os.fstat(fd); return st.st_uid == android.os.Process.myUid(); - } catch (FileNotFoundException e) { - return false; - } catch (Exception e) { + } catch (final Exception e) { + // when in doubt. better safe than sorry return true; } } diff --git a/src/main/java/eu/siacs/conversations/ui/ConversationFragment.java b/src/main/java/eu/siacs/conversations/ui/ConversationFragment.java index da281617e..c97aea645 100644 --- a/src/main/java/eu/siacs/conversations/ui/ConversationFragment.java +++ b/src/main/java/eu/siacs/conversations/ui/ConversationFragment.java @@ -2619,10 +2619,10 @@ public class ConversationFragment extends XmppFragment final Iterator iterator = uris.iterator(); while (iterator.hasNext()) { final Uri uri = iterator.next(); - if (FileBackend.weOwnFile(uri)) { + if (FileBackend.dangerousFile(uri)) { iterator.remove(); Toast.makeText( - getActivity(), + requireActivity(), R.string.security_violation_not_attaching_file, Toast.LENGTH_SHORT) .show();