From 7cf9ad5fee4b6315ef1d53b4f072e30c6be5c55b Mon Sep 17 00:00:00 2001
From: Christian Schneppe <christian@pix-art.de>
Date: Thu, 4 Oct 2018 20:17:58 +0200
Subject: Do weOwnFile security check only when attaching
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The general security check is recommend so a third party can not ask us to send an internal file. But we don’t need to do this for files we attach ourself from within the app
---
 .../pixart/messenger/services/XmppConnectionService.java  | 12 +-----------
 .../java/de/pixart/messenger/ui/ConversationFragment.java | 15 ++++++++++++++-
 src/main/res/values/strings.xml                           |  1 +
 3 files changed, 16 insertions(+), 12 deletions(-)

(limited to 'src')

diff --git a/src/main/java/de/pixart/messenger/services/XmppConnectionService.java b/src/main/java/de/pixart/messenger/services/XmppConnectionService.java
index 93b9fc3aa..289823f79 100644
--- a/src/main/java/de/pixart/messenger/services/XmppConnectionService.java
+++ b/src/main/java/de/pixart/messenger/services/XmppConnectionService.java
@@ -491,11 +491,6 @@ public class XmppConnectionService extends Service {
     }
 
     public void attachFileToConversation(final Conversation conversation, final Uri uri, final String type, final UiCallback<Message> callback) {
-        if (FileBackend.weOwnFile(this, uri)) {
-            Log.d(Config.LOGTAG, "trying to attach file that belonged to us");
-            callback.error(R.string.security_error_invalid_file_access, null);
-            return;
-        }
         final Message message;
         if (conversation.getNextEncryption() == Message.ENCRYPTION_PGP) {
             message = new Message(conversation, "", Message.ENCRYPTION_DECRYPTED);
@@ -513,11 +508,6 @@ public class XmppConnectionService extends Service {
     }
 
     public void attachImageToConversation(final Conversation conversation, final Uri uri, final UiCallback<Message> callback) {
-        if (FileBackend.weOwnFile(this, uri)) {
-            Log.d(Config.LOGTAG, "trying to attach file that belonged to us");
-            callback.error(R.string.security_error_invalid_file_access, null);
-            return;
-        }
         final String mimeType = MimeUtils.guessMimeTypeFromUri(this, uri);
         final String compressPictures = getCompressPicturesPreference();
 
@@ -1161,7 +1151,7 @@ public class XmppConnectionService extends Service {
         }
 
         this.pm = (PowerManager) getSystemService(Context.POWER_SERVICE);
-        this.wakeLock = pm.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, "XmppConnectionService");
+        this.wakeLock = pm.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, Config.LOGTAG + ":Service");
         toggleForegroundService();
         updateUnreadCountBadge();
         toggleScreenEventReceiver();
diff --git a/src/main/java/de/pixart/messenger/ui/ConversationFragment.java b/src/main/java/de/pixart/messenger/ui/ConversationFragment.java
index 215c7d39c..eddf5abdc 100644
--- a/src/main/java/de/pixart/messenger/ui/ConversationFragment.java
+++ b/src/main/java/de/pixart/messenger/ui/ConversationFragment.java
@@ -2245,7 +2245,8 @@ public class ConversationFragment extends XmppFragment implements EditMessage.Ke
         final boolean pm = extras.getBoolean(ConversationsActivity.EXTRA_IS_PRIVATE_MESSAGE, false);
         final List<Uri> uris = extractUris(extras);
         if (uris != null && uris.size() > 0) {
-            mediaPreviewAdapter.addMediaPreviews(Attachment.of(getActivity(), uris));
+            final List<Uri> cleanedUris = cleanUris(new ArrayList<>(uris));
+            mediaPreviewAdapter.addMediaPreviews(Attachment.of(getActivity(), cleanedUris));
             toggleInputMethod();
             return;
         }
@@ -2290,6 +2291,18 @@ public class ConversationFragment extends XmppFragment implements EditMessage.Ke
         }
     }
 
+    private List<Uri> cleanUris(List<Uri> uris) {
+        Iterator<Uri> iterator = uris.iterator();
+        while (iterator.hasNext()) {
+            final Uri uri = iterator.next();
+            if (FileBackend.weOwnFile(getActivity(), uri)) {
+                iterator.remove();
+                Toast.makeText(getActivity(), R.string.security_violation_not_attaching_file, Toast.LENGTH_SHORT).show();
+            }
+        }
+        return uris;
+    }
+
     private boolean showBlockSubmenu(View view) {
         final Jid jid = conversation.getJid();
         if (jid.getLocal() == null) {
diff --git a/src/main/res/values/strings.xml b/src/main/res/values/strings.xml
index 75d3dfc14..5b7046752 100644
--- a/src/main/res/values/strings.xml
+++ b/src/main/res/values/strings.xml
@@ -829,4 +829,5 @@
     <string name="account_status_stream_opening_error">Stream opening error</string>
     <string name="action_open">Open</string>
     <string name="action_delete">Delete</string>
+    <string name="security_violation_not_attaching_file">File omitted due to security violation.</string>
 </resources>
-- 
cgit v1.2.3