From 073fbd402f63dc20c1e925535ef5ad0c6643a947 Mon Sep 17 00:00:00 2001 From: Dale Hawkins <107309+dkhawk@users.noreply.github.com> Date: Wed, 5 Aug 2026 09:33:26 -0600 Subject: [PATCH 1/2] build(deps): bump xmlutil serialization to 1.0.1 and harden data layer parsers - Update io.github.pdvrieze.xmlutil:serialization in libs.versions.toml from 0.91.3 to 1.0.1. - Migrate XML {} configuration DSL in GpxParser and KmlParser by promoting isCollectingNSAttributes to the top-level XML {} builder per xmlutil 1.0.0+ DSL changes. - Update expected exception for malformed KML coordinate tests in KmlParserTest from XmlParsingException to XmlException per xmlutil 1.0.x error hierarchy. - Harden GeoJSON, KML, and GPX coordinate models (Coordinates, LatLngAlt, Wpt) by enforcing finiteness checks in init blocks and serializers to prevent NaN/Infinity poisoning from reaching LatLng/LatLngBounds. - Sanitize non-finite numeric style properties (stroke-width, fill-opacity, stroke-opacity, width, scale) in GeoJsonMapper and KmlMapper to safe visual defaults. - Add comprehensive SecurityHardeningTest suite covering adversarial coordinate poisoning and non-finite style injection across all three spatial formats. --- .../data/parser/geojson/GeoJsonObjects.kt | 8 +- .../maps/android/data/parser/gpx/GpxModel.kt | 8 +- .../maps/android/data/parser/gpx/GpxParser.kt | 2 +- .../maps/android/data/parser/kml/KmlParser.kt | 2 +- .../maps/android/data/parser/kml/LatLngAlt.kt | 8 +- .../data/parser/kml/LatLngAltSerializer.kt | 12 +- .../data/renderer/mapper/GeoJsonMapper.kt | 8 +- .../android/data/renderer/mapper/KmlMapper.kt | 6 +- .../data/parser/SecurityHardeningTest.kt | 202 ++++++++++++++++++ .../android/data/parser/kml/KmlParserTest.kt | 8 +- gradle/libs.versions.toml | 2 +- 11 files changed, 247 insertions(+), 19 deletions(-) create mode 100644 data/src/test/java/com/google/maps/android/data/parser/SecurityHardeningTest.kt diff --git a/data/src/main/java/com/google/maps/android/data/parser/geojson/GeoJsonObjects.kt b/data/src/main/java/com/google/maps/android/data/parser/geojson/GeoJsonObjects.kt index cea7bcaef..f9abf798a 100644 --- a/data/src/main/java/com/google/maps/android/data/parser/geojson/GeoJsonObjects.kt +++ b/data/src/main/java/com/google/maps/android/data/parser/geojson/GeoJsonObjects.kt @@ -26,7 +26,13 @@ package com.google.maps.android.data.parser.geojson * @property lng The longitude of the coordinate. * @property alt The altitude of the coordinate, in meters. Optional. */ -data class Coordinates(val lat: Double, val lng: Double, val alt: Double? = null) +data class Coordinates(val lat: Double, val lng: Double, val alt: Double? = null) { + init { + require(lat.isFinite() && lng.isFinite() && (alt == null || alt.isFinite())) { + "GeoJSON coordinate contains a non-finite value" + } + } +} // Using a sealed interface for all GeoJSON objects sealed interface GeoJsonObject { diff --git a/data/src/main/java/com/google/maps/android/data/parser/gpx/GpxModel.kt b/data/src/main/java/com/google/maps/android/data/parser/gpx/GpxModel.kt index a59fc6840..67b207cd6 100644 --- a/data/src/main/java/com/google/maps/android/data/parser/gpx/GpxModel.kt +++ b/data/src/main/java/com/google/maps/android/data/parser/gpx/GpxModel.kt @@ -78,7 +78,13 @@ data class Wpt( @XmlElement(true) @XmlSerialName("sym", namespace = GPX_NAMESPACE, prefix = "") val sym: String? = null, -) +) { + init { + require(lat.isFinite() && lon.isFinite() && (ele == null || ele.isFinite())) { + "GPX coordinate contains a non-finite value" + } + } +} @Serializable @XmlSerialName("rte", namespace = GPX_NAMESPACE, prefix = "") diff --git a/data/src/main/java/com/google/maps/android/data/parser/gpx/GpxParser.kt b/data/src/main/java/com/google/maps/android/data/parser/gpx/GpxParser.kt index 62b812b62..313447175 100644 --- a/data/src/main/java/com/google/maps/android/data/parser/gpx/GpxParser.kt +++ b/data/src/main/java/com/google/maps/android/data/parser/gpx/GpxParser.kt @@ -31,8 +31,8 @@ class GpxParser { XML { defaultPolicy { ignoreUnknownChildren() - isCollectingNSAttributes = true } + isCollectingNSAttributes = true } fun parse(inputStream: InputStream): Gpx { diff --git a/data/src/main/java/com/google/maps/android/data/parser/kml/KmlParser.kt b/data/src/main/java/com/google/maps/android/data/parser/kml/KmlParser.kt index e65648664..7e2d53505 100644 --- a/data/src/main/java/com/google/maps/android/data/parser/kml/KmlParser.kt +++ b/data/src/main/java/com/google/maps/android/data/parser/kml/KmlParser.kt @@ -29,8 +29,8 @@ class KmlParser { XML { defaultPolicy { ignoreUnknownChildren() - isCollectingNSAttributes = true } + isCollectingNSAttributes = true } fun parseAsKml(inputStream: InputStream): Kml { diff --git a/data/src/main/java/com/google/maps/android/data/parser/kml/LatLngAlt.kt b/data/src/main/java/com/google/maps/android/data/parser/kml/LatLngAlt.kt index a1e17de83..9f8058b0b 100644 --- a/data/src/main/java/com/google/maps/android/data/parser/kml/LatLngAlt.kt +++ b/data/src/main/java/com/google/maps/android/data/parser/kml/LatLngAlt.kt @@ -22,4 +22,10 @@ data class LatLngAlt( val latitude: Double, val longitude: Double, val altitude: Double? = null, -) +) { + init { + require(latitude.isFinite() && longitude.isFinite() && (altitude == null || altitude.isFinite())) { + "KML coordinate contains a non-finite value" + } + } +} diff --git a/data/src/main/java/com/google/maps/android/data/parser/kml/LatLngAltSerializer.kt b/data/src/main/java/com/google/maps/android/data/parser/kml/LatLngAltSerializer.kt index f2b90b098..bdaeff94a 100644 --- a/data/src/main/java/com/google/maps/android/data/parser/kml/LatLngAltSerializer.kt +++ b/data/src/main/java/com/google/maps/android/data/parser/kml/LatLngAltSerializer.kt @@ -37,10 +37,16 @@ internal object LatLngAltSerializer : KSerializer { internal fun parse(string: String): LatLngAlt { val parts = string.split(",").map { it.trim().toDouble() } + val lng = parts[0] + val lat = parts[1] + val alt = parts.getOrNull(2) + require(lng.isFinite() && lat.isFinite() && (alt == null || alt.isFinite())) { + "KML coordinate contains a non-finite value" + } return LatLngAlt( - longitude = parts[0], - latitude = parts[1], - altitude = parts.getOrNull(2), + longitude = lng, + latitude = lat, + altitude = alt, ) } } diff --git a/data/src/main/java/com/google/maps/android/data/renderer/mapper/GeoJsonMapper.kt b/data/src/main/java/com/google/maps/android/data/renderer/mapper/GeoJsonMapper.kt index 12eb6caa8..29dc965fe 100644 --- a/data/src/main/java/com/google/maps/android/data/renderer/mapper/GeoJsonMapper.kt +++ b/data/src/main/java/com/google/maps/android/data/renderer/mapper/GeoJsonMapper.kt @@ -95,7 +95,7 @@ object GeoJsonMapper { geometry is LineString || (geometry is MultiGeometry && !geometry.isPolygonal()) -> { // MultiGeometry could contain lines val strokeColor = props["stroke"]?.let { parseColor(it) } - val strokeWidth = props["stroke-width"]?.toFloatOrNull() + val strokeWidth = props["stroke-width"]?.toFloatOrNull()?.takeIf { it.isFinite() && it >= 0f } if (strokeColor != null || strokeWidth != null) { LineStyle( color = strokeColor ?: 0xFF000000.toInt(), @@ -105,10 +105,10 @@ object GeoJsonMapper { } geometry is ModelPolygon || (geometry is MultiGeometry && geometry.isPolygonal()) -> { val strokeColor = props["stroke"]?.let { parseColor(it) } - val strokeWidth = props["stroke-width"]?.toFloatOrNull() + val strokeWidth = props["stroke-width"]?.toFloatOrNull()?.takeIf { it.isFinite() && it >= 0f } val fillColor = props["fill"]?.let { parseColor(it) } - val fillOpacity = props["fill-opacity"]?.toFloatOrNull() - val strokeOpacity = props["stroke-opacity"]?.toFloatOrNull() + val fillOpacity = props["fill-opacity"]?.toFloatOrNull()?.takeIf { it.isFinite() && it in 0f..1f } + val strokeOpacity = props["stroke-opacity"]?.toFloatOrNull()?.takeIf { it.isFinite() && it in 0f..1f } val finalFillColor = if (fillColor != null && fillOpacity != null) { applyOpacity(fillColor, fillOpacity) diff --git a/data/src/main/java/com/google/maps/android/data/renderer/mapper/KmlMapper.kt b/data/src/main/java/com/google/maps/android/data/renderer/mapper/KmlMapper.kt index 7f47f5858..68476ce03 100644 --- a/data/src/main/java/com/google/maps/android/data/renderer/mapper/KmlMapper.kt +++ b/data/src/main/java/com/google/maps/android/data/renderer/mapper/KmlMapper.kt @@ -199,7 +199,7 @@ private fun KmlStyle.toRendererStyle(geometry: Geometry): Style? = is PointGeometry -> { iconStyle?.let { PointStyle( - scale = it.scale, + scale = it.scale.takeIf { s -> s.isFinite() && s >= 0f } ?: 1.0f, iconUrl = it.icon?.href, // TODO: Map other properties like heading, hotSpot if needed ) @@ -210,7 +210,7 @@ private fun KmlStyle.toRendererStyle(geometry: Geometry): Style? = lineStyle?.let { LineStyle( color = convertKmlColor(it.color ?: 0xFF000000.toInt()), - width = it.width ?: 1.0f, + width = it.width?.takeIf { w -> w.isFinite() && w >= 0f } ?: 1.0f, ) } } @@ -220,7 +220,7 @@ private fun KmlStyle.toRendererStyle(geometry: Geometry): Style? = PolygonStyle( fillColor = if (it.fill) convertKmlColor(it.color ?: 0x00000000) else 0x00000000, strokeColor = convertKmlColor(lineStyle?.color ?: 0xFF000000.toInt()), - strokeWidth = lineStyle?.width ?: 1.0f, + strokeWidth = lineStyle?.width?.takeIf { w -> w.isFinite() && w >= 0f } ?: 1.0f, // TODO: Handle outline property ) } diff --git a/data/src/test/java/com/google/maps/android/data/parser/SecurityHardeningTest.kt b/data/src/test/java/com/google/maps/android/data/parser/SecurityHardeningTest.kt new file mode 100644 index 000000000..a44633d23 --- /dev/null +++ b/data/src/test/java/com/google/maps/android/data/parser/SecurityHardeningTest.kt @@ -0,0 +1,202 @@ +/* + * Copyright 2026 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.maps.android.data.parser + +import com.google.common.truth.Truth.assertThat +import com.google.maps.android.data.parser.geojson.GeoJsonParser +import com.google.maps.android.data.parser.gpx.GpxParser +import com.google.maps.android.data.parser.kml.KmlParser +import com.google.maps.android.data.renderer.mapper.GeoJsonMapper +import com.google.maps.android.data.renderer.mapper.toLayer +import com.google.maps.android.data.renderer.model.LineStyle +import com.google.maps.android.data.renderer.model.PolygonStyle +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import kotlin.test.assertFailsWith + +@RunWith(RobolectricTestRunner::class) +class SecurityHardeningTest { + private val kmlParser = KmlParser() + private val gpxParser = GpxParser() + private val geoJsonParser = GeoJsonParser() + + @Test + fun testKmlCoordinateNanPoisoning_throwsException() { + val kml = """ + + + + + NaN,NaN,0 + + + + + """.trimIndent() + + assertFailsWith { + val parsed = kmlParser.parseAsKml(kml.byteInputStream()) + parsed.toLayer() + } + } + + @Test + fun testKmlCoordinateInfinityPoisoning_throwsException() { + val kml = """ + + + + + 10.0,Infinity,0 + + + + + """.trimIndent() + + assertFailsWith { + val parsed = kmlParser.parseAsKml(kml.byteInputStream()) + parsed.toLayer() + } + } + + @Test + fun testGpxCoordinateNanPoisoning_throwsException() { + val gpx = """ + + + Poisoned Waypoint + + + """.trimIndent() + + assertFailsWith { + val parsed = gpxParser.parse(gpx.byteInputStream()) + parsed.toLayer() + } + } + + @Test + fun testGpxCoordinateInfinityPoisoning_throwsException() { + val gpx = """ + + + Poisoned Waypoint + + + """.trimIndent() + + assertFailsWith { + val parsed = gpxParser.parse(gpx.byteInputStream()) + parsed.toLayer() + } + } + + @Test + fun testGeoJsonNonFiniteStyleProperties_sanitizedToSafeDefaults() { + val json = """ + { + "type": "FeatureCollection", + "features": [ + { + "type": "Feature", + "geometry": { + "type": "Polygon", + "coordinates": [[[0.0, 0.0], [0.0, 10.0], [10.0, 10.0], [10.0, 0.0], [0.0, 0.0]]] + }, + "properties": { + "stroke-width": "NaN", + "fill-opacity": "Infinity", + "stroke-opacity": "-5.0", + "stroke": "#FF0000", + "fill": "#00FF00" + } + } + ] + } + """.trimIndent() + + val layer = geoJsonParser.parse(json.byteInputStream())!!.toLayer() + val feature = layer.features.first() + val style = feature.style as PolygonStyle + + // Non-finite width must fall back to safe default 1.0f rather than Float.NaN + assertThat(style.strokeWidth).isEqualTo(1.0f) + assertThat(style.strokeWidth.isFinite()).isTrue() + } + + @Test + fun testGeoJsonLineStringNonFiniteStrokeWidth_sanitizedToSafeDefault() { + val json = """ + { + "type": "FeatureCollection", + "features": [ + { + "type": "Feature", + "geometry": { + "type": "LineString", + "coordinates": [[0.0, 0.0], [10.0, 10.0]] + }, + "properties": { + "stroke-width": "Infinity", + "stroke": "#0000FF" + } + } + ] + } + """.trimIndent() + + val layer = geoJsonParser.parse(json.byteInputStream())!!.toLayer() + val feature = layer.features.first() + val style = feature.style as LineStyle + + // Infinity width must fall back to safe default 1.0f rather than Float.POSITIVE_INFINITY + assertThat(style.width).isEqualTo(1.0f) + assertThat(style.width.isFinite()).isTrue() + } + + @Test + fun testKmlNonFiniteStyleProperties_sanitizedToSafeDefaults() { + val kml = """ + + + + + #poisonedStyle + + 0,0,0 10,10,0 + + + + + """.trimIndent() + + // Should parse safely without crashing and sanitize non-finite width/scale to safe defaults + val layer = kmlParser.parseAsKml(kml.byteInputStream()).toLayer() + val feature = layer.features.first() + val style = feature.style as LineStyle + assertThat(style.width.isFinite()).isTrue() + assertThat(style.width).isAtLeast(0.0f) + } +} diff --git a/data/src/test/java/com/google/maps/android/data/parser/kml/KmlParserTest.kt b/data/src/test/java/com/google/maps/android/data/parser/kml/KmlParserTest.kt index e5c3d8c0c..174748890 100644 --- a/data/src/test/java/com/google/maps/android/data/parser/kml/KmlParserTest.kt +++ b/data/src/test/java/com/google/maps/android/data/parser/kml/KmlParserTest.kt @@ -17,6 +17,8 @@ package com.google.maps.android.data.parser.kml import com.google.common.truth.Truth.assertThat import com.google.maps.android.data.parser.kml.assertThat +import nl.adaptivity.xmlutil.XmlException +import nl.adaptivity.xmlutil.serialization.XmlParsingException import org.junit.Test import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner @@ -658,7 +660,7 @@ class KmlParserTest { @Test fun testAmuWrongNotExistLatitudeCoordinates() { val stream = File("src/test/resources/amu_wrong_not_exist_latitude_coordinates.kml").inputStream() - assertFailsWith { + assertFailsWith { parser.parseAsKml(stream) } } @@ -672,7 +674,7 @@ class KmlParserTest { append("") } - assertFailsWith { + assertFailsWith { parser.parseAsKml(kml.byteInputStream()) } } @@ -687,7 +689,7 @@ class KmlParserTest { append("") } - assertFailsWith { + assertFailsWith { parser.parseAsKml(kml.byteInputStream()) } } diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 8294b7483..7d3ea919a 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -60,7 +60,7 @@ dokka-gradle-plugin = "2.2.0" gradle = "9.3.1" gradleMavenPublishPlugin = "0.37.0" secrets-gradle-plugin = "2.0.1" -serialization = "0.91.3" +serialization = "1.0.1" [libraries] From 71a820109431cf915cb361d838d82d7d7d2e5575 Mon Sep 17 00:00:00 2001 From: Dale Hawkins <107309+dkhawk@users.noreply.github.com> Date: Wed, 5 Aug 2026 11:13:33 -0600 Subject: [PATCH 2/2] refactor(data): add KDoc to Wpt and elevation property alias - address review feedback from @kikoso --- .../maps/android/data/parser/gpx/GpxModel.kt | 19 ++++++++++++++++++- .../android/data/parser/gpx/GpxParserTest.kt | 7 +++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/data/src/main/java/com/google/maps/android/data/parser/gpx/GpxModel.kt b/data/src/main/java/com/google/maps/android/data/parser/gpx/GpxModel.kt index 67b207cd6..d501f50b0 100644 --- a/data/src/main/java/com/google/maps/android/data/parser/gpx/GpxModel.kt +++ b/data/src/main/java/com/google/maps/android/data/parser/gpx/GpxModel.kt @@ -56,6 +56,17 @@ data class Metadata( val time: String? = null, ) +/** + * Represents a GPX waypoint (``), point of interest, or named feature on a map. + * + * @property lat The latitude of the waypoint. + * @property lon The longitude of the waypoint. + * @property ele The elevation (in meters) of the waypoint, or null if unspecified. + * @property time The timestamp of the waypoint. + * @property name The name of the waypoint. + * @property desc A description of the waypoint. + * @property sym The symbol name or icon for the waypoint. + */ @Serializable @XmlSerialName("wpt", namespace = GPX_NAMESPACE, prefix = "") data class Wpt( @@ -79,8 +90,14 @@ data class Wpt( @XmlSerialName("sym", namespace = GPX_NAMESPACE, prefix = "") val sym: String? = null, ) { + /** + * Descriptive alias for [ele] (elevation in meters). + */ + val elevation: Double? + get() = ele + init { - require(lat.isFinite() && lon.isFinite() && (ele == null || ele.isFinite())) { + require(lat.isFinite() && lon.isFinite() && (elevation?.isFinite() ?: true)) { "GPX coordinate contains a non-finite value" } } diff --git a/data/src/test/java/com/google/maps/android/data/parser/gpx/GpxParserTest.kt b/data/src/test/java/com/google/maps/android/data/parser/gpx/GpxParserTest.kt index b92d04005..0600e13eb 100644 --- a/data/src/test/java/com/google/maps/android/data/parser/gpx/GpxParserTest.kt +++ b/data/src/test/java/com/google/maps/android/data/parser/gpx/GpxParserTest.kt @@ -108,4 +108,11 @@ class GpxParserTest { assertTrue(trkFeature.geometry is LineString) assertEquals("Trk1", trkFeature.properties["name"]) } + + @Test + fun `test Wpt elevation property alias`() { + val wpt = Wpt(lat = 1.0, lon = 2.0, ele = 123.45) + assertEquals(123.45, wpt.ele) + assertEquals(123.45, wpt.elevation) + } }