Merge pull request #284 from jrudolph/fix-uncontrolled-recursion

CVE-2018-18855 Fix uncontrolled recursion in JsonParser

3 files changed, 68 insertions, 19 deletions

diff --git a/src/main/scala/spray/json/JsonParser.scala b/src/main/scala/spray/json/JsonParser.scala
index ded8d6a..4a723b5 100644
--- a/src/main/scala/spray/json/JsonParser.scala
+++ b/src/main/scala/spray/json/JsonParser.scala
@@ -48,7 +48,7 @@ class JsonParser(input: ParserInput, settings: JsonParserSettings = JsonParserSe
 
   def parseJsValue(allowTrailingInput: Boolean): JsValue = {
     ws()
-    `value`()
+    `value`(settings.maxDepth)
     if (!allowTrailingInput)
       require(EOI)
     jsValue
@@ -59,27 +59,33 @@ class JsonParser(input: ParserInput, settings: JsonParserSettings = JsonParserSe
   private final val EOI = '\uFFFF' // compile-time constant
 
   // http://tools.ietf.org/html/rfc4627#section-2.1
-  private def `value`(): Unit = {
-    val mark = input.cursor
-    def simpleValue(matched: Boolean, value: JsValue) = if (matched) jsValue = value else fail("JSON Value", mark)
-    (cursorChar: @switch) match {
-      case 'f' => simpleValue(`false`(), JsFalse)
-      case 'n' => simpleValue(`null`(), JsNull)
-      case 't' => simpleValue(`true`(), JsTrue)
-      case '{' => advance(); `object`()
-      case '[' => advance(); `array`()
-      case '0' | '1' | '2' | '3' | '4' | '5' | '6' | '7' | '8' | '9' | '-' => `number`()
-      case '"' => `string`(); jsValue = if (sb.length == 0) JsString.empty else JsString(sb.toString)
-      case _ => fail("JSON Value")
+  private def `value`(remainingNesting: Int): Unit =
+    if (remainingNesting == 0)
+      throw new ParsingException(
+        "JSON input nested too deeply",
+        s"JSON input was nested more deeply than the configured limit of maxNesting = ${settings.maxDepth}"
+      )
+    else {
+      val mark = input.cursor
+      def simpleValue(matched: Boolean, value: JsValue) = if (matched) jsValue = value else fail("JSON Value", mark)
+      (cursorChar: @switch) match {
+        case 'f' => simpleValue(`false`(), JsFalse)
+        case 'n' => simpleValue(`null`(), JsNull)
+        case 't' => simpleValue(`true`(), JsTrue)
+        case '{' => advance(); `object`(remainingNesting)
+        case '[' => advance(); `array`(remainingNesting)
+        case '0' | '1' | '2' | '3' | '4' | '5' | '6' | '7' | '8' | '9' | '-' => `number`()
+        case '"' => `string`(); jsValue = if (sb.length == 0) JsString.empty else JsString(sb.toString)
+        case _ => fail("JSON Value")
+      }
     }
-  }
 
   private def `false`() = advance() && ch('a') && ch('l') && ch('s') && ws('e')
   private def `null`() = advance() && ch('u') && ch('l') && ws('l')
   private def `true`() = advance() && ch('r') && ch('u') && ws('e')
 
   // http://tools.ietf.org/html/rfc4627#section-2.2
-  private def `object`(): Unit = {
+  private def `object`(remainingNesting: Int): Unit = {
     ws()
     jsValue = if (cursorChar != '}') {
       @tailrec def members(map: Map[String, JsValue]): Map[String, JsValue] = {
@@ -87,7 +93,7 @@ class JsonParser(input: ParserInput, settings: JsonParserSettings = JsonParserSe
         require(':')
         ws()
         val key = sb.toString
-        `value`()
+        `value`(remainingNesting - 1)
         val nextMap = map.updated(key, jsValue)
         if (ws(',')) members(nextMap) else nextMap
       }
@@ -102,12 +108,12 @@ class JsonParser(input: ParserInput, settings: JsonParserSettings = JsonParserSe
   }
 
   // http://tools.ietf.org/html/rfc4627#section-2.3
-  private def `array`(): Unit = {
+  private def `array`(remainingNesting: Int): Unit = {
     ws()
     jsValue = if (cursorChar != ']') {
       val list = Vector.newBuilder[JsValue]
       @tailrec def values(): Unit = {
-        `value`()
+        `value`(remainingNesting - 1)
         list += jsValue
         if (ws(',')) values()
       }
diff --git a/src/main/scala/spray/json/JsonParserSettings.scala b/src/main/scala/spray/json/JsonParserSettings.scala
index 31692fd..d07075e 100644
--- a/src/main/scala/spray/json/JsonParserSettings.scala
+++ b/src/main/scala/spray/json/JsonParserSettings.scala
@@ -1,10 +1,27 @@
 package spray.json
 
 trait JsonParserSettings {
+  /**
+   * The JsonParser uses recursive decent parsing that keeps intermediate values on the stack. To prevent
+   * StackOverflowExceptions a limit is enforced on the depth of the parsed JSON structure.
+   *
+   * As a guideline we tested that one level of depth needs about 300 bytes of stack space.
+   *
+   * The default is a depth of 1000.
+   */
+  def maxDepth: Int
 
+  /**
+   * Return a copy of this settings object with the `maxDepth` setting changed to the new value.
+   */
+  def withMaxDepth(newValue: Int): JsonParserSettings
 }
 object JsonParserSettings {
   val default: JsonParserSettings = SettingsImpl()
 
-  private case class SettingsImpl() extends JsonParserSettings
+  private case class SettingsImpl(
+    maxDepth: Int = 1000
+  ) extends JsonParserSettings {
+    override def withMaxDepth(newValue: Int): JsonParserSettings = copy(maxDepth = newValue)
+  }
 }
\ No newline at end of file
diff --git a/src/test/scala/spray/json/JsonParserSpec.scala b/src/test/scala/spray/json/JsonParserSpec.scala
index 36c9726..9b849b3 100644
--- a/src/test/scala/spray/json/JsonParserSpec.scala
+++ b/src/test/scala/spray/json/JsonParserSpec.scala
@@ -18,6 +18,8 @@ package spray.json
 
 import org.specs2.mutable._
 
+import scala.util.control.NonFatal
+
 class JsonParserSpec extends Specification {
 
   "The JsonParser" should {
@@ -144,6 +146,30 @@ class JsonParserSpec extends Specification {
           |""".stripMargin
     }
 
+    "fail gracefully for deeply nested structures" in {
+      val queue = new java.util.ArrayDeque[String]()
+
+      // testing revealed that each recursion will need approx. 280 bytes of stack space
+      val depth = 1500
+      val runnable = new Runnable {
+        override def run(): Unit =
+          try {
+            val nested = "[{\"key\":" * (depth / 2)
+            JsonParser(nested)
+            queue.push("didn't fail")
+          } catch {
+            case s: StackOverflowError => queue.push("stackoverflow")
+            case NonFatal(e) =>
+              queue.push(s"nonfatal: ${e.getMessage}")
+          }
+      }
+
+      val thread = new Thread(null, runnable, "parser-test", 655360)
+      thread.start()
+      thread.join()
+      queue.peek() === "nonfatal: JSON input nested too deeply:JSON input was nested more deeply than the configured limit of maxNesting = 1000"
+    }
+
     "parse multiple values when allowTrailingInput" in {
       val parser = new JsonParser("""{"key":1}{"key":2}""")
       parser.parseJsValue(true) === JsObject("key" -> JsNumber(1))