Merge pull request #283 from jrudolph/limit-size-of-numbers

CVE-2018-18853 Limit the number of characters for numbers in the parser, fixes #278

3 files changed, 35 insertions, 5 deletions

diff --git a/src/main/scala/spray/json/JsonParser.scala b/src/main/scala/spray/json/JsonParser.scala
index 4a723b5..3efdac8 100644
--- a/src/main/scala/spray/json/JsonParser.scala
+++ b/src/main/scala/spray/json/JsonParser.scala
@@ -135,9 +135,19 @@ class JsonParser(input: ParserInput, settings: JsonParserSettings = JsonParserSe
     `int`()
     `frac`()
     `exp`()
+    val numberLength = input.cursor - start
+
     jsValue =
       if (startChar == '0' && input.cursor - start == 1) JsNumber.zero
-      else JsNumber(input.sliceCharArray(start, input.cursor))
+      else if (numberLength <= settings.maxNumberCharacters) JsNumber(input.sliceCharArray(start, input.cursor))
+      else {
+        val numberSnippet = new String(input.sliceCharArray(start, math.min(input.cursor, start + 20)))
+        throw new ParsingException("Number too long",
+          s"The number starting with '$numberSnippet' had " +
+          s"$numberLength characters which is more than the allowed limit maxNumberCharacters = ${settings.maxNumberCharacters}. If this is legit input " +
+          s"consider increasing the limit."
+        )
+      }
     ws()
   }
 
diff --git a/src/main/scala/spray/json/JsonParserSettings.scala b/src/main/scala/spray/json/JsonParserSettings.scala
index d07075e..b82c47d 100644
--- a/src/main/scala/spray/json/JsonParserSettings.scala
+++ b/src/main/scala/spray/json/JsonParserSettings.scala
@@ -12,16 +12,29 @@ trait JsonParserSettings {
   def maxDepth: Int
 
   /**
-   * Return a copy of this settings object with the `maxDepth` setting changed to the new value.
+   * Returns a copy of this settings object with the `maxDepth` setting changed to the new value.
    */
   def withMaxDepth(newValue: Int): JsonParserSettings
+
+  /**
+   * The maximum number of characters the parser should support for numbers. This is restricted because creating
+   * `BigDecimal`s with high precision can be very slow (approx. quadratic runtime per amount of characters).
+   */
+  def maxNumberCharacters: Int
+
+  /**
+   * Returns a copy of this settings object with the `maxNumberCharacters` setting changed to the new value.
+   */
+  def withMaxNumberCharacters(newValue: Int): JsonParserSettings
 }
 object JsonParserSettings {
   val default: JsonParserSettings = SettingsImpl()
 
   private case class SettingsImpl(
-    maxDepth: Int = 1000
+    maxDepth: Int = 1000,
+    maxNumberCharacters: Int = 100
   ) extends JsonParserSettings {
     override def withMaxDepth(newValue: Int): JsonParserSettings = copy(maxDepth = newValue)
+    override def withMaxNumberCharacters(newValue: Int): JsonParserSettings = copy(maxNumberCharacters = 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 9b849b3..1ca0ddc 100644
--- a/src/test/scala/spray/json/JsonParserSpec.scala
+++ b/src/test/scala/spray/json/JsonParserSpec.scala
@@ -118,8 +118,8 @@ class JsonParserSpec extends Specification {
     }
 
     "produce proper error messages" in {
-      def errorMessage(input: String) =
-        try JsonParser(input) catch { case e: JsonParser.ParsingException => e.getMessage }
+      def errorMessage(input: String, settings: JsonParserSettings = JsonParserSettings.default) =
+        try JsonParser(input, settings) catch { case e: JsonParser.ParsingException => e.getMessage }
 
       errorMessage("""[null, 1.23 {"key":true } ]""") ===
         """Unexpected character '{' at input index 12 (line 1, position 13), expected ']':
@@ -144,6 +144,13 @@ class JsonParserSpec extends Specification {
           |{}x
           |  ^
           |""".stripMargin
+
+      "reject numbers which are too big / have too high precision" in {
+        val settings = JsonParserSettings.default.withMaxNumberCharacters(5)
+        errorMessage("123.4567890", settings) ===
+          "Number too long:The number starting with '123.4567890' had 11 characters which is more than the allowed limit " +
+          "maxNumberCharacters = 5. If this is legit input consider increasing the limit."
+      }
     }
 
     "fail gracefully for deeply nested structures" in {