From 69532badee212e9223699482828664b083cc62c1 Mon Sep 17 00:00:00 2001 From: Vitor Pamplona Date: Mon, 31 Aug 2026 11:28:56 -0400 Subject: [PATCH] fix: narrow FileProvider external root to the app-specific dir MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `` rooted the provider at Environment.getExternalStorageDirectory() (/storage/emulated/0), which is far broader than anything Amethyst hands out. The only external-storage consumer is TakePicture's getPhotoUri/getVideoUri, both of which write into getExternalFilesDir(...) — so `` describes the actual surface exactly. Not a live vulnerability: the provider is exported="false", every getUriForFile() call site builds its File from app-controlled constants under cacheDir or getExternalFilesDir, and the one name derived from event content (shareIcs) is passed through IcsExport.safeFilename, which strips '/' — so no attacker-influenced path can reach the provider today. This is defence in depth plus an accurate declaration. Prefer external-files-path over hardcoding the path under Android/data//: the latter is wrong for the .debug and .benchmark applicationIdSuffixes, while external-files-path resolves per variant. The `external_files` name is kept so the generated content:// URI shape does not change. FileProviderPathsTest pins both halves on device: the capture URIs still resolve under /external_files/, cacheDir still resolves under /cache/, and a file at the external-storage root no longer maps. Against the old config that last case fails with content://com.vitorpamplona.amethyst.debug.provider/external_files/Download/not-ours.pdf. Supersedes nostr proposal a5d172d8. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01GYVqQqpUY1xqYG5LUY6jQr --- .../actions/uploads/FileProviderPathsTest.kt | 84 +++++++++++++++++++ amethyst/src/main/res/xml/file_paths.xml | 6 +- 2 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 amethyst/src/androidTest/java/com/vitorpamplona/amethyst/ui/actions/uploads/FileProviderPathsTest.kt diff --git a/amethyst/src/androidTest/java/com/vitorpamplona/amethyst/ui/actions/uploads/FileProviderPathsTest.kt b/amethyst/src/androidTest/java/com/vitorpamplona/amethyst/ui/actions/uploads/FileProviderPathsTest.kt new file mode 100644 index 0000000000..ae096595f1 --- /dev/null +++ b/amethyst/src/androidTest/java/com/vitorpamplona/amethyst/ui/actions/uploads/FileProviderPathsTest.kt @@ -0,0 +1,84 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.ui.actions.uploads + +import android.os.Environment +import androidx.core.content.FileProvider +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.platform.app.InstrumentationRegistry +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Assert.fail +import org.junit.Test +import org.junit.runner.RunWith +import java.io.File + +/** + * Pins what `res/xml/file_paths.xml` is allowed to hand out. + * + * The provider root used to be ``, i.e. the whole of + * `Environment.getExternalStorageDirectory()`. It is now the app-specific + * ``, which is the only external location Amethyst ever + * shares from (camera/video capture). These tests fail if either half of that + * regresses: the capture paths must still resolve, and the external-storage + * root must not. + */ +@RunWith(AndroidJUnit4::class) +class FileProviderPathsTest { + private val context = InstrumentationRegistry.getInstrumentation().targetContext + private val authority = "${context.packageName}.provider" + + @Test + fun photoCaptureUriResolves() { + val uri = getPhotoUri(context) + assertEquals("content", uri.scheme) + assertEquals(authority, uri.authority) + assertTrue("expected the external_files root, got $uri", uri.path!!.startsWith("/external_files/")) + } + + @Test + fun videoCaptureUriResolves() { + val uri = getVideoUri(context) + assertEquals("content", uri.scheme) + assertEquals(authority, uri.authority) + assertTrue("expected the external_files root, got $uri", uri.path!!.startsWith("/external_files/")) + } + + @Test + fun cacheDirStillResolves() { + val file = File(context.cacheDir, "amethyst_share_probe.png") + val uri = FileProvider.getUriForFile(context, authority, file) + assertEquals(authority, uri.authority) + assertTrue("expected the cache root, got $uri", uri.path!!.startsWith("/cache/")) + } + + @Test + fun externalStorageRootIsNoLongerShareable() { + @Suppress("DEPRECATION") + val outside = File(Environment.getExternalStorageDirectory(), "Download/not-ours.pdf") + try { + val uri = FileProvider.getUriForFile(context, authority, outside) + fail("FileProvider should not map $outside, but produced $uri") + } catch (expected: IllegalArgumentException) { + // Correct: no configured root contains it. + } + } +} diff --git a/amethyst/src/main/res/xml/file_paths.xml b/amethyst/src/main/res/xml/file_paths.xml index 0b339a9cf9..bac4fa1a57 100644 --- a/amethyst/src/main/res/xml/file_paths.xml +++ b/amethyst/src/main/res/xml/file_paths.xml @@ -1,6 +1,10 @@ - +