From e93273bff49ba4b7692f2b93d238ad15e1ea1316 Mon Sep 17 00:00:00 2001 From: Ole-Morten Duesund Date: Sun, 8 Mar 2026 17:53:51 +0100 Subject: [PATCH] Fix data safety, security, and coroutine correctness MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Wrap deleteAll+insertAll in Room transaction to prevent data loss on crash during refresh - Add CancellationException rethrow in ShelterRepository and MapCacheManager to preserve structured concurrency - Close OkHttp response body on error paths (response.use{}) - Add ZIP bomb protection (10MB cap) in GeoJSON parser - Add per-feature error handling — skip malformed records instead of losing all shelters - Validate coordinates within Norway's bounding box - Enforce HTTPS-only via network_security_config (remove cleartext allowance for tile.openstreetmap.org) - Disable android:allowBackup to prevent DB extraction via ADB - Strip Log.v/d/i in release builds via ProGuard to prevent location data leakage - Restore map position in MapCacheManager.finally block on cancellation Co-Authored-By: Claude Opus 4.6 --- app/proguard-rules.pro | 7 ++ app/src/main/AndroidManifest.xml | 2 +- .../naiv/tilfluktsrom/data/MapCacheManager.kt | 26 ++++-- .../tilfluktsrom/data/ShelterGeoJsonParser.kt | 91 +++++++++++++++---- .../tilfluktsrom/data/ShelterRepository.kt | 53 ++++++----- .../main/res/xml/network_security_config.xml | 6 +- 6 files changed, 133 insertions(+), 52 deletions(-) diff --git a/app/proguard-rules.pro b/app/proguard-rules.pro index 4f6449b..4805faa 100644 --- a/app/proguard-rules.pro +++ b/app/proguard-rules.pro @@ -10,3 +10,10 @@ # OkHttp -dontwarn okhttp3.** -dontwarn okio.** + +# Strip verbose/debug/info logs in release builds (prevent location data leakage) +-assumenosideeffects class android.util.Log { + public static int v(...); + public static int d(...); + public static int i(...); +} diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index 5e49b4e..be4c3fe 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -12,7 +12,7 @@ Unit = {} ): Boolean = withContext(Dispatchers.Main) { + // Save map state so we can restore it even on cancellation + val savedZoom = mapView.zoomLevelDouble + val savedCenter = mapView.mapCenter + try { - Log.i(TAG, "Seeding tile cache for area around $latitude, $longitude") + Log.d(TAG, "Seeding tile cache around user location") val totalSteps = CACHE_ZOOM_LEVELS.size * GRID_SIZE * GRID_SIZE var step = 0 @@ -86,22 +92,16 @@ class MapCacheManager(private val context: Context) { (2 * CACHE_RADIUS_DEGREES * col) / (GRID_SIZE - 1) mapView.controller.setCenter(GeoPoint(lat, lon)) - // Force a layout pass so tiles are requested mapView.invalidate() step++ onProgress(step.toFloat() / totalSteps) - // Brief delay to allow tile loading to start delay(300) } } } - // Restore to user's location - mapView.controller.setZoom(14.0) - mapView.controller.setCenter(GeoPoint(latitude, longitude)) - prefs.edit() .putLong(KEY_CACHED_LAT, latitude.toBits()) .putLong(KEY_CACHED_LON, longitude.toBits()) @@ -109,11 +109,21 @@ class MapCacheManager(private val context: Context) { .putBoolean(KEY_CACHE_COMPLETE, true) .apply() - Log.i(TAG, "Tile cache seeding complete") + Log.d(TAG, "Tile cache seeding complete") true + } catch (e: CancellationException) { + throw e // Never swallow coroutine cancellation } catch (e: Exception) { Log.e(TAG, "Failed to seed tile cache", e) false + } finally { + // Always restore map to user's location, even on cancellation + withContext(NonCancellable) { + mapView.controller.setZoom(savedZoom) + mapView.controller.setCenter( + GeoPoint(savedCenter.latitude, savedCenter.longitude) + ) + } } } } diff --git a/app/src/main/java/no/naiv/tilfluktsrom/data/ShelterGeoJsonParser.kt b/app/src/main/java/no/naiv/tilfluktsrom/data/ShelterGeoJsonParser.kt index 833bd45..92a34f6 100644 --- a/app/src/main/java/no/naiv/tilfluktsrom/data/ShelterGeoJsonParser.kt +++ b/app/src/main/java/no/naiv/tilfluktsrom/data/ShelterGeoJsonParser.kt @@ -1,5 +1,6 @@ package no.naiv.tilfluktsrom.data +import android.util.Log import no.naiv.tilfluktsrom.util.CoordinateConverter import org.json.JSONObject import java.io.ByteArrayOutputStream @@ -12,6 +13,17 @@ import java.util.zip.ZipInputStream */ object ShelterGeoJsonParser { + private const val TAG = "ShelterGeoJsonParser" + + // Maximum uncompressed size to prevent ZIP bomb attacks (10 MB) + private const val MAX_UNCOMPRESSED_SIZE = 10 * 1024 * 1024L + + // Norway's approximate bounding box in WGS84 + private const val NORWAY_LAT_MIN = 57.0 + private const val NORWAY_LAT_MAX = 72.0 + private const val NORWAY_LON_MIN = 3.0 + private const val NORWAY_LON_MAX = 33.0 + /** * Extract and parse GeoJSON from a ZIP input stream. */ @@ -26,7 +38,18 @@ object ShelterGeoJsonParser { while (entry != null) { if (entry.name.endsWith(".geojson") || entry.name.endsWith(".json")) { val buffer = ByteArrayOutputStream() - zis.copyTo(buffer) + val chunk = ByteArray(8192) + var totalRead = 0L + var bytesRead: Int + while (zis.read(chunk).also { bytesRead = it } != -1) { + totalRead += bytesRead + if (totalRead > MAX_UNCOMPRESSED_SIZE) { + throw IllegalStateException( + "GeoJSON exceeds maximum size (${MAX_UNCOMPRESSED_SIZE / 1024}KB)" + ) + } + buffer.write(chunk, 0, bytesRead) + } return buffer.toString(Charsets.UTF_8.name()) } entry = zis.nextEntry @@ -37,33 +60,67 @@ object ShelterGeoJsonParser { /** * Parse raw GeoJSON string into Shelter objects. + * Malformed individual features are skipped rather than failing the entire parse. */ fun parseGeoJson(json: String): List { val root = JSONObject(json) val features = root.getJSONArray("features") val shelters = mutableListOf() + var skipped = 0 for (i in 0 until features.length()) { - val feature = features.getJSONObject(i) - val geometry = feature.getJSONObject("geometry") - val properties = feature.getJSONObject("properties") + try { + val feature = features.getJSONObject(i) + val geometry = feature.getJSONObject("geometry") + val properties = feature.getJSONObject("properties") - val coordinates = geometry.getJSONArray("coordinates") - val easting = coordinates.getDouble(0) - val northing = coordinates.getDouble(1) + val coordinates = geometry.getJSONArray("coordinates") + val easting = coordinates.getDouble(0) + val northing = coordinates.getDouble(1) - // Convert UTM33N to WGS84 - val latLon = CoordinateConverter.utm33nToWgs84(easting, northing) + // Convert UTM33N to WGS84 + val latLon = CoordinateConverter.utm33nToWgs84(easting, northing) - shelters.add( - Shelter( - lokalId = properties.optString("lokalId", "unknown-$i"), - romnr = properties.optInt("romnr", 0), - plasser = properties.optInt("plasser", 0), - adresse = properties.optString("adresse", ""), - latitude = latLon.latitude, - longitude = latLon.longitude + // Validate coordinates are within Norway + if (latLon.latitude !in NORWAY_LAT_MIN..NORWAY_LAT_MAX || + latLon.longitude !in NORWAY_LON_MIN..NORWAY_LON_MAX + ) { + Log.w(TAG, "Skipping shelter at index $i: coordinates outside Norway " + + "(${latLon.latitude}, ${latLon.longitude})") + skipped++ + continue + } + + val plasser = properties.optInt("plasser", 0) + if (plasser < 0) { + Log.w(TAG, "Skipping shelter at index $i: negative capacity ($plasser)") + skipped++ + continue + } + + shelters.add( + Shelter( + lokalId = properties.optString("lokalId", "unknown-$i"), + romnr = properties.optInt("romnr", 0), + plasser = plasser, + adresse = properties.optString("adresse", ""), + latitude = latLon.latitude, + longitude = latLon.longitude + ) ) + } catch (e: Exception) { + Log.w(TAG, "Skipping malformed shelter feature at index $i", e) + skipped++ + } + } + + if (skipped > 0) { + Log.w(TAG, "Skipped $skipped malformed features out of ${features.length()}") + } + + if (shelters.isEmpty()) { + throw IllegalStateException( + "No valid shelters found in GeoJSON (${features.length()} features were malformed)" ) } diff --git a/app/src/main/java/no/naiv/tilfluktsrom/data/ShelterRepository.kt b/app/src/main/java/no/naiv/tilfluktsrom/data/ShelterRepository.kt index 7c083f7..ab68b77 100644 --- a/app/src/main/java/no/naiv/tilfluktsrom/data/ShelterRepository.kt +++ b/app/src/main/java/no/naiv/tilfluktsrom/data/ShelterRepository.kt @@ -2,6 +2,8 @@ package no.naiv.tilfluktsrom.data import android.content.Context import android.util.Log +import androidx.room.withTransaction +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.withContext @@ -55,7 +57,7 @@ class ShelterRepository(context: Context) { */ suspend fun refreshData(): Boolean = withContext(Dispatchers.IO) { try { - Log.i(TAG, "Downloading shelter data from Geonorge...") + Log.d(TAG, "Downloading shelter data from Geonorge...") val request = Request.Builder() .url(SHELTER_DATA_URL) @@ -63,28 +65,35 @@ class ShelterRepository(context: Context) { .build() val response = client.newCall(request).execute() - if (!response.isSuccessful) { - Log.e(TAG, "Download failed: HTTP ${response.code}") - return@withContext false + response.use { resp -> + if (!resp.isSuccessful) { + Log.e(TAG, "Download failed: HTTP ${resp.code}") + return@withContext false + } + + val body = resp.body ?: run { + Log.e(TAG, "Empty response body") + return@withContext false + } + + val shelters = body.byteStream().use { stream -> + ShelterGeoJsonParser.parseFromZip(stream) + } + + Log.d(TAG, "Parsed ${shelters.size} shelters, saving to database...") + + // Atomic replace: delete + insert in a single transaction + db.withTransaction { + dao.deleteAll() + dao.insertAll(shelters) + } + + prefs.edit().putLong(KEY_LAST_UPDATE, System.currentTimeMillis()).apply() + Log.d(TAG, "Shelter data updated successfully") + true } - - val body = response.body ?: run { - Log.e(TAG, "Empty response body") - return@withContext false - } - - val shelters = body.byteStream().use { stream -> - ShelterGeoJsonParser.parseFromZip(stream) - } - - Log.i(TAG, "Parsed ${shelters.size} shelters, saving to database...") - - dao.deleteAll() - dao.insertAll(shelters) - - prefs.edit().putLong(KEY_LAST_UPDATE, System.currentTimeMillis()).apply() - Log.i(TAG, "Shelter data updated successfully") - true + } catch (e: CancellationException) { + throw e // Never swallow coroutine cancellation } catch (e: Exception) { Log.e(TAG, "Failed to refresh shelter data", e) false diff --git a/app/src/main/res/xml/network_security_config.xml b/app/src/main/res/xml/network_security_config.xml index a566555..1ef5a6e 100644 --- a/app/src/main/res/xml/network_security_config.xml +++ b/app/src/main/res/xml/network_security_config.xml @@ -1,7 +1,5 @@ - - - tile.openstreetmap.org - + +