fix: bound untrusted GeoJSON input to prevent parser resource exhaustion (#1733)

* fix: bound untrusted GeoJSON input to prevent parser resource exhaustion

#1699 and #1710 added nesting-depth guards, but they run on the tree that
Json.parseToJsonElement() has already fully materialised, so they act too late:
GeoJsonParser.parse() reads and materialises the whole untrusted document
before any check. A hostile layer can therefore still

  - overflow the parser stack with deep nesting (StackOverflowError) before the
    MAX_GEOMETRY_DEPTH=20 guard runs (contrast KmlParser, which bounds depth
    *during* streaming via DepthLimitingReader);
  - exhaust the heap with a wide, shallow document (OutOfMemoryError), which no
    depth guard addresses;
  - forward non-finite coordinates ("Infinity"/"NaN", accepted by
    String.toDouble()) straight into LatLng/LatLngBounds.

The first two throw java.lang.Error subclasses, so the catch (Exception) in
DataLayerLoader does not contain them and the host app crashes on load.

Bound the input up front, before parseToJsonElement:

  - readTextBounded() caps the characters read (configurable, default 10 MiB);
  - checkStructuralDepth() rejects raw '{'/'[' nesting beyond a limit in a
    single string-aware pass (configurable, default 512 - safe on small Android
    stacks, far above any legitimate GeoJSON);
  - parseCoordinates() rejects non-finite values.

Limits are constructor parameters with safe defaults, mirroring KmzParser.
Existing behaviour is preserved (incl. the #1699 depth-200 GeometryCollection
test); adds tests for each case.

* test: make the deep-nesting test reproduce the overflow deterministically

The previous version of this test could pass on unpatched code for an
unrelated reason: with the nesting inside "coordinates", a parse that
survives the stack reaches parseCoordinates(), which calls jsonPrimitive
on a JsonArray and throws IllegalArgumentException - the very type the
test asserts.

Move the nesting under "bbox", a member the parser never dereferences,
so nothing can reject the document first, and run the parse on a thread
with an explicit 1 MB stack so the outcome does not depend on the stack
size of the thread the test framework happens to use. 50 000 levels
overflow that stack whether or not the parser is JIT-compiled, while the
structural-depth check rejects the document at nesting level 513 without
recursing at all. A small document is parsed first so that class loading
is not charged to the bounded stack.

On main this test now fails with StackOverflowError instead of passing.

Author: agilira

data/src/main/java/com/google/maps/android/data/parser/geojson/GeoJsonParser.kt

@@ -25,9 +25,17 @@ import kotlinx.serialization.json.jsonObject
 import kotlinx.serialization.json.jsonPrimitive
 import java.io.InputStream
 
-class GeoJsonParser {
+class GeoJsonParser(
+    private val maxInputSize: Long = DEFAULT_MAX_INPUT_SIZE,
+    private val maxStructuralDepth: Int = DEFAULT_MAX_STRUCTURAL_DEPTH,
+) {
     fun parse(inputStream: InputStream): GeoJsonObject? {
-        val json = inputStream.bufferedReader().use { it.readText() }
+        // Bound the untrusted input *before* materialising it. [Json.parseToJsonElement] reads the
+        // whole document into an in-memory tree, so without these guards a hostile document can
+        // exhaust memory (a wide document) or overflow the parser's stack (a deeply nested one)
+        // before the [MAX_GEOMETRY_DEPTH] check — which runs on the already-built tree — can act.
+        val json = inputStream.readTextBounded(maxInputSize)
+        checkStructuralDepth(json, maxStructuralDepth)
         val jsonElement = Json.parseToJsonElement(json)
 
         return when (jsonElement.jsonObject["type"]?.jsonPrimitive?.content) {
@@ -53,12 +61,84 @@ class GeoJsonParser {
 
     companion object {
         const val MAX_GEOMETRY_DEPTH = 20
+
+        /**
+         * Maximum number of characters read from an untrusted document. [Json.parseToJsonElement]
+         * materialises the whole document in memory (with a large text-to-object amplification), so an
+         * unbounded input is an [OutOfMemoryError] vector even with no nesting. The default is far
+         * above any realistic layer (~60x the largest bundled sample); raise it via the constructor
+         * for trusted large sources.
+         */
+        const val DEFAULT_MAX_INPUT_SIZE = 10L * 1024 * 1024
+
+        /**
+         * Maximum raw structural nesting (`{`/`[`) accepted. [Json.parseToJsonElement] parses nested
+         * JSON with stack recursion, so a deeply nested document overflows the stack *before* the
+         * [MAX_GEOMETRY_DEPTH] guard (which runs on the already-built tree) can act. Chosen to stay
+         * safe on small Android thread stacks while remaining far above any legitimate GeoJSON, whose
+         * structural depth is well under 100 even with maximally nested geometries.
+         */
+        const val DEFAULT_MAX_STRUCTURAL_DEPTH = 512
+
         val SUPPORTED_EXTENSIONS = setOf("json", "geojson")
 
         fun canParse(header: String): Boolean = header.trimStart().startsWith("{")
     }
 }
 
+/**
+ * Reads the stream as UTF-8 text, failing fast with an [IllegalArgumentException] once more than
+ * [maxChars] characters have been read, so an oversized untrusted document is never fully materialised.
+ */
+private fun InputStream.readTextBounded(maxChars: Long): String {
+    val reader = bufferedReader()
+    val builder = StringBuilder()
+    val buffer = CharArray(8 * 1024)
+    var total = 0L
+    while (true) {
+        val read = reader.read(buffer)
+        if (read < 0) break
+        total += read
+        require(total <= maxChars) {
+            "GeoJSON input exceeds the maximum allowed size of $maxChars characters"
+        }
+        builder.append(buffer, 0, read)
+    }
+    return builder.toString()
+}
+
+/**
+ * Rejects, with an [IllegalArgumentException], any document whose raw structural nesting (`{`/`[`)
+ * exceeds [maxDepth]. Runs in a single pass that ignores brackets inside string literals, *before*
+ * [Json.parseToJsonElement], so a maliciously deep document is rejected instead of overflowing the
+ * parser's stack.
+ */
+private fun checkStructuralDepth(json: String, maxDepth: Int) {
+    var depth = 0
+    var inString = false
+    var escaped = false
+    for (c in json) {
+        if (inString) {
+            when {
+                escaped -> escaped = false
+                c == '\\' -> escaped = true
+                c == '"' -> inString = false
+            }
+            continue
+        }
+        when (c) {
+            '"' -> inString = true
+            '[', '{' -> {
+                depth++
+                require(depth <= maxDepth) {
+                    "GeoJSON structural nesting exceeds the maximum depth of $maxDepth"
+                }
+            }
+            ']', '}' -> if (depth > 0) depth--
+        }
+    }
+}
+
 private fun parseFeatureCollection(json: JsonElement): GeoJsonFeatureCollection {
     val featuresJson = json.jsonObject["features"]?.jsonArray
     val features = featuresJson?.map { parseFeature(it) } ?: emptyList()
@@ -118,6 +198,11 @@ private fun parseCoordinates(coordinates: List<JsonElement>): Coordinates {
     val lng = coordinates[0].jsonPrimitive.content.toDouble()
     val lat = coordinates[1].jsonPrimitive.content.toDouble()
     val alt = if (coordinates.size > 2) coordinates[2].jsonPrimitive.content.toDouble() else null
+    // Reject non-finite values (e.g. "Infinity"/"NaN", which String.toDouble() accepts) so poisoned
+    // coordinates cannot flow unchecked into LatLng/LatLngBounds and corrupt rendering downstream.
+    require(lng.isFinite() && lat.isFinite() && (alt == null || alt.isFinite())) {
+        "GeoJSON coordinate contains a non-finite value"
+    }
     return Coordinates(lat, lng, alt)
 }
 

data/src/test/java/com/google/maps/android/data/parser/geojson/GeoJsonParserTest.kt

@@ -326,6 +326,87 @@ class GeoJsonParserTest {
         assertThat(parseGeometry(element, -1)).isNull()
     }
 
+    @Test
+    fun testDeeplyNestedArraysAreRejectedInsteadOfOverflowingTheParserStack() {
+        // Json.parseToJsonElement() descends into nested *arrays* on the call stack
+        // (JsonTreeReader.readArray -> read -> readArray ...; only nested objects switch to the
+        // stackless path), and it runs before anything inspects the parsed tree, so the
+        // MAX_GEOMETRY_DEPTH guard never sees this document: unpatched, the parse dies with a
+        // StackOverflowError, which is an Error and therefore not contained by the
+        // catch (Exception) in DataLayerLoader.
+        //
+        // Three details keep that deterministic rather than machine-dependent:
+        //  - the nesting sits under "bbox", a member the parser never dereferences, so no
+        //    coordinate/type check can reject the document first and mask the overflow;
+        //  - the parse runs on a thread with an explicit 1 MB stack, the usual size of a worker
+        //    thread, instead of whatever (much larger) stack the test runner's thread has. 1 MB is
+        //    above every platform minimum, so the request is honoured as-is;
+        //  - 50 000 levels overflow that stack whether or not the parser is JIT-compiled (the
+        //    threshold is a few thousand levels interpreted and ~17 000 once compiled), while the
+        //    structural-depth check rejects the document at nesting level 513 without recursing
+        //    at all.
+        // A small document is parsed first so that class loading is not charged to the bounded
+        // stack.
+        val point = """{"type": "Point", "coordinates": [0.0, 0.0]}"""
+        parser.parse(ByteArrayInputStream(point.toByteArray()))
+
+        val deep = "[".repeat(50000) + "1" + "]".repeat(50000)
+        val geoJson = """{"type": "FeatureCollection", "bbox": $deep, "features": []}"""
+        assertFailsWith<IllegalArgumentException> {
+            parseOnThreadWithStackSize(1024L * 1024) {
+                parser.parse(ByteArrayInputStream(geoJson.toByteArray()))
+            }
+        }
+    }
+
+    @Test
+    fun testExcessivelyNestedGeometryCollectionIsRejected() {
+        val nested = buildNestedGeometryCollectionJson(1000)
+        val stream = ByteArrayInputStream(nested.toByteArray())
+        assertFailsWith<IllegalArgumentException> { parser.parse(stream) }
+    }
+
+    @Test
+    fun testOversizedInputIsRejected() {
+        val boundedParser = GeoJsonParser(maxInputSize = 1024L)
+        val big =
+            "{\"type\":\"MultiPoint\",\"coordinates\":[" +
+                (0 until 2000).joinToString(",") { "[1,1]" } + "]}"
+        val stream = ByteArrayInputStream(big.toByteArray())
+        assertFailsWith<IllegalArgumentException> { boundedParser.parse(stream) }
+    }
+
+    @Test
+    fun testNonFiniteCoordinateIsRejected() {
+        val geoJson =
+            """{"type": "Feature", "geometry": {"type": "Point", "coordinates": ["Infinity", "NaN"]}}"""
+        val stream = ByteArrayInputStream(geoJson.toByteArray())
+        assertFailsWith<IllegalArgumentException> { parser.parse(stream) }
+    }
+
+    /**
+     * Runs [block] on a thread with a fixed stack size and rethrows whatever it threw, so that a
+     * test about stack consumption does not depend on the stack size of the thread the test
+     * framework happens to use.
+     */
+    private fun parseOnThreadWithStackSize(
+        stackSizeBytes: Long,
+        block: () -> Unit,
+    ) {
+        var thrown: Throwable? = null
+        val thread =
+            Thread(null, {
+                try {
+                    block()
+                } catch (t: Throwable) {
+                    thrown = t
+                }
+            }, "geojson-parse", stackSizeBytes)
+        thread.start()
+        thread.join()
+        thrown?.let { throw it }
+    }
+
     private fun buildNestedGeometryCollectionJson(depth: Int): String {
         var current = """{"type": "Point", "coordinates": [0.0, 0.0]}"""
         repeat(depth) {