From fda9140720483ecfb94c209be4c7fa1258dbca19 Mon Sep 17 00:00:00 2001 From: James Rich <2199651+jamesarich@users.noreply.github.com> Date: Tue, 29 Sep 2026 23:47:49 +0000 Subject: [PATCH] fix(app): check a shared map file before importing it (#7463) --- .skills/compose-ui/strings-index.txt | 2 + .../kotlin/org/meshtastic/app/MainActivity.kt | 42 ++- .../org/meshtastic/app/SharedMapFile.kt | 144 +++++++++ .../org/meshtastic/app/SharedMapFileTest.kt | 299 ++++++++++++++++++ .../composeResources/values/strings.xml | 2 + 5 files changed, 484 insertions(+), 5 deletions(-) create mode 100644 androidApp/src/main/kotlin/org/meshtastic/app/SharedMapFile.kt create mode 100644 androidApp/src/test/kotlin/org/meshtastic/app/SharedMapFileTest.kt diff --git a/.skills/compose-ui/strings-index.txt b/.skills/compose-ui/strings-index.txt index cbee61c8cb..fc4841260e 100644 --- a/.skills/compose-ui/strings-index.txt +++ b/.skills/compose-ui/strings-index.txt @@ -980,6 +980,8 @@ map_filter_show_ignored map_filter_title map_layer_formats map_layer_opacity +map_layer_open_failed +map_layer_too_large map_node_popup_details map_offline_banner map_offline_download_failed_build diff --git a/androidApp/src/main/kotlin/org/meshtastic/app/MainActivity.kt b/androidApp/src/main/kotlin/org/meshtastic/app/MainActivity.kt index 48ed731076..e0e150f60c 100644 --- a/androidApp/src/main/kotlin/org/meshtastic/app/MainActivity.kt +++ b/androidApp/src/main/kotlin/org/meshtastic/app/MainActivity.kt @@ -49,6 +49,7 @@ import androidx.lifecycle.lifecycleScope import co.touchlab.kermit.Logger import com.eygraber.uri.toKmpUri import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext import org.koin.android.ext.android.inject import org.koin.androidx.viewmodel.ext.android.viewModel import org.koin.compose.koinInject @@ -60,6 +61,7 @@ import org.meshtastic.app.node.metrics.getTracerouteMapOverlayInsets import org.meshtastic.app.ui.MainScreen import org.meshtastic.core.barcode.rememberBarcodeScanner import org.meshtastic.core.common.state.LaunchOptions +import org.meshtastic.core.di.CoroutineDispatchers import org.meshtastic.core.model.DeviceAddress import org.meshtastic.core.navigation.DEEP_LINK_BASE_URI import org.meshtastic.core.network.repository.UsbRepository @@ -68,6 +70,9 @@ import org.meshtastic.core.nfc.NfcScannerEffect import org.meshtastic.core.nfc.NfcWriterEffect import org.meshtastic.core.resources.Res import org.meshtastic.core.resources.channel_invalid +import org.meshtastic.core.resources.map_layer_formats +import org.meshtastic.core.resources.map_layer_open_failed +import org.meshtastic.core.resources.map_layer_too_large import org.meshtastic.core.service.MeshService import org.meshtastic.core.service.ServiceStartTrigger import org.meshtastic.core.service.startService @@ -105,7 +110,7 @@ import org.meshtastic.feature.intro.IntroViewModel import org.meshtastic.feature.map.MapScreen import org.meshtastic.feature.map.SharedMapViewModel import org.meshtastic.feature.map.layers.MapLayersManager -import org.meshtastic.feature.map.layers.toPickedMapFile +import org.meshtastic.feature.map.layers.PickedMapFile class MainActivity : AppCompatActivity() { private val model: UIViewModel by viewModel() @@ -113,6 +118,7 @@ class MainActivity : AppCompatActivity() { private val usbRepository: UsbRepository by inject() private val mapLayersManager: MapLayersManager by inject() private val launchOptions: LaunchOptions by inject() + private val dispatchers: CoroutineDispatchers by inject() override fun onCreate(savedInstanceState: Bundle?) { installSplashScreen() @@ -372,13 +378,39 @@ class MainActivity : AppCompatActivity() { * * Handed to the layer store directly rather than through a one-slot bus for the map to drain. The store is common * code now, so both flavours import a shared file; the bus only ever reached the Google map, which meant the same - * share silently did nothing on F-Droid. The read grant on [uri] lives as long as this activity, which is long - * enough for the store to copy the file in. + * share silently did nothing on F-Droid. The file is read here, in this activity's scope, because the read grant on + * [uri] lives only as long as the activity; the Map tab opens once the read succeeds. */ private fun importMapFile(uri: Uri) { Logger.d { "Importing shared map file: $uri" } - mapLayersManager.addMapLayer(uri.toPickedMapFile(this)) - handleMeshtasticUri("$DEEP_LINK_BASE_URI/map".toUri()) + lifecycleScope.launch { + when (val load = withContext(dispatchers.io) { contentResolver.loadSharedMapFile(uri) }) { + is SharedMapFileLoad.Refused -> { + Logger.w { "Refusing shared map file: ${load.reason}" } + when (load.reason) { + SharedMapFileRejection.UNSUPPORTED_TYPE -> showToast(Res.string.map_layer_formats) + + SharedMapFileRejection.TOO_LARGE -> + showToast(Res.string.map_layer_too_large, MAX_SHARED_MAP_FILE_MB) + + SharedMapFileRejection.NOT_CONTENT_URI, + SharedMapFileRejection.UNREADABLE, + -> showToast(Res.string.map_layer_open_failed) + } + } + + is SharedMapFileLoad.Loaded -> { + mapLayersManager.addMapLayer( + PickedMapFile( + displayName = load.file.displayName, + extensionOrMime = load.file.extensionOrMime, + read = { load.bytes }, + ), + ) + handleMeshtasticUri("$DEEP_LINK_BASE_URI/map".toUri()) + } + } + } } private fun createShareIntent(message: String): PendingIntent { diff --git a/androidApp/src/main/kotlin/org/meshtastic/app/SharedMapFile.kt b/androidApp/src/main/kotlin/org/meshtastic/app/SharedMapFile.kt new file mode 100644 index 0000000000..5a16a94ee4 --- /dev/null +++ b/androidApp/src/main/kotlin/org/meshtastic/app/SharedMapFile.kt @@ -0,0 +1,144 @@ +/* + * Copyright (c) 2026 Meshtastic LLC + * + * 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 org.meshtastic.app + +import android.content.ContentResolver +import android.net.Uri +import android.provider.OpenableColumns +import co.touchlab.kermit.Logger +import org.meshtastic.feature.map.layers.LayerType +import org.meshtastic.feature.map.layers.MAX_KMZ_INFLATED_BYTES +import org.meshtastic.feature.map.layers.resolveLayerType +import java.io.ByteArrayOutputStream +import java.io.InputStream + +/** What another app's VIEW or SEND says about a map file, read from its provider before any of the file is. */ +internal data class SharedMapFile( + val scheme: String?, + val displayName: String, + val mimeType: String?, + val size: Long?, +) { + /** Same precedence as the in-app picker: the name's extension, then the MIME subtype. */ + val extensionOrMime: String? + get() = displayName.substringAfterLast('.', "").ifBlank { mimeType?.substringAfterLast('/') } + + fun rejection(maxBytes: Long = MAX_SHARED_MAP_FILE_BYTES): SharedMapFileRejection? = when { + // The app holds no storage permission, so any file:// it can open is its own private data or a system path. + scheme != ContentResolver.SCHEME_CONTENT -> SharedMapFileRejection.NOT_CONTENT_URI + + resolveLayerType(extensionOrMime) !in SHAREABLE_LAYER_TYPES -> SharedMapFileRejection.UNSUPPORTED_TYPE + + size != null && size > maxBytes -> SharedMapFileRejection.TOO_LARGE + + else -> null + } +} + +internal enum class SharedMapFileRejection { + NOT_CONTENT_URI, + UNSUPPORTED_TYPE, + TOO_LARGE, + UNREADABLE, +} + +/** No bigger than the most the KMZ reader will inflate, so a larger file could never be read whole anyway. */ +internal const val MAX_SHARED_MAP_FILE_BYTES: Long = MAX_KMZ_INFLATED_BYTES +internal const val MAX_SHARED_MAP_FILE_MB = (MAX_SHARED_MAP_FILE_BYTES / (1024L * 1024L)).toInt() + +private val SHAREABLE_LAYER_TYPES = setOf(LayerType.KML, LayerType.GEOJSON) + +private const val TAG = "SharedMapFile" + +/** A shared map file read whole, or why it wasn't. */ +internal sealed interface SharedMapFileLoad { + class Loaded(val file: SharedMapFile, val bytes: ByteArray) : SharedMapFileLoad + + data class Refused(val reason: SharedMapFileRejection) : SharedMapFileLoad +} + +/** + * Checks [uri] and reads it at most once, never past [maxBytes]. Blocking provider IPC, so never on the main thread. + * Nothing is opened when the metadata alone refuses the file. + */ +internal fun ContentResolver.loadSharedMapFile( + uri: Uri, + maxBytes: Long = MAX_SHARED_MAP_FILE_BYTES, +): SharedMapFileLoad { + val file = sharedMapFile(uri) ?: return SharedMapFileLoad.Refused(SharedMapFileRejection.UNREADABLE) + val refusal = file.rejection(maxBytes) + return if (refusal != null) SharedMapFileLoad.Refused(refusal) else readSharedMapFile(uri, file, maxBytes) +} + +/** + * Metadata only, null if the provider throws. Nothing is queried for a non-`content://` [uri], which + * [SharedMapFile.rejection] refuses anyway. + */ +internal fun ContentResolver.sharedMapFile(uri: Uri): SharedMapFile? { + val fallbackName = uri.lastPathSegment.orEmpty() + if (uri.scheme != ContentResolver.SCHEME_CONTENT) return SharedMapFile(uri.scheme, fallbackName, null, null) + return try { + val mimeType = getType(uri) + val nameAndSize = queryNameAndSize(uri) + SharedMapFile(uri.scheme, nameAndSize?.first ?: fallbackName, mimeType, nameAndSize?.second) + } catch (@Suppress("TooGenericExceptionCaught") e: Exception) { + // Another app's provider: binder rethrows whatever it throws. + Logger.withTag(TAG).w(e) { "Shared map file provider failed its metadata query" } + null + } +} + +private fun ContentResolver.queryNameAndSize(uri: Uri): Pair? = + query(uri, arrayOf(OpenableColumns.DISPLAY_NAME, OpenableColumns.SIZE), null, null, null)?.use { cursor -> + if (!cursor.moveToFirst()) return@use null + val nameIndex = cursor.getColumnIndex(OpenableColumns.DISPLAY_NAME) + val sizeIndex = cursor.getColumnIndex(OpenableColumns.SIZE) + val name = if (nameIndex >= 0 && !cursor.isNull(nameIndex)) cursor.getString(nameIndex) else null + val size = if (sizeIndex >= 0 && !cursor.isNull(sizeIndex)) cursor.getLong(sizeIndex) else null + name to size + } + +/** A reported size is only a claim, so the cap holds on the stream too. */ +private fun ContentResolver.readSharedMapFile(uri: Uri, file: SharedMapFile, maxBytes: Long): SharedMapFileLoad = try { + val stream = openInputStream(uri) + val bytes = stream?.use { it.readAtMost(maxBytes) } + when { + stream == null -> SharedMapFileLoad.Refused(SharedMapFileRejection.UNREADABLE) + bytes == null -> SharedMapFileLoad.Refused(SharedMapFileRejection.TOO_LARGE) + else -> SharedMapFileLoad.Loaded(file, bytes) + } +} catch (@Suppress("TooGenericExceptionCaught") e: Exception) { + Logger.withTag(TAG).w(e) { "Could not read the shared map file" } + SharedMapFileLoad.Refused(SharedMapFileRejection.UNREADABLE) +} + +/** Every byte of the stream, or null once it runs past [maxBytes]. */ +internal fun InputStream.readAtMost(maxBytes: Long): ByteArray? { + val out = ByteArrayOutputStream() + val buffer = ByteArray(DEFAULT_BUFFER_SIZE) + var total = 0L + while (true) { + val count = read(buffer) + if (count < 0) return out.toByteArray() + total += count + if (total > maxBytes) { + Logger.withTag(TAG).w { "Refusing a shared map file past the ${maxBytes}B cap" } + return null + } + out.write(buffer, 0, count) + } +} diff --git a/androidApp/src/test/kotlin/org/meshtastic/app/SharedMapFileTest.kt b/androidApp/src/test/kotlin/org/meshtastic/app/SharedMapFileTest.kt new file mode 100644 index 0000000000..40bfda8ee0 --- /dev/null +++ b/androidApp/src/test/kotlin/org/meshtastic/app/SharedMapFileTest.kt @@ -0,0 +1,299 @@ +/* + * Copyright (c) 2026 Meshtastic LLC + * + * 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 org.meshtastic.app + +import android.content.ContentProvider +import android.content.ContentResolver +import android.content.ContentValues +import android.content.Context +import android.database.Cursor +import android.database.MatrixCursor +import android.net.Uri +import android.os.ParcelFileDescriptor +import android.provider.OpenableColumns +import androidx.test.core.app.ApplicationProvider +import org.junit.Rule +import org.junit.rules.TemporaryFolder +import org.junit.runner.RunWith +import org.robolectric.Robolectric +import org.robolectric.RobolectricTestRunner +import java.io.ByteArrayInputStream +import java.io.File +import kotlin.test.Test +import kotlin.test.assertContentEquals +import kotlin.test.assertEquals +import kotlin.test.assertIs +import kotlin.test.assertNotNull +import kotlin.test.assertNull + +@RunWith(RobolectricTestRunner::class) +class SharedMapFileTest { + + @get:Rule val temp = TemporaryFolder() + + private fun shared( + displayName: String, + mimeType: String? = null, + size: Long? = null, + scheme: String? = ContentResolver.SCHEME_CONTENT, + ) = SharedMapFile(scheme = scheme, displayName = displayName, mimeType = mimeType, size = size) + + @Test + fun `kml kmz geojson and json files are accepted by their extension`() { + listOf("route.kml", "route.kmz", "coverage.geojson", "area.json", "ROUTE.KML").forEach { name -> + assertNull(shared(name, mimeType = "application/octet-stream").rejection(), name) + } + } + + @Test + fun `a file with no extension is accepted by a map MIME type`() { + listOf( + "application/vnd.google-earth.kml+xml", + "application/vnd.google-earth.kmz", + "application/geo+json", + "application/vnd.geo+json", + "application/json", + ) + .forEach { mime -> assertNull(shared("export", mimeType = mime).rejection(), mime) } + } + + @Test + fun `a file whose extension is not a map format is refused`() { + listOf("photo.jpg", "track.gpx", "notes.txt", "route.kml.zip").forEach { name -> + assertEquals( + SharedMapFileRejection.UNSUPPORTED_TYPE, + shared(name, mimeType = "application/json").rejection(), + name, + ) + } + } + + @Test + fun `a file with no extension and a non-map MIME type is refused`() { + listOf("image/png", "text/plain", "application/octet-stream").forEach { mime -> + assertEquals(SharedMapFileRejection.UNSUPPORTED_TYPE, shared("export", mimeType = mime).rejection(), mime) + } + assertEquals(SharedMapFileRejection.UNSUPPORTED_TYPE, shared("export", mimeType = null).rejection()) + } + + @Test + fun `a coverage file from another app is refused`() { + assertEquals(SharedMapFileRejection.UNSUPPORTED_TYPE, shared("estimate.coverage").rejection()) + } + + @Test + fun `a file uri is refused whatever it names`() { + listOf("route.kml", "coverage.geojson").forEach { name -> + assertEquals( + SharedMapFileRejection.NOT_CONTENT_URI, + shared(name, scheme = ContentResolver.SCHEME_FILE, size = 10).rejection(), + name, + ) + } + assertEquals(SharedMapFileRejection.NOT_CONTENT_URI, shared("route.kml", scheme = null).rejection()) + } + + @Test + fun `a file reported over the cap is refused before it is read`() { + assertEquals(SharedMapFileRejection.TOO_LARGE, shared("route.kml", size = 101).rejection(maxBytes = 100)) + assertNull(shared("route.kml", size = 100).rejection(maxBytes = 100)) + assertNull(shared("route.kml", size = 0).rejection(maxBytes = 100)) + } + + @Test + fun `a file of unknown size is left to the capped read`() { + assertNull(shared("route.kml", size = null).rejection(maxBytes = 100)) + } + + @Test + fun `a read that runs past the cap returns nothing`() { + assertNull(ByteArrayInputStream(ByteArray(DEFAULT_BUFFER_SIZE * 3 + 1)).readAtMost(DEFAULT_BUFFER_SIZE * 3L)) + assertNull(ByteArrayInputStream(ByteArray(101)).readAtMost(100)) + } + + @Test + fun `a read at the cap returns every byte`() { + val bytes = ByteArray(DEFAULT_BUFFER_SIZE * 2 + 7) { it.toByte() } + assertContentEquals(bytes, ByteArrayInputStream(bytes).readAtMost(bytes.size.toLong())) + assertContentEquals(ByteArray(0), ByteArrayInputStream(ByteArray(0)).readAtMost(100)) + } + + @Test + fun `the provider's name size and type are what the check sees`() { + val provider = registerProvider() + provider.displayName = "Ridge Trail.kml" + provider.size = 42L + provider.mimeType = "application/vnd.google-earth.kml+xml" + + assertEquals( + SharedMapFile( + scheme = ContentResolver.SCHEME_CONTENT, + displayName = "Ridge Trail.kml", + mimeType = "application/vnd.google-earth.kml+xml", + size = 42L, + ), + resolver.sharedMapFile(uri), + ) + } + + @Test + fun `a provider with no name or size falls back to the last path segment`() { + val provider = registerProvider() + provider.displayName = null + provider.size = null + provider.mimeType = "application/geo+json" + + val shared = assertNotNull(resolver.sharedMapFile(uri)) + + assertEquals("shared-layer", shared.displayName) + assertNull(shared.size) + assertEquals("geo+json", shared.extensionOrMime) + } + + @Test + fun `a provider that throws on the metadata query is refused unopened`() { + val provider = registerProvider() + provider.mimeType = "application/geo+json" + provider.refuse = true + + assertEquals(SharedMapFileLoad.Refused(SharedMapFileRejection.UNREADABLE), resolver.loadSharedMapFile(uri)) + assertEquals(0, provider.opens) + } + + @Test + fun `a file uri is not queried`() { + val fileUri = Uri.fromFile(File("/data/data/com.geeksville.mesh/databases/meshtastic.db")) + + assertEquals( + SharedMapFile(ContentResolver.SCHEME_FILE, "meshtastic.db", null, null), + resolver.sharedMapFile(fileUri), + ) + } + + @Test + fun `a map file of unknown size is read whole up to the cap and refused past it`() { + val provider = registerProvider() + provider.displayName = "layer.geojson" + val bytes = """{"type":"FeatureCollection","features":[]}""".encodeToByteArray() + provider.file = temp.newFile("layer.geojson").apply { writeBytes(bytes) } + + val loaded = assertIs(resolver.loadSharedMapFile(uri, maxBytes = bytes.size.toLong())) + assertContentEquals(bytes, loaded.bytes) + assertEquals("layer.geojson", loaded.file.displayName) + assertEquals( + SharedMapFileLoad.Refused(SharedMapFileRejection.TOO_LARGE), + resolver.loadSharedMapFile(uri, maxBytes = bytes.size - 1L), + ) + } + + @Test + fun `a file the metadata refuses is never opened`() { + val provider = registerProvider() + provider.file = temp.newFile("photo.jpg") + + provider.displayName = "photo.jpg" + assertEquals( + SharedMapFileLoad.Refused(SharedMapFileRejection.UNSUPPORTED_TYPE), + resolver.loadSharedMapFile(uri), + ) + + provider.displayName = "route.kml" + provider.size = 101L + assertEquals( + SharedMapFileLoad.Refused(SharedMapFileRejection.TOO_LARGE), + resolver.loadSharedMapFile(uri, maxBytes = 100), + ) + + assertEquals(0, provider.opens) + } + + @Test + fun `a provider that throws on open is refused instead of crashing`() { + val provider = registerProvider() + provider.displayName = "route.kml" + listOf(SecurityException("no grant"), IllegalStateException("provider bug")).forEach { error -> + provider.openError = error + + assertEquals( + SharedMapFileLoad.Refused(SharedMapFileRejection.UNREADABLE), + resolver.loadSharedMapFile(uri), + error.toString(), + ) + } + } + + @Test + fun `a file uri is refused without being opened`() { + val fileUri = Uri.fromFile(temp.newFile("route.kml")) + + assertEquals( + SharedMapFileLoad.Refused(SharedMapFileRejection.NOT_CONTENT_URI), + resolver.loadSharedMapFile(fileUri), + ) + } + + private val resolver: ContentResolver + get() = ApplicationProvider.getApplicationContext().contentResolver + + private val uri = Uri.parse("content://$AUTHORITY/shared-layer") + + private fun registerProvider(): FakeMapFileProvider = + Robolectric.buildContentProvider(FakeMapFileProvider::class.java).create(AUTHORITY).get() + + class FakeMapFileProvider : ContentProvider() { + var displayName: String? = null + var size: Long? = null + var mimeType: String? = null + var file: File? = null + var refuse = false + var openError: RuntimeException? = null + var opens = 0 + + override fun onCreate() = true + + override fun query( + uri: Uri, + projection: Array?, + selection: String?, + selectionArgs: Array?, + sortOrder: String?, + ): Cursor { + if (refuse) throw SecurityException("no grant") + return MatrixCursor(arrayOf(OpenableColumns.DISPLAY_NAME, OpenableColumns.SIZE)).apply { + addRow(arrayOf(displayName, size)) + } + } + + override fun getType(uri: Uri): String? = mimeType + + override fun openFile(uri: Uri, mode: String): ParcelFileDescriptor { + opens++ + openError?.let { throw it } + return ParcelFileDescriptor.open(checkNotNull(file), ParcelFileDescriptor.MODE_READ_ONLY) + } + + override fun insert(uri: Uri, values: ContentValues?): Uri? = null + + override fun delete(uri: Uri, selection: String?, selectionArgs: Array?) = 0 + + override fun update(uri: Uri, values: ContentValues?, selection: String?, selectionArgs: Array?) = 0 + } + + private companion object { + const val AUTHORITY = "org.meshtastic.test.sharedmapfile" + } +} diff --git a/core/resources/src/commonMain/composeResources/values/strings.xml b/core/resources/src/commonMain/composeResources/values/strings.xml index 977f58cb54..e5a4f55106 100644 --- a/core/resources/src/commonMain/composeResources/values/strings.xml +++ b/core/resources/src/commonMain/composeResources/values/strings.xml @@ -1022,6 +1022,8 @@ Filter map Map layers support .kml, .kmz, or GeoJSON formats. Opacity: %1$d% + Couldn't open that map file. + Map files over %1$d MB can't be imported. %1$s<br>Last heard: %2$s<br>Last position: %3$s<br>Battery: %4$s Offline — showing cached map data No offline map data available right now — try again later