check file owner when attaching files or using them as avatar

Daniel Gultsch created 9 years ago

Change summary

src/main/java/eu/siacs/conversations/persistance/FileBackend.java          | 33 
src/main/java/eu/siacs/conversations/services/XmppConnectionService.java   | 10 
src/main/java/eu/siacs/conversations/ui/PublishProfilePictureActivity.java | 13 
src/main/res/values/strings.xml                                            |  1 
4 files changed, 55 insertions(+), 2 deletions(-)

Detailed changes

src/main/java/eu/siacs/conversations/persistance/FileBackend.java

@@ -1,5 +1,7 @@
 package eu.siacs.conversations.persistance;
 
+import android.annotation.TargetApi;
+import android.content.ContentResolver;
 import android.content.Context;
 import android.content.Intent;
 import android.database.Cursor;
@@ -9,8 +11,13 @@ import android.graphics.Canvas;
 import android.graphics.Matrix;
 import android.graphics.RectF;
 import android.net.Uri;
+import android.os.Build;
 import android.os.Environment;
+import android.os.ParcelFileDescriptor;
 import android.provider.OpenableColumns;
+import android.system.ErrnoException;
+import android.system.Os;
+import android.system.StructStat;
 import android.util.Base64;
 import android.util.Base64OutputStream;
 import android.util.Log;
@@ -19,6 +26,7 @@ import android.webkit.MimeTypeMap;
 import java.io.ByteArrayOutputStream;
 import java.io.Closeable;
 import java.io.File;
+import java.io.FileDescriptor;
 import java.io.FileNotFoundException;
 import java.io.FileOutputStream;
 import java.io.IOException;
@@ -646,4 +654,29 @@ public class FileBackend {
 			}
 		}
 	}
+
+
+	public static boolean weOwnFile(Uri uri) {
+		if (Build.VERSION.SDK_INT < Build.VERSION_CODES.LOLLIPOP) {
+			return false;
+		} else {
+			return uri != null
+					&& ContentResolver.SCHEME_FILE.equals(uri.getScheme())
+					&& weOwnFileLollipop(uri);
+		}
+	}
+
+	@TargetApi(Build.VERSION_CODES.LOLLIPOP)
+	private static boolean weOwnFileLollipop(Uri uri) {
+		try {
+			File file = new File(uri.getPath());
+			FileDescriptor fd = ParcelFileDescriptor.open(file, ParcelFileDescriptor.MODE_READ_ONLY).getFileDescriptor();
+			StructStat st = Os.fstat(fd);
+			return st.st_uid == android.os.Process.myUid();
+		} catch (ErrnoException e) {
+			return true;
+		} catch (FileNotFoundException e) {
+			return false;
+		}
+	}
 }

src/main/java/eu/siacs/conversations/services/XmppConnectionService.java

@@ -403,6 +403,11 @@ public class XmppConnectionService extends Service implements OnPhoneContactsLoa
 	public void attachFileToConversation(final Conversation conversation,
 										 final Uri uri,
 										 final UiCallback<Message> callback) {
+		if (FileBackend.weOwnFile(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);
@@ -441,6 +446,11 @@ public class XmppConnectionService extends Service implements OnPhoneContactsLoa
 	}
 
 	public void attachImageToConversation(final Conversation conversation, final Uri uri, final UiCallback<Message> callback) {
+		if (FileBackend.weOwnFile(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 compressPictures = getCompressPicturesPreference();
 		if ("never".equals(compressPictures)
 				|| ("auto".equals(compressPictures) && getFileBackend().useImageAsIs(uri))) {

src/main/java/eu/siacs/conversations/ui/PublishProfilePictureActivity.java

@@ -23,6 +23,7 @@ import java.io.File;
 import eu.siacs.conversations.Config;
 import eu.siacs.conversations.R;
 import eu.siacs.conversations.entities.Account;
+import eu.siacs.conversations.persistance.FileBackend;
 import eu.siacs.conversations.utils.FileUtils;
 import eu.siacs.conversations.utils.PhoneHelper;
 import eu.siacs.conversations.xmpp.pep.Avatar;
@@ -187,9 +188,13 @@ public class PublishProfilePictureActivity extends XmppActivity {
 	protected void onActivityResult(int requestCode, int resultCode, final Intent data) {
 		super.onActivityResult(requestCode, resultCode, data);
 		if (resultCode == RESULT_OK) {
+			Uri source = data.getData();
 			switch (requestCode) {
 				case REQUEST_CHOOSE_FILE_AND_CROP:
-					Uri source = data.getData();
+					if (FileBackend.weOwnFile(source)) {
+						Toast.makeText(this,R.string.security_error_invalid_file_access,Toast.LENGTH_SHORT).show();
+						return;
+					}
 					String original = FileUtils.getPath(this, source);
 					if (original != null) {
 						source = Uri.parse("file://"+original);
@@ -199,7 +204,11 @@ public class PublishProfilePictureActivity extends XmppActivity {
 					Crop.of(source, destination).asSquare().withMaxSize(size, size).start(this);
 					break;
 				case REQUEST_CHOOSE_FILE:
-					this.avatarUri = data.getData();
+					if (FileBackend.weOwnFile(source)) {
+						Toast.makeText(this,R.string.security_error_invalid_file_access,Toast.LENGTH_SHORT).show();
+						return;
+					}
+					this.avatarUri = source;
 					if (xmppConnectionServiceBound) {
 						loadImageIntoPreview(this.avatarUri);
 					}

src/main/res/values/strings.xml

@@ -609,4 +609,5 @@
 	<string name="no_keys_just_confirm">You already trust this contact. By selecting \'done\' you are just confirming that %s is part of this conference.</string>
 	<string name="select_image_and_crop">Select image and crop</string>
 	<string name="this_account_is_disabled">You have disabled this account</string>
+	<string name="security_error_invalid_file_access">Security error: Invalid file access</string>
 </resources>