Skip to content

Commit

Permalink
Merge pull request #283 from jrudolph/limit-size-of-numbers
Browse files Browse the repository at this point in the history
CVE-2018-18853 Limit the number of characters for numbers in the parser, fixes #278
  • Loading branch information
jrudolph authored Nov 8, 2018
2 parents d56d7f4 + a8c45e7 commit b2f485e
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 5 deletions.
12 changes: 11 additions & 1 deletion src/main/scala/spray/json/JsonParser.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}

Expand Down
17 changes: 15 additions & 2 deletions src/main/scala/spray/json/JsonParserSettings.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
11 changes: 9 additions & 2 deletions src/test/scala/spray/json/JsonParserSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 ']':
Expand All @@ -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 {
Expand Down

0 comments on commit b2f485e

Please sign in to comment.