From 59d03cbeb2e3f6cfb4bf3ef71721e57c9e5dbcb7 Mon Sep 17 00:00:00 2001
From: Greyson Parrelli <greyson@signal.org>
Date: Fri, 18 Oct 2019 11:07:49 -0400
Subject: [PATCH] Address possible issues with AvatarMigrationJob.

---
 .../migrations/AvatarMigrationJob.java        | 26 +++++++++++++++----
 .../securesms/migrations/MigrationJob.java    |  2 +-
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/src/org/thoughtcrime/securesms/migrations/AvatarMigrationJob.java b/src/org/thoughtcrime/securesms/migrations/AvatarMigrationJob.java
index 0e5355187f..20088c8ebd 100644
--- a/src/org/thoughtcrime/securesms/migrations/AvatarMigrationJob.java
+++ b/src/org/thoughtcrime/securesms/migrations/AvatarMigrationJob.java
@@ -6,13 +6,16 @@ import org.thoughtcrime.securesms.database.DatabaseFactory;
 import org.thoughtcrime.securesms.jobmanager.Data;
 import org.thoughtcrime.securesms.jobmanager.Job;
 import org.thoughtcrime.securesms.logging.Log;
+import org.thoughtcrime.securesms.phonenumbers.NumberUtil;
 import org.thoughtcrime.securesms.profiles.AvatarHelper;
 import org.thoughtcrime.securesms.recipients.Recipient;
+import org.thoughtcrime.securesms.util.GroupUtil;
 import org.thoughtcrime.securesms.util.Util;
 
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
+import java.util.regex.Pattern;
 
 /**
  * Previously, we used a recipient's address as the filename for their avatar. We want to use
@@ -24,6 +27,8 @@ public class AvatarMigrationJob extends MigrationJob {
 
   private static final String TAG = Log.tag(AvatarMigrationJob.class);
 
+  private static final Pattern NUMBER_PATTERN = Pattern.compile("^[0-9\\-+]+$");
+
   AvatarMigrationJob() {
     this(new Parameters.Builder().build());
   }
@@ -47,22 +52,29 @@ public class AvatarMigrationJob extends MigrationJob {
     File   oldDirectory = new File(context.getFilesDir(), "avatars");
     File[] files        = oldDirectory.listFiles();
 
+    if (files == null) {
+      Log.w(TAG, "Unable to read directory, and therefore unable to migrate any avatars.");
+      return;
+    }
+
     Log.i(TAG, "Preparing to move " + files.length + " avatars.");
 
     for (File file : files) {
       try {
-        Recipient recipient = Recipient.external(context, file.getName());
-        byte[]    data      = Util.readFully(new FileInputStream(file));
+        if (isValidFileName(file.getName())) {
+          Recipient recipient = Recipient.external(context, file.getName());
+          byte[]    data      = Util.readFully(new FileInputStream(file));
 
-        AvatarHelper.setAvatar(context, recipient.getId(), data);
+          AvatarHelper.setAvatar(context, recipient.getId(), data);
+        } else {
+          Log.w(TAG, "Invalid file name! Can't migrate this file. It'll just get deleted.");
+        }
       } catch (IOException e) {
         Log.w(TAG, "Failed to copy avatar file. Skipping it.", e);
       } finally {
         file.delete();
       }
     }
-
-    oldDirectory.delete();
   }
 
   @Override
@@ -70,6 +82,10 @@ public class AvatarMigrationJob extends MigrationJob {
     return false;
   }
 
+  private static boolean isValidFileName(@NonNull String name) {
+    return NUMBER_PATTERN.matcher(name).matches() || GroupUtil.isEncodedGroup(name) || NumberUtil.isValidEmail(name);
+  }
+
   public static class Factory implements Job.Factory<AvatarMigrationJob> {
     @Override
     public @NonNull AvatarMigrationJob create(@NonNull Parameters parameters, @NonNull Data data) {
diff --git a/src/org/thoughtcrime/securesms/migrations/MigrationJob.java b/src/org/thoughtcrime/securesms/migrations/MigrationJob.java
index f2eea63b8a..35ba128eba 100644
--- a/src/org/thoughtcrime/securesms/migrations/MigrationJob.java
+++ b/src/org/thoughtcrime/securesms/migrations/MigrationJob.java
@@ -41,7 +41,7 @@ abstract class MigrationJob extends Job {
       return Result.success();
     } catch (RuntimeException e) {
       Log.w(TAG, JobLogger.format(this, "Encountered a runtime exception."), e);
-      throw e;
+      throw new FailedMigrationError(e);
     } catch (Exception e) {
       if (shouldRetry(e)) {
         Log.w(TAG, JobLogger.format(this, "Encountered a retryable exception."), e);