diff --git a/app/src/main/java/com/infomaniak/mail/data/api/ApiRepository.kt b/app/src/main/java/com/infomaniak/mail/data/api/ApiRepository.kt index 3513d856996..85a5f76561f 100644 --- a/app/src/main/java/com/infomaniak/mail/data/api/ApiRepository.kt +++ b/app/src/main/java/com/infomaniak/mail/data/api/ApiRepository.kt @@ -83,6 +83,7 @@ import com.infomaniak.mail.ui.newMessage.AiViewModel.Shortcut import com.infomaniak.mail.utils.AccountUtils import com.infomaniak.mail.utils.Utils import com.infomaniak.mail.utils.Utils.EML_CONTENT_TYPE +import com.infomaniak.mail.utils.toSafeFileName import io.realm.kotlin.ext.copyFromRealm import kotlinx.serialization.json.Json import okhttp3.MediaType.Companion.toMediaType @@ -613,7 +614,7 @@ object ApiRepository : ApiRepositoryCore() { @OptIn(ManualAuthorizationRequired::class) val headers = HttpUtils.getHeaders(contentType = null).newBuilder() .set("Authorization", "Bearer $userApiToken") - .addUnsafeNonAscii("x-ws-attachment-filename", attachment.name) + .addUnsafeNonAscii("x-ws-attachment-filename", attachment.name.toSafeFileName()) .add("x-ws-attachment-mime-type", attachment.mimeType) .add("x-ws-attachment-disposition", "attachment") .build() diff --git a/app/src/main/java/com/infomaniak/mail/data/models/extensions/AttachableExtensions.kt b/app/src/main/java/com/infomaniak/mail/data/models/extensions/AttachableExtensions.kt index 600cf370a0b..ae74c961006 100644 --- a/app/src/main/java/com/infomaniak/mail/data/models/extensions/AttachableExtensions.kt +++ b/app/src/main/java/com/infomaniak/mail/data/models/extensions/AttachableExtensions.kt @@ -29,6 +29,7 @@ import com.infomaniak.mail.utils.AccountUtils import com.infomaniak.mail.utils.AttachableMimeTypeUtils import com.infomaniak.mail.utils.LocalStorageUtils import com.infomaniak.mail.utils.Utils +import com.infomaniak.mail.utils.resolveContainedFileName import java.io.File val Attachable.downloadUrl get() = ApiRoutes.resource(resource!!) @@ -45,13 +46,13 @@ fun Attachable.hasUsableCache( ): Boolean = when (this) { is Attachment -> { val cachedFile = file ?: getCacheFile(context, userId, mailboxId) - cachedFile.length() > 0 && cachedFile.canRead() + cachedFile != null && cachedFile.length() > 0 && cachedFile.canRead() } is SwissTransferFile -> false } fun Attachable.isInlineCachedFile(context: Context): Boolean = when (this) { - is Attachment -> getCacheFile(context).exists() && disposition == AttachmentDisposition.INLINE + is Attachment -> getCacheFile(context)?.exists() == true && disposition == AttachmentDisposition.INLINE is SwissTransferFile -> false } @@ -59,10 +60,10 @@ fun Attachable.getCacheFile( context: Context, userId: Int = AccountUtils.currentUserId, mailboxId: Int = AccountUtils.currentMailboxId, -): File = when (this) { +): File? = when (this) { is Attachment -> { val cacheFolder = LocalStorageUtils.getAttachmentsCacheDir(context, extractPathFromResource(), userId, mailboxId) - File(cacheFolder, name) + cacheFolder?.resolveContainedFileName(untrustedName = name) } is SwissTransferFile -> File("") } diff --git a/app/src/main/java/com/infomaniak/mail/ui/main/thread/webViewClient/MessageWebViewClient.kt b/app/src/main/java/com/infomaniak/mail/ui/main/thread/webViewClient/MessageWebViewClient.kt index 596358787dd..0cd9b7a5a02 100644 --- a/app/src/main/java/com/infomaniak/mail/ui/main/thread/webViewClient/MessageWebViewClient.kt +++ b/app/src/main/java/com/infomaniak/mail/ui/main/thread/webViewClient/MessageWebViewClient.kt @@ -48,7 +48,7 @@ abstract class MessageWebViewClient( if (url?.scheme.equals(CID_SCHEME, ignoreCase = true)) { val cid = url.schemeSpecificPart return cidDictionary[cid]?.let { attachment -> - val cacheFile = attachment.getCacheFile(context) + val cacheFile = attachment.getCacheFile(context) ?: return@runCatchingRealm null val data = if (attachment.hasUsableCache(context, cacheFile)) { cacheFile.inputStream() diff --git a/app/src/main/java/com/infomaniak/mail/utils/FileNameUtils.kt b/app/src/main/java/com/infomaniak/mail/utils/FileNameUtils.kt new file mode 100644 index 00000000000..45fcc33e892 --- /dev/null +++ b/app/src/main/java/com/infomaniak/mail/utils/FileNameUtils.kt @@ -0,0 +1,105 @@ +/* + * Infomaniak Mail - Android + * Copyright (C) 2026 Infomaniak Network SA + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package com.infomaniak.mail.utils + +import io.sentry.Sentry +import java.io.File +import java.nio.charset.StandardCharsets +import java.text.Normalizer + +/** Android’s filesystems limit each path component to 255 UTF-8 bytes. + * 240 leaves 15 bytes for prefixes/suffixes. + * The upload prefix (an Int hash code plus an underscore) needs at most 12 bytes, yielding at most 252 bytes. + * The remaining 3 bytes are conservative headroom. */ +private const val MAX_FILE_NAME_SIZE_BYTES = 240 +private const val MAX_PRESERVED_EXTENSION_SIZE_BYTES = 32 +private const val DEFAULT_FILE_NAME = "attachment" +private val invalidFileNameCharacters = Regex("[\\\\/:*?\"<>|%\\p{Cc}]") + +fun String.toSafeFileName(): String { + val normalizedName = Normalizer.normalize(this, Normalizer.Form.NFKC) + .replace(invalidFileNameCharacters, "_") + .trim() + .takeUnless { it.isEmpty() || it == "." || it == ".." } + ?: DEFAULT_FILE_NAME + + return if (normalizedName.utf8Size <= MAX_FILE_NAME_SIZE_BYTES) normalizedName else computeTruncatedFileName(normalizedName) +} + +fun File.resolveContainedPath(untrustedPath: String): File? = runCatching { + if (File(untrustedPath).isAbsolute) throw SecurityException("Absolute paths are not allowed") + + val canonicalRoot = canonicalFile + val resolvedFile = File(canonicalRoot, untrustedPath).canonicalFile + if (resolvedFile != canonicalRoot && !resolvedFile.toPath().startsWith(canonicalRoot.toPath())) { + throw SecurityException("Resolved path escapes its allowed root") + } + + return@runCatching resolvedFile +}.getOrElse { + Sentry.captureException(it) + null +} + +fun File.resolveContainedFileName(untrustedName: String): File? { + val canonicalRoot = canonicalFile + val resolvedFile = File(canonicalRoot, untrustedName.toSafeFileName()).canonicalFile + return if (resolvedFile.parentFile != canonicalRoot) { + Sentry.captureException(SecurityException("Resolved file escapes its allowed directory")) + null + } else { + resolvedFile + } +} + +private fun computeTruncatedFileName(normalizedName: String): String { + val extensionStart = normalizedName.lastIndexOf('.').takeIf { it in 1 until normalizedName.lastIndex } + val extension = extensionStart?.let(normalizedName::substring) + ?.takeIf { it.utf8Size <= MAX_PRESERVED_EXTENSION_SIZE_BYTES } + .orEmpty() + val fileNameWithoutExtension = normalizedName.dropLast(extension.length) + val truncatedFileName = fileNameWithoutExtension.takeUtf8Bytes(MAX_FILE_NAME_SIZE_BYTES - extension.utf8Size) + + return truncatedFileName.ifEmpty { DEFAULT_FILE_NAME } + extension +} + +private inline val String.utf8Size: Int get() = toByteArray(StandardCharsets.UTF_8).size + +/** + * Compute with utf8 size because Kotlin manipulate utf16 entities + * This avoids character needing 2 bytes like 'é' to count only for 1 in a take(MAX_FILE_NAME_SIZE_BYTES) instead of 2. + */ +private fun String.takeUtf8Bytes(maxBytes: Int): String { + if (maxBytes <= 0) return "" + if (utf8Size <= maxBytes) return this + + val result = StringBuilder() + var byteCount = 0 + var index = 0 + while (index < length) { + val codePoint = codePointAt(index) + val character = String(Character.toChars(codePoint)) + val characterSize = character.utf8Size + if (byteCount + characterSize > maxBytes) break + + result.append(character) + byteCount += characterSize + index += Character.charCount(codePoint) + } + return result.toString() +} diff --git a/app/src/main/java/com/infomaniak/mail/utils/LocalStorageUtils.kt b/app/src/main/java/com/infomaniak/mail/utils/LocalStorageUtils.kt index 5615ee0e472..819935e0d52 100644 --- a/app/src/main/java/com/infomaniak/mail/utils/LocalStorageUtils.kt +++ b/app/src/main/java/com/infomaniak/mail/utils/LocalStorageUtils.kt @@ -55,8 +55,9 @@ object LocalStorageUtils { attachmentPath: String, userId: Int = AccountUtils.currentUserId, mailboxId: Int = AccountUtils.currentMailboxId, - ): File { - return File(generateRootDir(context.attachmentsCacheRootDir, userId, mailboxId), attachmentPath) + ): File? { + val cacheRoot = generateRootDir(context.attachmentsCacheRootDir, userId, mailboxId) + return cacheRoot.resolveContainedPath(attachmentPath) } suspend fun downloadThenSaveAttachmentToCacheDir(context: Context, localAttachment: Attachment): Boolean { @@ -75,7 +76,10 @@ object LocalStorageUtils { val attachment = runCatching { localAttachment.resource?.let { ApiRepository.downloadAttachment(it) } }.cancellable().getOrNull() - return attachment?.saveAttachmentTo(localAttachment.getCacheFile(context)) == true + + val cacheFile = localAttachment.getCacheFile(context) ?: return false + + return attachment?.saveAttachmentTo(cacheFile) == true } /** @@ -128,7 +132,7 @@ object LocalStorageUtils { return context.contentResolver.openInputStream(uri)?.use { inputStream -> val attachmentsUploadDir = getAttachmentUploadDir(context, draftLocalUuid, attachmentLocalUuid) attachmentsUploadDir.mkdirs() - val hashedFileName = "${uri.toString().substringAfter("document/").hashCode()}_$fileName" + val hashedFileName = "${uri.toString().substringAfter("document/").hashCode()}_${fileName.toSafeFileName()}" return@use getFileToUpload(context, uri, snackbarManager, attachmentsUploadDir, hashedFileName, inputStream) } ?: run { @@ -149,7 +153,7 @@ object LocalStorageUtils { hashedFileName: String, inputStream: InputStream, ): File? { - val file = File(attachmentsUploadDir, hashedFileName) + val file = attachmentsUploadDir.resolveContainedFileName(hashedFileName) ?: return null val isSuccess = runCatching { FileOutputStream(file).use(inputStream::copyTo) true diff --git a/app/src/main/java/com/infomaniak/mail/utils/extensions/AttachmentExt.kt b/app/src/main/java/com/infomaniak/mail/utils/extensions/AttachmentExt.kt index d777158bafe..911f50a7dee 100644 --- a/app/src/main/java/com/infomaniak/mail/utils/extensions/AttachmentExt.kt +++ b/app/src/main/java/com/infomaniak/mail/utils/extensions/AttachmentExt.kt @@ -61,8 +61,8 @@ object AttachmentExt { const val DOWNLOAD_ATTACHMENT_RESULT = "download_attachment_result" //region Intent - private fun Attachment.saveToDriveIntent(context: Context): Intent { - val fileFromCache = getCacheFile(context) + private fun Attachment.saveToDriveIntent(context: Context): Intent? { + val fileFromCache = getCacheFile(context) ?: return null val lastModifiedDate = fileFromCache.lastModified() val uri = FileProvider.getUriForFile(context, context.getString(R.string.ATTACHMENTS_AUTHORITY), fileFromCache) @@ -75,8 +75,8 @@ object AttachmentExt { } } - private fun Attachment.openWithIntent(context: Context): Intent { - val file = getUploadLocalFile() ?: getCacheFile(context) + private fun Attachment.openWithIntent(context: Context): Intent? { + val file = getUploadLocalFile() ?: getCacheFile(context) ?: return null val uri = FileProvider.getUriForFile(context, context.getString(R.string.ATTACHMENTS_AUTHORITY), file) return Intent().apply { @@ -113,7 +113,7 @@ object AttachmentExt { navigateToDownloadProgressDialog: (Attachment, AttachmentIntentType) -> Unit, snackbarManager: SnackbarManager, ) { - if (openWithIntent(context).hasSupportedApplications(context)) { + if (openWithIntent(context)?.hasSupportedApplications(context) == true) { executeIntent(context, OPEN_WITH, navigateToDownloadProgressDialog) } else { snackbarManager.setValue(context.getString(RCore.string.errorNoSupportingAppFound)) diff --git a/app/src/test/java/com/infomaniak/mail/utils/FileNameUtilsTest.kt b/app/src/test/java/com/infomaniak/mail/utils/FileNameUtilsTest.kt new file mode 100644 index 00000000000..3032df614d1 --- /dev/null +++ b/app/src/test/java/com/infomaniak/mail/utils/FileNameUtilsTest.kt @@ -0,0 +1,101 @@ +/* + * Infomaniak Mail - Android + * Copyright (C) 2026 Infomaniak Network SA + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package com.infomaniak.mail.utils + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test +import java.io.File +import java.nio.charset.StandardCharsets +import java.nio.file.Files + +class FileNameUtilsTest { + + @Test + fun pathTraversalNames_areSanitized() { + assertEquals(".._secret.txt", "../secret.txt".toSafeFileName()) + assertEquals("....__secret.txt", "....//secret.txt".toSafeFileName()) + assertEquals(".._.._secret.txt", """..\..\secret.txt""".toSafeFileName()) + assertEquals("_data_data_secret.txt", "/data/data/secret.txt".toSafeFileName()) + } + + @Test + fun controlCharacters_areSanitized() { + val safeName = "invoice.pdf\r\nInjected: true".toSafeFileName() + + assertFalse(safeName.contains('\r')) + assertFalse(safeName.contains('\n')) + assertFalse(safeName.contains(':')) + } + + @Test + fun invalidEmptyNames_useFallback() { + assertEquals("attachment", "".toSafeFileName()) + assertEquals("attachment", " ".toSafeFileName()) + assertEquals("attachment", ".".toSafeFileName()) + assertEquals("attachment", "..".toSafeFileName()) + } + + @Test + fun unicodePathSeparators_areNormalizedBeforeSanitizing() { + assertEquals("_data_secret.txt", "\uFF0Fdata\uFF0Fsecret.txt".toSafeFileName()) + } + + @Test + fun longNames_areTruncatedAndKeepTheirExtension() { + val safeName = "${"é".repeat(200)}.pdf".toSafeFileName() + + assertTrue(safeName.toByteArray(StandardCharsets.UTF_8).size <= 240) + assertTrue(safeName.endsWith(".pdf")) + } + + @Test + fun validInternationalNames_areUnchanged() { + assertEquals("résumé été 2026.pdf", "résumé été 2026.pdf".toSafeFileName()) + } + + @Test + fun validResourcePath_keepsItsStructure() = withTemporaryDirectory { root -> + val relativePath = "folder/123/message/456/attachment/789" + + assertEquals(File(root, relativePath).canonicalFile, root.resolveContainedPath(relativePath)) + } + + @Test + fun traversingResourcePath_isRejected() = withTemporaryDirectory { root -> + assertEquals(null, root.resolveContainedPath("folder/123/../../../../outside")) + assertEquals(null, root.resolveContainedPath("/data/data/outside")) + } + + @Test + fun maliciousFileName_staysInsideExpectedDirectory() = withTemporaryDirectory { root -> + val file = root.resolveContainedFileName("....//../../secret.txt") + + assertEquals(root.canonicalFile, file?.parentFile) + } + + private fun withTemporaryDirectory(block: (File) -> Unit) { + val directory = Files.createTempDirectory("safe-file-name-test").toFile() + try { + block(directory) + } finally { + directory.deleteRecursively() + } + } +}