#3318 Improve validation of forwarded attachment file paths

FlowCrypt/src/main/java/com/flowcrypt/email/jetpack/workmanager/DownloadForwardedAttachmentsWorker.kt

@@ -34,6 +34,7 @@ import org.apache.commons.io.FileUtils
 import org.eclipse.angus.mail.imap.IMAPFolder
 import java.io.File
 import java.io.FileOutputStream
+import java.io.IOException
 import java.io.InputStream
 import java.util.UUID
 
@@ -254,9 +255,13 @@ class DownloadForwardedAttachmentsWorker(context: Context, params: WorkerParamet
         continue
       }
 
-      val attName = attachmentEntity.name
-
-      val attFile = File(parentDirectory, attName)
+      val attFile = try {
+        FileAndDirectoryUtils.getFileInsideDir(parentDirectory, attachmentEntity.name)
+      } catch (e: IOException) {
+        LogsUtil.d(TAG, "Rejected unsafe forwarded attachment name", e)
+        msgState = MessageState.ERROR_ORIGINAL_ATTACHMENT_NOT_FOUND
+        break
+      }
       val exists = attFile.exists()
 
       var uri: Uri?

FlowCrypt/src/main/java/com/flowcrypt/email/util/FileAndDirectoryUtils.kt

@@ -148,6 +148,31 @@ class FileAndDirectoryUtils {
       }
     }
 
+    /**
+     * Resolve a file name inside a directory and ensure the result stays strictly inside it.
+     *
+     * @param directory The directory that must contain the resolved file.
+     * @param fileName The untrusted file name. It must not contain path separators.
+     * @return The canonical file located inside [directory].
+     * @throws IOException If the file name is invalid or resolves outside [directory].
+     */
+    @Throws(IOException::class)
+    fun getFileInsideDir(directory: File, fileName: String): File {
+      if (fileName.indexOf(File.separatorChar) >= 0) {
+        throw IOException("File name contains a path separator: '$fileName'")
+      }
+
+      val canonicalDirectory = directory.canonicalFile
+      val file = File(canonicalDirectory, fileName).canonicalFile
+      val directoryPrefix = canonicalDirectory.path.trimEnd(File.separatorChar) + File.separator
+
+      if (file.path == canonicalDirectory.path || !file.path.startsWith(directoryPrefix)) {
+        throw IOException("Resolved file '$fileName' is outside of '$canonicalDirectory'")
+      }
+
+      return file
+    }
+
     /**
      * Get a directory. Create a new directory if it doesn't exist
      *

FlowCrypt/src/test/java/com/flowcrypt/email/util/GetFileInsideDirTest.kt

@@ -0,0 +1,144 @@
+/*
+ * © 2016-present FlowCrypt a.s. Limitations apply. Contact human@flowcrypt.com
+ */
+
+package com.flowcrypt.email.util
+
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertThrows
+import org.junit.Assume.assumeNoException
+import org.junit.Rule
+import org.junit.Test
+import org.junit.rules.TemporaryFolder
+import java.io.File
+import java.io.IOException
+import java.nio.file.Files
+
+class GetFileInsideDirTest {
+  @get:Rule
+  val temporaryFolder: TemporaryFolder = TemporaryFolder()
+
+  @Test
+  fun acceptFileDirectlyInsideDirectory() {
+    val directory = temporaryFolder.newFolder("forwarded")
+
+    val file = FileAndDirectoryUtils.getFileInsideDir(directory, "photo.jpg")
+
+    assertEquals(File(directory.canonicalFile, "photo.jpg").path, file.path)
+  }
+
+  @Test
+  fun rejectDatabaseJournalTraversal() {
+    val directory = temporaryFolder.newFolder("forwarded")
+
+    assertThrows(IOException::class.java) {
+      FileAndDirectoryUtils.getFileInsideDir(
+        directory,
+        "../../../../databases/flowcrypt.db-journal"
+      )
+    }
+  }
+
+  @Test
+  fun rejectNestedTraversal() {
+    val directory = temporaryFolder.newFolder("forwarded")
+
+    assertThrows(IOException::class.java) {
+      FileAndDirectoryUtils.getFileInsideDir(
+        directory,
+        "subdir/../../databases/flowcrypt.db-journal"
+      )
+    }
+  }
+
+  @Test
+  fun rejectAbsolutePathName() {
+    val directory = temporaryFolder.newFolder("forwarded")
+
+    assertThrows(IOException::class.java) {
+      FileAndDirectoryUtils.getFileInsideDir(
+        directory,
+        "/data/data/com.flowcrypt.email/databases/flowcrypt.db-journal"
+      )
+    }
+  }
+
+  @Test
+  fun rejectNestedPathName() {
+    val directory = temporaryFolder.newFolder("forwarded")
+
+    assertThrows(IOException::class.java) {
+      FileAndDirectoryUtils.getFileInsideDir(directory, "subdir/flowcrypt.db-journal")
+    }
+  }
+
+  @Test
+  fun rejectTrailingPathSeparator() {
+    val directory = temporaryFolder.newFolder("forwarded")
+
+    assertThrows(IOException::class.java) {
+      FileAndDirectoryUtils.getFileInsideDir(directory, "attachment/")
+    }
+  }
+
+  @Test
+  fun rejectParentDirectoryName() {
+    val directory = temporaryFolder.newFolder("forwarded")
+
+    assertThrows(IOException::class.java) {
+      FileAndDirectoryUtils.getFileInsideDir(directory, "..")
+    }
+  }
+
+  @Test
+  fun rejectCurrentDirectoryName() {
+    val directory = temporaryFolder.newFolder("forwarded")
+
+    assertThrows(IOException::class.java) {
+      FileAndDirectoryUtils.getFileInsideDir(directory, ".")
+    }
+  }
+
+  @Test
+  fun rejectEmptyName() {
+    val directory = temporaryFolder.newFolder("forwarded")
+
+    assertThrows(IOException::class.java) {
+      FileAndDirectoryUtils.getFileInsideDir(directory, "")
+    }
+  }
+
+  @Test
+  fun rejectSiblingSharingDirectoryNamePrefix() {
+    val root = temporaryFolder.newFolder("root")
+    val directory = File(root, "forwarded").apply { mkdirs() }
+
+    assertThrows(IOException::class.java) {
+      FileAndDirectoryUtils.getFileInsideDir(directory, "../forwarded-other/payload")
+    }
+  }
+
+  @Test
+  fun rejectSymlinkEscapingDirectory() {
+    val directory = temporaryFolder.newFolder("forwarded")
+    val outsideDirectory = temporaryFolder.newFolder("outside")
+    try {
+      Files.createSymbolicLink(File(directory, "link").toPath(), outsideDirectory.toPath())
+    } catch (e: Exception) {
+      assumeNoException(e)
+    }
+
+    assertThrows(IOException::class.java) {
+      FileAndDirectoryUtils.getFileInsideDir(directory, "link/flowcrypt.db-journal")
+    }
+  }
+
+  @Test
+  fun rejectInvalidFileName() {
+    val directory = temporaryFolder.newFolder("forwarded")
+
+    assertThrows(IOException::class.java) {
+      FileAndDirectoryUtils.getFileInsideDir(directory, "bad\u0000name")
+    }
+  }
+}