Skip to content

Commit

Permalink
CVE-2018-18855 Fix uncontrolled recursion in the JsonParser by imposi…
Browse files Browse the repository at this point in the history
…ng a configurable limit on the depth, fixes #286
  • Loading branch information
jrudolph committed Nov 7, 2018
1 parent 3ccb076 commit a558753
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 19 deletions.
42 changes: 24 additions & 18 deletions src/main/scala/spray/json/JsonParser.scala
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class JsonParser(input: ParserInput, settings: JsonParserSettings = JsonParserSe

def parseJsValue(allowTrailingInput: Boolean): JsValue = {
ws()
`value`()
`value`(settings.maxDepth)
if (!allowTrailingInput)
require(EOI)
jsValue
Expand All @@ -57,35 +57,41 @@ 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] = {
`string`()
require(':')
ws()
val key = sb.toString
`value`()
`value`(remainingNesting - 1)
val nextMap = map.updated(key, jsValue)
if (ws(',')) members(nextMap) else nextMap
}
Expand All @@ -101,12 +107,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()
}
Expand Down
19 changes: 18 additions & 1 deletion src/main/scala/spray/json/JsonParserSettings.scala
Original file line number Diff line number Diff line change
@@ -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)
}
}
26 changes: 26 additions & 0 deletions src/test/scala/spray/json/JsonParserSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ package spray.json

import org.specs2.mutable._

import scala.util.control.NonFatal

class JsonParserSpec extends Specification {

"The JsonParser" should {
Expand Down Expand Up @@ -114,6 +116,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))
Expand Down

0 comments on commit a558753

Please sign in to comment.