Repository: apple/swift-protobuf
Pull request #1179 (2021-09-16 19:29:26)

Track recursion depth for nested JSON ListValue structures
This prevents stack overrun for inputs like

   [[[[[[....[[[[[[]]]]]]....]]]]]]

Test case exercises direct nesting up to a million
levels.

Thanks to oss-fuzz for finding this!


Patch:
diff --git a/Sources/SwiftProtobuf/Google_Protobuf_ListValue+Extensions.swift b/Sources/SwiftProtobuf/Google_Protobuf_ListValue+Extensions.swift
index bbeffff0..a26e0d73 100644
--- a/Sources/SwiftProtobuf/Google_Protobuf_ListValue+Extensions.swift
+++ b/Sources/SwiftProtobuf/Google_Protobuf_ListValue+Extensions.swift
@@ -44,7 +44,11 @@ extension Google_Protobuf_ListValue: _CustomJSONCodable {
       return
     }
     try decoder.scanner.skipRequiredArrayStart()
+    // Since we override the JSON decoding, we can't rely
+    // on the default recursion depth tracking.
+    try decoder.scanner.incrementRecursionDepth()
     if decoder.scanner.skipOptionalArrayEnd() {
+      decoder.scanner.decrementRecursionDepth()
       return
     }
     while true {
@@ -52,6 +56,7 @@ extension Google_Protobuf_ListValue: _CustomJSONCodable {
       try v.decodeJSON(from: &decoder)
       values.append(v)
       if decoder.scanner.skipOptionalArrayEnd() {
+        decoder.scanner.decrementRecursionDepth()
         return
       }
       try decoder.scanner.skipRequiredComma()
diff --git a/Sources/SwiftProtobuf/JSONScanner.swift b/Sources/SwiftProtobuf/JSONScanner.swift
index b4c746c5..8960a409 100644
--- a/Sources/SwiftProtobuf/JSONScanner.swift
+++ b/Sources/SwiftProtobuf/JSONScanner.swift
@@ -410,14 +410,14 @@ internal struct JSONScanner {
     self.extensions = extensions ?? SimpleExtensionMap()
   }
 
-  private mutating func incrementRecursionDepth() throws {
+  internal mutating func incrementRecursionDepth() throws {
     recursionBudget -= 1
     if recursionBudget < 0 {
       throw JSONDecodingError.messageDepthLimit
     }
   }
 
-  private mutating func decrementRecursionDepth() {
+  internal mutating func decrementRecursionDepth() {
     recursionBudget += 1
     // This should never happen, if it does, something is probably corrupting memory, and
     // simply throwing doesn't make much sense.
diff --git a/Sources/SwiftProtobuf/TextFormatScanner.swift b/Sources/SwiftProtobuf/TextFormatScanner.swift
index e2bbbf6e..0e279084 100644
--- a/Sources/SwiftProtobuf/TextFormatScanner.swift
+++ b/Sources/SwiftProtobuf/TextFormatScanner.swift
@@ -262,7 +262,7 @@ internal struct TextFormatScanner {
         skipWhitespace()
     }
 
-    private mutating func incrementRecursionDepth() throws {
+    internal mutating func incrementRecursionDepth() throws {
         recursionBudget -= 1
         if recursionBudget < 0 {
             throw TextFormatDecodingError.messageDepthLimit


Test patch:
diff --git a/Tests/LinuxMain.swift b/Tests/LinuxMain.swift
index 75343a12..54489099 100644
--- a/Tests/LinuxMain.swift
+++ b/Tests/LinuxMain.swift
@@ -991,6 +991,7 @@ extension Test_JSON_ListValue {
     static var allTests = [
         ("testProtobuf", testProtobuf),
         ("testJSON", testJSON),
+        ("test_JSON_nested_list", test_JSON_nested_list),
         ("test_equality", test_equality)
     ]
 }
diff --git a/Tests/SwiftProtobufTests/Test_Struct.swift b/Tests/SwiftProtobufTests/Test_Struct.swift
index 18119f76..bd074b65 100644
--- a/Tests/SwiftProtobufTests/Test_Struct.swift
+++ b/Tests/SwiftProtobufTests/Test_Struct.swift
@@ -143,6 +143,42 @@ class Test_JSON_ListValue: XCTestCase, PBTestHelpers {
         assertJSONDecodeSucceeds("[true]") {$0.values == [Google_Protobuf_Value(boolValue: true)]}
     }
 
+    func test_JSON_nested_list() throws {
+        let limit = JSONDecodingOptions().messageDepthLimit
+        let depths = [
+            // Small lists
+            1,2,3,4,5,
+            // Little less than default messageDepthLimit, should succeed
+            limit - 3, limit - 2, limit - 1, limit,
+            // Little bigger than default messageDepthLimit, should fail
+            limit + 1, limit + 2, limit + 3, limit + 4,
+            // Really big, should fail cleanly (not crash)
+            1000,10000,100000,1000000
+        ]
+        for depth in depths {
+            var s = ""
+            for _ in 0..<(depth / 10) {
+                s.append("[[[[[[[[[[")
+            }
+            for _ in 0..<(depth % 10) {
+                s.append("[")
+            }
+            for _ in 0..<(depth / 10) {
+                s.append("]]]]]]]]]]")
+            }
+            for _ in 0..<(depth % 10) {
+                s.append("]")
+            }
+            // Recursion limits should cause this to
+            // fail cleanly without crashing
+            if depth <= limit {
+                assertJSONDecodeSucceeds(s) {_ in true}
+            } else {
+                assertJSONDecodeFails(s)
+            }
+        }
+    }
+
     func test_equality() throws {
         let a1decoded = try Google_Protobuf_ListValue(jsonString: "[1]")
         let a2decoded = try Google_Protobuf_ListValue(jsonString: "[2]")